dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.12k stars 4.7k forks source link

Sanitizer failed to work #553

Closed xiangzhai closed 4 years ago

xiangzhai commented 4 years ago

Hi,

Base on this commit.

Let enablesanitizers.sh to support clang v8.0:

diff --git a/src/coreclr/enablesanitizers.sh b/src/coreclr/enablesanitizers.sh
index aedb95d..6429692 100755
--- a/src/coreclr/enablesanitizers.sh
+++ b/src/coreclr/enablesanitizers.sh
@@ -73,6 +73,11 @@ else
                 __ClangMinorVersion=9
                 __ExportSymbolizerPath=0
                 ;;
+            clang8.0)
+                __ClangMajorVersion=8
+                __ClangMinorVersion=0
+                __ExportSymbolizerPath=0
+                ;;
             *)
                 echo "Unknown arg: $i"
                 return 1

Sanitizer failed to work:

source ./src/coreclr/enablesanitizers.sh all clang8.0
./src/coreclr/build.sh skipcrossgen

...
<command line>:7:9: error: macro name must be an identifier                     
#define -fsanitize-blacklist /home/zhaixiang/runtime/src/coreclr/sanitizerblacklist.txt
        ^                                                                       
<command line>:8:9: error: macro name must be an identifier                     
#define -fsanitize address,bool,bounds,enum,float-cast-overflow,float-divide-by-zero,function,integer,nonnull-attribute,null,object-size,return,returns-nonnull-attribute,shift,unreachable,vla-bound,vptr
        ^                                                                       
<command line>:9:9: error: macro name must be an identifier                     
#define -fdata-sections 1                                                       
        ^                                                                       
<command line>:10:9: error: macro name must be an identifier                    
#define --ffunction-sections 1                                                  
        ^                                                                       
<command line>:11:9: error: macro name must be an identifier                    
#define -O1 1
...

Here is the detailed build log runtime.x64.Debug.log

Workaround is just revert the src/coreclr/configurecompiler.cmake https://github.com/dotnet/coreclr/pull/26980

diff --git a/src/coreclr/configurecompiler.cmake b/src/coreclr/configurecompiler.cmake
index 588aff2..7a7eafa 100644
--- a/src/coreclr/configurecompiler.cmake
+++ b/src/coreclr/configurecompiler.cmake
@@ -298,52 +298,37 @@ elseif (CLR_CMAKE_PLATFORM_UNIX)
     string(FIND "$ENV{DEBUG_SANITIZERS}" "asan" __ASAN_POS)
     string(FIND "$ENV{DEBUG_SANITIZERS}" "ubsan" __UBSAN_POS)
     if ((${__ASAN_POS} GREATER -1) OR (${__UBSAN_POS} GREATER -1))
-      list(APPEND CLR_SANITIZE_CXX_OPTIONS -fsanitize-blacklist=${CMAKE_CURRENT_SOURCE_DIR}/sanitizerblacklist.txt)
-      set (CLR_CXX_SANITIZERS "")
-      set (CLR_LINK_SANITIZERS "")
+      set(CLR_SANITIZE_CXX_FLAGS "${CLR_SANITIZE_CXX_FLAGS} -fsanitize-blacklist=${CMAKE_SOURCE_DIR}/sanitizerblacklist.txt -fsanitize=")
+      set(CLR_SANITIZE_LINK_FLAGS "${CLR_SANITIZE_LINK_FLAGS} -fsanitize=")
       if (${__ASAN_POS} GREATER -1)
-        list(APPEND CLR_CXX_SANITIZERS address)
-        list(APPEND CLR_LINK_SANITIZERS address)
         set(CLR_SANITIZE_CXX_FLAGS "${CLR_SANITIZE_CXX_FLAGS}address,")
         set(CLR_SANITIZE_LINK_FLAGS "${CLR_SANITIZE_LINK_FLAGS}address,")
         add_definitions(-DHAS_ASAN)
         message("Address Sanitizer (asan) enabled")
       endif ()
       if (${__UBSAN_POS} GREATER -1)
