[compiler-rt] e1dcd4b - [compiler-rt][builtins] Add compiler flags to catch potential errors

Akira Hatanaka via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 24 15:53:56 PDT 2022


Author: Akira Hatanaka
Date: 2022-08-24T15:52:31-07:00
New Revision: e1dcd4ba444b0aaac05c399670d870925cef4459

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

LOG: [compiler-rt][builtins] Add compiler flags to catch potential errors
that can lead to security vulnerabilities

Also, fix a few places that were causing -Wshadow and
-Wformat-nonliteral warnings to be emitted.

This reapplies the patch that was reverted in 0d66dc57e8c7 because it
broke a few bots.

I made changes so that cmake checks whether some of the flags are
supported by the compiler that is used before adding them to the list.
Also, I moved function add_security_warnings to CompilerRTUtils.cmake so
that it is defined before it's used.

Differential Revision: https://reviews.llvm.org/D131714

Added: 
    

Modified: 
    compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
    compiler-rt/cmake/Modules/CompilerRTUtils.cmake
    compiler-rt/cmake/config-ix.cmake
    compiler-rt/include/profile/InstrProfData.inc
    compiler-rt/lib/builtins/CMakeLists.txt
    compiler-rt/lib/builtins/eprintf.c
    compiler-rt/lib/profile/InstrProfiling.c
    compiler-rt/lib/profile/InstrProfilingWriter.c

Removed: 
    


################################################################################
diff  --git a/compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake b/compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
index 2c9983c6a1ae3..57e913ad81c8a 100644
--- a/compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
+++ b/compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
@@ -413,6 +413,12 @@ macro(darwin_add_builtin_libraries)
                       ../profile/InstrProfilingInternal.c
                       ../profile/InstrProfilingVersionVar.c)
   foreach (os ${ARGN})
+    set(macosx_sdk_version 0)
+    if ("${os}" STREQUAL "osx")
+      find_darwin_sdk_version(macosx_sdk_version "macosx")
+    endif()
+    add_security_warnings(CFLAGS ${macosx_sdk_version})
+
     list_intersect(DARWIN_BUILTIN_ARCHS DARWIN_${os}_BUILTIN_ARCHS BUILTIN_SUPPORTED_ARCH)
 
     if((arm64 IN_LIST DARWIN_BUILTIN_ARCHS OR arm64e IN_LIST DARWIN_BUILTIN_ARCHS) AND NOT TARGET lse_builtin_symlinks)

diff  --git a/compiler-rt/cmake/Modules/CompilerRTUtils.cmake b/compiler-rt/cmake/Modules/CompilerRTUtils.cmake
index 9b5e03a6607ba..2f28824d8dcb8 100644
--- a/compiler-rt/cmake/Modules/CompilerRTUtils.cmake
+++ b/compiler-rt/cmake/Modules/CompilerRTUtils.cmake
@@ -593,3 +593,28 @@ function(add_compiler_rt_install_targets name)
     endif()
   endif()
 endfunction()
+
+# Add warnings to catch potential errors that can lead to security
+# vulnerabilities.
+function(add_security_warnings out_flags macosx_sdk_version)
+  set(flags "${${out_flags}}" -Werror=format-security -Werror=array-bounds
+      -Werror=uninitialized -Werror=shadow -Werror=empty-body
+      -Werror=sizeof-pointer-memaccess -Werror=sizeof-array-argument
+      -Werror=memset-transposed-args)
+
+  append_list_if(COMPILER_RT_HAS_BUILTIN_MEMCPY_CHK_SIZE_FLAG -Werror=builtin-memcpy-chk-size flags)
+  append_list_if(COMPILER_RT_HAS_ARRAY_BOUNDS_POINTER_ARITHMETIC_FLAG -Werror=array-bounds-pointer-arithmetic flags)
+  append_list_if(COMPILER_RT_HAS_RETURN_STACK_ADDRESS_FLAG -Werror=return-stack-address flags)
+  append_list_if(COMPILER_RT_HAS_SIZEOF_ARRAY_DECAY_FLAG -Werror=sizeof-array-decay flags)
+  append_list_if(COMPILER_RT_HAS_FORMAT_INSUFFICIENT_ARGS_FLAG -Werror=format-insufficient-args flags)
+
+  # Add -Wformat-nonliteral only if we can avoid adding the defintion of
+  # eprintf. On Apple platforms, eprintf is needed only on macosx and only if
+  # its version is older than 10.7.
+  if ("${macosx_sdk_version}" VERSION_GREATER_EQUAL 10.7 OR
+      "${macosx_sdk_version}" EQUAL 0)
+    list(APPEND flags -Werror=format-nonliteral -DDONT_DEFINE_EPRINTF)
+  endif()
+
+  set(${out_flags} "${flags}" PARENT_SCOPE)
+endfunction()

diff  --git a/compiler-rt/cmake/config-ix.cmake b/compiler-rt/cmake/config-ix.cmake
index afa3596b1aa79..3af5e7ff218be 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -137,6 +137,12 @@ check_cxx_compiler_flag(/wd4391 COMPILER_RT_HAS_WD4391_FLAG)
 check_cxx_compiler_flag(/wd4722 COMPILER_RT_HAS_WD4722_FLAG)
 check_cxx_compiler_flag(/wd4800 COMPILER_RT_HAS_WD4800_FLAG)
 
