[compiler-rt] 9c8f888 - sanitizer_common: prepare for enabling format string checking

Dmitry Vyukov via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 13 04:44:03 PDT 2021


Author: Dmitry Vyukov
Date: 2021-08-13T13:43:57+02:00
New Revision: 9c8f888f5fcafe8a9b69e7d652dc7328612f8ec6

URL: https://github.com/llvm/llvm-project/commit/9c8f888f5fcafe8a9b69e7d652dc7328612f8ec6
DIFF: https://github.com/llvm/llvm-project/commit/9c8f888f5fcafe8a9b69e7d652dc7328612f8ec6.diff

LOG: 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

Added: 
    

Modified: 
    compiler-rt/CMakeLists.txt
    compiler-rt/cmake/config-ix.cmake
    compiler-rt/lib/sanitizer_common/sanitizer_common.h
    compiler-rt/lib/sanitizer_common/sanitizer_libc.h
    compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/CMakeLists.txt b/compiler-rt/CMakeLists.txt
index 1e242415ffdc4..28e267636723c 100644
--- a/compiler-rt/CMakeLists.txt
+++ b/compiler-rt/CMakeLists.txt
@@ -438,6 +438,10 @@ endif()
 append_list_if(COMPILER_RT_HAS_WGNU_FLAG -Wno-gnu SANITIZER_COMMON_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WVARIADIC_MACROS_FLAG -Wno-variadic-macros SANITIZER_COMMON_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WC99_EXTENSIONS_FLAG -Wno-c99-extensions SANITIZER_COMMON_CFLAGS)
+# Too many existing bugs, needs cleanup.
+append_list_if(COMPILER_RT_HAS_WNO_FORMAT -Wno-format SANITIZER_COMMON_CFLAGS)
+# format-pedantic warns about passing T* for %p, which is not useful.
+append_list_if(COMPILER_RT_HAS_WNO_FORMAT_PEDANTIC -Wno-format-pedantic SANITIZER_COMMON_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WD4146_FLAG /wd4146 SANITIZER_COMMON_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WD4291_FLAG /wd4291 SANITIZER_COMMON_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WD4391_FLAG /wd4391 SANITIZER_COMMON_CFLAGS)

diff  --git a/compiler-rt/cmake/config-ix.cmake b/compiler-rt/cmake/config-ix.cmake
index a95c398741867..0dc8043d28b83 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -114,6 +114,8 @@ check_cxx_compiler_flag("-Werror -Wunused-parameter"   COMPILER_RT_HAS_WUNUSED_P
 check_cxx_compiler_flag("-Werror -Wcovered-switch-default" COMPILER_RT_HAS_WCOVERED_SWITCH_DEFAULT_FLAG)
 check_cxx_compiler_flag("-Werror -Wsuggest-override"   COMPILER_RT_HAS_WSUGGEST_OVERRIDE_FLAG)
 check_cxx_compiler_flag(-Wno-pedantic COMPILER_RT_HAS_WNO_PEDANTIC)
+check_cxx_compiler_flag(-Wno-format COMPILER_RT_HAS_WNO_FORMAT)
+check_cxx_compiler_flag(-Wno-format-pedantic COMPILER_RT_HAS_WNO_FORMAT_PEDANTIC)
 
 check_cxx_compiler_flag(/W4 COMPILER_RT_HAS_W4_FLAG)
 check_cxx_compiler_flag(/WX COMPILER_RT_HAS_WX_FLAG)

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
index c214f6f72cc68..7e7c94a37419b 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -222,8 +222,8 @@ void CatastrophicErrorWrite(const char *buffer, uptr length);
 void RawWrite(const char *buffer);
 bool ColorizeReports();
 void RemoveANSIEscapeSequencesFromString(char *buffer);
-void Printf(const char *format, ...);
-void Report(const char *format, ...);
+void Printf(const char *format, ...) FORMAT(1, 2);
+void Report(const char *format, ...) FORMAT(1, 2);
 void SetPrintfAndReportCallback(void (*callback)(const char *));
 #define VReport(level, ...)                                              \
   do {                                                                   \
@@ -618,7 +618,7 @@ class InternalScopedString {
     buffer_.resize(1);
     buffer_[0] = '\0';
   }
-  void append(const char *format, ...);
+  void append(const char *format, ...) FORMAT(2, 3);
   const char *data() const { return buffer_.data(); }
   char *data() { return buffer_.data(); }
 

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_libc.h b/compiler-rt/lib/sanitizer_common/sanitizer_libc.h
index bcb81ebbc803e..e2a71774b4b15 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_libc.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_libc.h
@@ -49,7 +49,8 @@ char *internal_strrchr(const char *s, int c);
 char *internal_strstr(const char *haystack, const char *needle);
 // Works only for base=10 and doesn't set errno.
 s64 internal_simple_strtoll(const char *nptr, const char **endptr, int base);
-int internal_snprintf(char *buffer, uptr length, const char *format, ...);
+int internal_snprintf(char *buffer, uptr length, const char *format, ...)
+    FORMAT(3, 4);
 
 // Return true if all bytes in [mem, mem+size) are zero.
 // Optimized for the case when the result is true.

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp
index b913c92e16f10..60e9d567d1fe4 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp
@@ -317,7 +317,6 @@ static void NOINLINE SharedPrintfCode(bool append_pid, const char *format,
                            format, args);
 }
 
-FORMAT(1, 2)
 void Printf(const char *format, ...) {
   va_list args;
   va_start(args, format);
@@ -326,7 +325,6 @@ void Printf(const char *format, ...) {
 }
 
 // Like Printf, but prints the current PID before the output string.
-FORMAT(1, 2)
 void Report(const char *format, ...) {
   va_list args;
   va_start(args, format);
@@ -338,7 +336,6 @@ void Report(const char *format, ...) {
 // Returns the number of symbols that should have been written to buffer
 // (not including trailing '\0'). Thus, the string is truncated
 // iff return value is not less than "length".
-FORMAT(3, 4)
 int internal_snprintf(char *buffer, uptr length, const char *format, ...) {
   va_list args;
   va_start(args, format);
@@ -347,7 +344,6 @@ int internal_snprintf(char *buffer, uptr length, const char *format, ...) {
   return needed_length;
 }
 
-FORMAT(2, 3)
 void InternalScopedString::append(const char *format, ...) {
   uptr prev_len = length();
 


        


More information about the llvm-commits mailing list