-        # all sanitizier flags are enabled except alignment (due to heavy use of __unaligned modifier)
-        list(APPEND CLR_CXX_SANITIZERS
-          "bool"
-          bounds
-          enum
-          float-cast-overflow
-          float-divide-by-zero
-          "function"
-          integer
-          nonnull-attribute
-          null
-          object-size
-          "return"
-          returns-nonnull-attribute
-          shift
-          unreachable
-          vla-bound
-          vptr)
-        list(APPEND CLR_LINK_SANITIZERS
-          undefined)
+        # all sanitizier flags are enabled except alignment (due to heavy use of __unaligned modifier) and vptr (due to -fno-rtti #553)
+        if (CLR_CMAKE_PLATFORM_ARCH_MIPS64)
+          set(CLR_SANITIZE_CXX_FLAGS "${CLR_SANITIZE_CXX_FLAGS}bool,bounds,enum,float-cast-overflow,float-divide-by-zero,integer,nonnull-attribute,null,object-size,return,returns-nonnull-attribute,shift,unreachable,vla-bound")
+        else ()
+          set(CLR_SANITIZE_CXX_FLAGS "${CLR_SANITIZE_CXX_FLAGS}bool,bounds,enum,float-cast-overflow,float-divide-by-zero,function,integer,nonnull-attribute,null,object-size,return,returns-nonnull-attribute,shift,unreachable,vla-bound")
+        endif ()
+        set(CLR_SANITIZE_LINK_FLAGS "${CLR_SANITIZE_LINK_FLAGS}undefined")
         message("Undefined Behavior Sanitizer (ubsan) enabled")
       endif ()
-      list(JOIN CLR_CXX_SANITIZERS "," CLR_CXX_SANITIZERS_OPTIONS)
-      list(APPEND CLR_SANITIZE_CXX_OPTIONS "-fsanitize=${CLR_CXX_SANITIZERS_OPTIONS}")
-      list(JOIN CLR_LINK_SANITIZERS "," CLR_LINK_SANITIZERS_OPTIONS)
-      list(APPEND CLR_SANITIZE_LINK_OPTIONS "-fsanitize=${CLR_LINK_SANITIZERS_OPTIONS}")

       # -fdata-sections -ffunction-sections: each function has own section instead of one per .o file (needed for --gc-sections)
+      # -fPIC: enable Position Independent Code normally just for shared libraries but required when linking with address sanitizer
       # -O1: optimization level used instead of -O0 to avoid compile error "invalid operand for inline asm constraint"
-      add_compile_definitions("$<$<OR:$<CONFIG:DEBUG>,$<CONFIG:CHECKED>>:${CLR_SANITIZE_CXX_OPTIONS};-fdata-sections;--ffunction-sections;-O1>")
-      add_link_options($<$<AND:$<OR:$<CONFIG:DEBUG>,$<CONFIG:CHECKED>>,$<STREQUAL:$<TARGET_PROPERTY:TYPE>,EXECUTABLE>>:${CLR_SANITIZE_LINK_OPTIONS}>)
+      set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} ${CLR_SANITIZE_CXX_FLAGS} -fdata-sections -ffunction-sections -fPIC -O1")
+      set(CMAKE_CXX_FLAGS_CHECKED "${CMAKE_CXX_FLAGS_CHECKED} ${CLR_SANITIZE_CXX_FLAGS} -fdata-sections -ffunction-sections -fPIC -O1")
+
+      set(CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} ${CLR_SANITIZE_LINK_FLAGS}")
+      set(CMAKE_EXE_LINKER_FLAGS_CHECKED "${CMAKE_EXE_LINKER_FLAGS_CHECKED} ${CLR_SANITIZE_LINK_FLAGS}")

       # -Wl and --gc-sections: drop unused sections\functions (similar to Windows /Gy function-level-linking)
-      add_link_options("$<$<AND:$<OR:$<CONFIG:DEBUG>,$<CONFIG:CHECKED>>,$<STREQUAL:$<TARGET_PROPERTY:TYPE>,SHARED_LIBRARY>>:${CLR_SANITIZE_LINK_OPTIONS};-Wl,--gc-sections>")
+      set(CMAKE_SHARED_LINKER_FLAGS_DEBUG "${CMAKE_SHARED_LINKER_FLAGS_DEBUG} ${CLR_SANITIZE_LINK_FLAGS} -Wl,--gc-sections")
+      set(CMAKE_SHARED_LINKER_FLAGS_CHECKED "${CMAKE_SHARED_LINKER_FLAGS_CHECKED} ${CLR_SANITIZE_LINK_FLAGS} -Wl,--gc-sections")
     endif ()
   endif(UPPERCASE_CMAKE_BUILD_TYPE STREQUAL DEBUG OR UPPERCASE_CMAKE_BUILD_TYPE STREQUAL CHECKED)
 endif(MSVC)

