Skip to content

Commit 9c8f888

Browse files
committed
sanitizer_common: prepare for enabling format string checking
The __attribute__((format)) was added somewhere in 2012, the lost during refactoring, then re-added in 2014 but to te source files, which is a no-op. Move it back to header files so that it actually takes effect. But over the past 7 years we've accumulated whole lot of format string bugs of different types, so disable the warning with -Wno-format for now for incremental clean up. Among the bugs that it warns about are all kinds of bad things: - wrong sizes of arguments - missing/excessive arguments - printing wrong things (e.g. *ptr instead of ptr) - completely messed up format strings - security issues where external string is used as format Reviewed By: vitalybuka Differential Revision: https://reviews.llvm.org/D107977
1 parent e772e25 commit 9c8f888

File tree

5 files changed

+11
-8
lines changed

5 files changed

+11
-8
lines changed

compiler-rt/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,10 @@ endif()
438438
append_list_if(COMPILER_RT_HAS_WGNU_FLAG -Wno-gnu SANITIZER_COMMON_CFLAGS)
439439
append_list_if(COMPILER_RT_HAS_WVARIADIC_MACROS_FLAG -Wno-variadic-macros SANITIZER_COMMON_CFLAGS)
440440
append_list_if(COMPILER_RT_HAS_WC99_EXTENSIONS_FLAG -Wno-c99-extensions SANITIZER_COMMON_CFLAGS)
441+
# Too many existing bugs, needs cleanup.
442+
append_list_if(COMPILER_RT_HAS_WNO_FORMAT -Wno-format SANITIZER_COMMON_CFLAGS)
443+
# format-pedantic warns about passing T* for %p, which is not useful.
444+
append_list_if(COMPILER_RT_HAS_WNO_FORMAT_PEDANTIC -Wno-format-pedantic SANITIZER_COMMON_CFLAGS)
441445
append_list_if(COMPILER_RT_HAS_WD4146_FLAG /wd4146 SANITIZER_COMMON_CFLAGS)
442446
append_list_if(COMPILER_RT_HAS_WD4291_FLAG /wd4291 SANITIZER_COMMON_CFLAGS)
443447
append_list_if(COMPILER_RT_HAS_WD4391_FLAG /wd4391 SANITIZER_COMMON_CFLAGS)

compiler-rt/cmake/config-ix.cmake

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ check_cxx_compiler_flag("-Werror -Wunused-parameter" COMPILER_RT_HAS_WUNUSED_P
114114
check_cxx_compiler_flag("-Werror -Wcovered-switch-default" COMPILER_RT_HAS_WCOVERED_SWITCH_DEFAULT_FLAG)
115115
check_cxx_compiler_flag("-Werror -Wsuggest-override" COMPILER_RT_HAS_WSUGGEST_OVERRIDE_FLAG)
116116
check_cxx_compiler_flag(-Wno-pedantic COMPILER_RT_HAS_WNO_PEDANTIC)
117+
check_cxx_compiler_flag(-Wno-format COMPILER_RT_HAS_WNO_FORMAT)
118+
check_cxx_compiler_flag(-Wno-format-pedantic COMPILER_RT_HAS_WNO_FORMAT_PEDANTIC)
117119

118120
check_cxx_compiler_flag(/W4 COMPILER_RT_HAS_W4_FLAG)
119121
check_cxx_compiler_flag(/WX COMPILER_RT_HAS_WX_FLAG)

compiler-rt/lib/sanitizer_common/sanitizer_common.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,8 @@ void CatastrophicErrorWrite(const char *buffer, uptr length);
222222
void RawWrite(const char *buffer);
223223
bool ColorizeReports();
224224
void RemoveANSIEscapeSequencesFromString(char *buffer);
225-
void Printf(const char *format, ...);
226-
void Report(const char *format, ...);
225+
void Printf(const char *format, ...) FORMAT(1, 2);
226+
void Report(const char *format, ...) FORMAT(1, 2);
227227
void SetPrintfAndReportCallback(void (*callback)(const char *));
228228
#define VReport(level, ...) \
229229
do { \
@@ -618,7 +618,7 @@ class InternalScopedString {
618618
buffer_.resize(1);
619619
buffer_[0] = '\0';
620620
}
621-
void append(const char *format, ...);
621+
void append(const char *format, ...) FORMAT(2, 3);
622622
const char *data() const { return buffer_.data(); }
623623
char *data() { return buffer_.data(); }
624624

compiler-rt/lib/sanitizer_common/sanitizer_libc.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ char *internal_strrchr(const char *s, int c);
4949
char *internal_strstr(const char *haystack, const char *needle);
5050
// Works only for base=10 and doesn't set errno.
5151
s64 internal_simple_strtoll(const char *nptr, const char **endptr, int base);
52-
int internal_snprintf(char *buffer, uptr length, const char *format, ...);
52+
int internal_snprintf(char *buffer, uptr length, const char *format, ...)
53+
FORMAT(3, 4);
5354

5455
// Return true if all bytes in [mem, mem+size) are zero.
5556
// Optimized for the case when the result is true.

compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,6 @@ static void NOINLINE SharedPrintfCode(bool append_pid, const char *format,
317317
format, args);
318318
}
319319

320-
FORMAT(1, 2)
321320
void Printf(const char *format, ...) {
322321
va_list args;
323322
va_start(args, format);
@@ -326,7 +325,6 @@ void Printf(const char *format, ...) {
326325
}
327326

328327
// Like Printf, but prints the current PID before the output string.
329-
FORMAT(1, 2)
330328
void Report(const char *format, ...) {
331329
va_list args;
332330
va_start(args, format);
@@ -338,7 +336,6 @@ void Report(const char *format, ...) {
338336
// Returns the number of symbols that should have been written to buffer
339337
// (not including trailing '\0'). Thus, the string is truncated
340338
// iff return value is not less than "length".
341-
FORMAT(3, 4)
342339
int internal_snprintf(char *buffer, uptr length, const char *format, ...) {
343340
va_list args;
344341
va_start(args, format);
@@ -347,7 +344,6 @@ int internal_snprintf(char *buffer, uptr length, const char *format, ...) {
347344
return needed_length;
348345
}
349346

350-
FORMAT(2, 3)
351347
void InternalScopedString::append(const char *format, ...) {
352348
uptr prev_len = length();
353349

0 commit comments

Comments
 (0)