+check_cxx_compiler_flag(-Wbuiltin-memcpy-chk-size COMPILER_RT_HAS_BUILTIN_MEMCPY_CHK_SIZE_FLAG)
+check_cxx_compiler_flag(-Warray-bounds-pointer-arithmetic COMPILER_RT_HAS_ARRAY_BOUNDS_POINTER_ARITHMETIC_FLAG)
+check_cxx_compiler_flag(-Wreturn-stack-address COMPILER_RT_HAS_RETURN_STACK_ADDRESS_FLAG)
+check_cxx_compiler_flag(-Wsizeof-array-decay COMPILER_RT_HAS_SIZEOF_ARRAY_DECAY_FLAG)
+check_cxx_compiler_flag(-Wformat-insufficient-args COMPILER_RT_HAS_FORMAT_INSUFFICIENT_ARGS_FLAG)
+
 # Symbols.
 check_symbol_exists(__func__ "" COMPILER_RT_HAS_FUNC_SYMBOL)
 

diff  --git a/compiler-rt/include/profile/InstrProfData.inc b/compiler-rt/include/profile/InstrProfData.inc
index 282620d8b5dc0..a92f5f62b18af 100644
--- a/compiler-rt/include/profile/InstrProfData.inc
+++ b/compiler-rt/include/profile/InstrProfData.inc
@@ -129,10 +129,10 @@ INSTR_PROF_RAW_HEADER(uint64_t, Magic, __llvm_profile_get_magic())
 INSTR_PROF_RAW_HEADER(uint64_t, Version, __llvm_profile_get_version())
 INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL))
 /* FIXME: A more accurate name is NumData */
-INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSize)
+INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSizeInitVal)
 INSTR_PROF_RAW_HEADER(uint64_t, PaddingBytesBeforeCounters, PaddingBytesBeforeCounters)
 /* FIXME: A more accurate name is NumCounters */
-INSTR_PROF_RAW_HEADER(uint64_t, CountersSize, CountersSize)
+INSTR_PROF_RAW_HEADER(uint64_t, CountersSize, CountersSizeInitVal)
 INSTR_PROF_RAW_HEADER(uint64_t, PaddingBytesAfterCounters, PaddingBytesAfterCounters)
 INSTR_PROF_RAW_HEADER(uint64_t, NamesSize,  NamesSize)
 INSTR_PROF_RAW_HEADER(uint64_t, CountersDelta,

diff  --git a/compiler-rt/lib/builtins/CMakeLists.txt b/compiler-rt/lib/builtins/CMakeLists.txt
index bbba2497fce30..8baf4f51a5888 100644
--- a/compiler-rt/lib/builtins/CMakeLists.txt
+++ b/compiler-rt/lib/builtins/CMakeLists.txt
@@ -699,6 +699,7 @@ if (APPLE)
   darwin_add_builtin_libraries(${BUILTIN_SUPPORTED_OS})
 else ()
   set(BUILTIN_CFLAGS "")
+  add_security_warnings(BUILTIN_CFLAGS -1)
 
   if (COMPILER_RT_HAS_FCF_PROTECTION_FLAG)
     append_list_if(COMPILER_RT_ENABLE_CET -fcf-protection=full BUILTIN_CFLAGS)

diff  --git a/compiler-rt/lib/builtins/eprintf.c b/compiler-rt/lib/builtins/eprintf.c
index 89fb0e315b2ee..daf90b4993eca 100644
--- a/compiler-rt/lib/builtins/eprintf.c
+++ b/compiler-rt/lib/builtins/eprintf.c
@@ -15,6 +15,7 @@
 //
 // It should never be exported from a dylib, so it is marked
 // visibility hidden.
+#ifndef DONT_DEFINE_EPRINTF
 #ifndef _WIN32
 __attribute__((visibility("hidden")))
 #endif
@@ -25,3 +26,4 @@ __eprintf(const char *format, const char *assertion_expression,
   fflush(stderr);
   compilerrt_abort();
 }
+#endif

diff  --git a/compiler-rt/lib/profile/InstrProfiling.c b/compiler-rt/lib/profile/InstrProfiling.c
index 4bf8463f1ef9c..fdb7b7cd806ce 100644
--- a/compiler-rt/lib/profile/InstrProfiling.c
+++ b/compiler-rt/lib/profile/InstrProfiling.c
@@ -64,11 +64,11 @@ COMPILER_RT_VISIBILITY void __llvm_profile_reset_counters(void) {
       CurrentVSiteCount += DI->NumValueSites[VKI];
 
     for (i = 0; i < CurrentVSiteCount; ++i) {
-      ValueProfNode *CurrentVNode = ValueCounters[i];
+      ValueProfNode *CurrVNode = ValueCounters[i];
 
-      while (CurrentVNode) {
-        CurrentVNode->Count = 0;
-        CurrentVNode = CurrentVNode->Next;
+      while (CurrVNode) {
+        CurrVNode->Count = 0;
+        CurrVNode = CurrVNode->Next;
       }
     }
   }

diff  --git a/compiler-rt/lib/profile/InstrProfilingWriter.c b/compiler-rt/lib/profile/InstrProfilingWriter.c
index 800103d199ab0..1209f753773b6 100644
--- a/compiler-rt/lib/profile/InstrProfilingWriter.c
+++ b/compiler-rt/lib/profile/InstrProfilingWriter.c
@@ -291,8 +291,8 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const __llvm_profile_data *DataBegin,
     // TODO: Unfortunately the header's fields are named DataSize and
     // CountersSize when they should be named NumData and NumCounters,
     // respectively.
-    const uint64_t CountersSize = NumCounters;
-    const uint64_t DataSize = NumData;
+    const uint64_t CountersSizeInitVal = NumCounters;
+    const uint64_t DataSizeInitVal = NumData;
 /* Initialize header structure.  */
 #define INSTR_PROF_RAW_HEADER(Type, Name, Init) Header.Name = Init;
 #include "profile/InstrProfData.inc"


        


More information about the llvm-commits mailing list