Then sanitizer is just enable to work:

...
SUMMARY: AddressSanitizer: heap-buffer-overflow /home/zhaixiang/runtime/src/coreclr/src/pal/src/thread/thread.cpp:1669:17 in CorUnix::InternalSetThreadDescription(CorUnix::CPalThread*, void*, char16_t const*)
Shadow bytes around the buggy address:
  0x0c047fff8820: fa fa 00 00 fa fa 04 fa fa fa 00 00 fa fa 00 00
  0x0c047fff8830: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
  0x0c047fff8840: fa fa 00 00 fa fa 00 fa fa fa 00 00 fa fa 00 00
  0x0c047fff8850: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
  0x0c047fff8860: fa fa 00 00 fa fa 00 00 fa fa 04 fa fa fa 00 fa
=>0x0c047fff8870: fa fa 00[07]fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8880: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8890: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff88a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff88b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff88c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==18491==ABORTING

Thanks, Leslie Zhai

xiangzhai commented 4 years ago

\cc @steveharter

xiangzhai commented 4 years ago

BTW -fsanitize=vptr should be disabled:

...
../../dlls/mscordac/libmscordaccore.so: undefined reference to `typeinfo for LCGMethodResolver'
../../dlls/mscordac/libmscordaccore.so: undefined reference to `typeinfo for DebuggerRCThread'
clang-8: error: linker command failed with exit code 1 (use -v to see invocation)
src/debug/createdump/CMakeFiles/createdump.dir/build.make:107: recipe for target 'src/debug/createdump/createdump' failed
...

Because:

-fsanitize=vptr: Use of an object whose vptr indicates that it is of the wrong dynamic type, or that its lifetime has not begun or has ended. Incompatible with -fno-rtti

https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html

xiangzhai commented 4 years ago

\cc @jkoritzinsky

jkoritzinsky commented 4 years ago

I believe the fix here is to change the add_compile_definitions in the following line to add_compile_options:https://github.com/dotnet/runtime/blob/f72a3217606068a3ab4a67e7255a034726d8c3b3/src/coreclr/configurecompiler.cmake#L342

xiangzhai commented 4 years ago

Hi @jkoritzinsky

Thanks for your response!

Could you reproduce the issue? Workaround for enablesanitizers.sh:

diff --git a/src/coreclr/enablesanitizers.sh b/src/coreclr/enablesanitizers.sh
index aedb95d..6429692 100755
--- a/src/coreclr/enablesanitizers.sh
+++ b/src/coreclr/enablesanitizers.sh
@@ -73,6 +73,11 @@ else
                 __ClangMinorVersion=9
                 __ExportSymbolizerPath=0
                 ;;
+            clang8.0)
+                __ClangMajorVersion=8
+                __ClangMinorVersion=0
+                __ExportSymbolizerPath=0
+                ;;
             *)
                 echo "Unknown arg: $i"
                 return 1

Thanks, Leslie Zhai

steveharter commented 4 years ago

@xiangzhai the sanitizer script is not used internally and thus needs to be manually updated every so often due to new releases of clang.

However, can you explain you use of sanitizers? Currently there is too much noise coming from the sanitizer output (that needs to be addressed by whitelisting or by changing the code), so I'm curious on the usefulness of it for your needs.

xiangzhai commented 4 years ago

Hi @steveharter

Thanks for your response!

However, can you explain you use of sanitizers?

I am the author of Clang Static Analyzer Checkers. And I try to contribute to dynamic analysis for MIPS64 :)

Currently there is too much noise coming from the sanitizer output (that needs to be addressed by whitelisting or by changing the code)

Yes, there are lots of False Positive!

Thanks, Leslie Zhai