[llvm] 907baee - Revert "Make -fsanitize=scudo use scudo_standalone. Delete check-scudo."

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 23 07:13:27 PST 2022


Author: Hans Wennborg
Date: 2022-11-23T16:07:07+01:00
New Revision: 907baeec49bfbe9e76498634a9418e1dc6c973d9

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

LOG: Revert "Make -fsanitize=scudo use scudo_standalone. Delete check-scudo."

It broke the build, see comments on code review.

> Leaves the implementation and tests files in-place for right now, but
> deletes the ability to build the old sanitizer-common based scudo. This
> has been on life-support for a long time, and the newer scudo_standalone
> is much better supported and maintained.
>
> Also patches up some GWP-ASan wording, primarily related to the fact
> that -fsanitize=scudo now is scudo_standalone, and therefore the way to
> reference the GWP-ASan options through the environment variable has
> changed.
>
> Future follow-up patches will delete the original scudo, and migrate all
> its tests over to be part of the scudo_standalone test suite.
>
> Reviewed By: vitalybuka
>
> Differential Revision: https://reviews.llvm.org/D138157

This reverts commit ab1a5991fe765d71c0f3262f25726d6b4d66a545.

Added: 
    compiler-rt/test/scudo/CMakeLists.txt

Modified: 
    clang/lib/Driver/ToolChains/CommonArgs.cpp
    clang/test/Driver/fuchsia.c
    clang/test/Driver/sanitizer-ld.c
    compiler-rt/cmake/config-ix.cmake
    compiler-rt/lib/CMakeLists.txt
    compiler-rt/test/CMakeLists.txt
    llvm/docs/GwpAsan.rst

Removed: 
    


################################################################################
diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index d45721b0bd3b6..30fca3a4c0590 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -941,7 +941,10 @@ collectSanitizerRuntimes(const ToolChain &TC, const ArgList &Args,
         SharedRuntimes.push_back("ubsan_standalone");
     }
     if (SanArgs.needsScudoRt() && SanArgs.linkRuntimes()) {
-      SharedRuntimes.push_back("scudo_standalone");
+      if (SanArgs.requiresMinimalRuntime())
+        SharedRuntimes.push_back("scudo_minimal");
+      else
+        SharedRuntimes.push_back("scudo");
     }
     if (SanArgs.needsTsanRt() && SanArgs.linkRuntimes())
       SharedRuntimes.push_back("tsan");
@@ -1038,9 +1041,15 @@ collectSanitizerRuntimes(const ToolChain &TC, const ArgList &Args,
     RequiredSymbols.push_back("__sanitizer_stats_register");
   }
   if (!SanArgs.needsSharedRt() && SanArgs.needsScudoRt() && SanArgs.linkRuntimes()) {
-    StaticRuntimes.push_back("scudo_standalone");
-    if (SanArgs.linkCXXRuntimes())
-      StaticRuntimes.push_back("scudo_standalone_cxx");
+    if (SanArgs.requiresMinimalRuntime()) {
+      StaticRuntimes.push_back("scudo_minimal");
+      if (SanArgs.linkCXXRuntimes())
+        StaticRuntimes.push_back("scudo_cxx_minimal");
+    } else {
+      StaticRuntimes.push_back("scudo");
+      if (SanArgs.linkCXXRuntimes())
+        StaticRuntimes.push_back("scudo_cxx");
+    }
   }
 }
 

diff  --git a/clang/test/Driver/fuchsia.c b/clang/test/Driver/fuchsia.c
index 339fd92309796..099a88c2e4e36 100644
--- a/clang/test/Driver/fuchsia.c
+++ b/clang/test/Driver/fuchsia.c
@@ -183,7 +183,7 @@
 // CHECK-SCUDO-X86: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-SCUDO-X86: "-fsanitize=safe-stack,scudo"
 // CHECK-SCUDO-X86: "-pie"
-// CHECK-SCUDO-X86: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}fuchsia{{/|\\\\}}libclang_rt.scudo_standalone-x86_64.so"
+// CHECK-SCUDO-X86: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}x86_64-unknown-fuchsia{{/|\\\\}}libclang_rt.scudo.so"
 
 // RUN: %clang -### %s --target=aarch64-unknown-fuchsia \
 // RUN:     -fsanitize=scudo 2>&1 \
@@ -193,7 +193,7 @@
 // CHECK-SCUDO-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-SCUDO-AARCH64: "-fsanitize=shadow-call-stack,scudo"
 // CHECK-SCUDO-AARCH64: "-pie"
-// CHECK-SCUDO-AARCH64: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}fuchsia{{/|\\\\}}libclang_rt.scudo_standalone-aarch64.so"
+// CHECK-SCUDO-AARCH64: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}aarch64-unknown-fuchsia{{/|\\\\}}libclang_rt.scudo.so"
 
 // RUN: %clang -### %s --target=x86_64-unknown-fuchsia \
 // RUN:     -fsanitize=scudo -fPIC -shared 2>&1 \
@@ -202,7 +202,7 @@
 // RUN:     | FileCheck %s -check-prefix=CHECK-SCUDO-SHARED
 // CHECK-SCUDO-SHARED: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-SCUDO-SHARED: "-fsanitize=safe-stack,scudo"
-// CHECK-SCUDO-SHARED: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}fuchsia{{/|\\\\}}libclang_rt.scudo_standalone-x86_64.so"
+// CHECK-SCUDO-SHARED: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}x86_64-unknown-fuchsia{{/|\\\\}}libclang_rt.scudo.so"
 
 // RUN: %clang -### %s --target=aarch64-unknown-fuchsia \
 // RUN:     -fsanitize=leak 2>&1 \

diff  --git a/clang/test/Driver/sanitizer-ld.c b/clang/test/Driver/sanitizer-ld.c
index e58a6e51547ff..1c9b6d783be3d 100644
--- a/clang/test/Driver/sanitizer-ld.c
+++ b/clang/test/Driver/sanitizer-ld.c
@@ -852,12 +852,23 @@
 // RUN:   | FileCheck --check-prefix=CHECK-SCUDO-LINUX %s
 // CHECK-SCUDO-LINUX: "{{.*}}ld{{(.exe)?}}"
 // CHECK-SCUDO-LINUX: "-pie"
-// CHECK-SCUDO-LINUX: "--whole-archive" "{{.*}}libclang_rt.scudo_standalone-i386.a" "--no-whole-archive"
+// CHECK-SCUDO-LINUX: "--whole-archive" "{{.*}}libclang_rt.scudo-i386.a" "--no-whole-archive"
 // CHECK-SCUDO-LINUX-NOT: "-lstdc++"
 // CHECK-SCUDO-LINUX: "-lpthread"
 // CHECK-SCUDO-LINUX: "-ldl"
 // CHECK-SCUDO-LINUX: "-lresolv"
 
+// RUN: %clang -fsanitize=scudo -fsanitize-minimal-runtime -### %s 2>&1 \
+// RUN:     --target=i386-unknown-linux -fuse-ld=ld \
+// RUN:     -resource-dir=%S/Inputs/resource_dir \
+// RUN:     --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SCUDO-MINIMAL-LINUX %s
+// CHECK-SCUDO-MINIMAL-LINUX: "{{.*}}ld{{(.exe)?}}"
+// CHECK-SCUDO-MINIMAL-LINUX: "-pie"
+// CHECK-SCUDO-MINIMAL-LINUX: "--whole-archive" "{{.*}}libclang_rt.scudo_minimal-i386.a" "--no-whole-archive"
+// CHECK-SCUDO-MINIMAL-LINUX: "-lpthread"
+// CHECK-SCUDO-MINIMAL-LINUX: "-lresolv"
+
 // RUN: %clang -### %s -o %t.so -shared 2>&1 \
 // RUN:     --target=i386-unknown-linux -fuse-ld=ld -fsanitize=scudo -shared-libsan \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \
@@ -866,8 +877,8 @@
 //
 // CHECK-SCUDO-SHARED-LINUX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-SCUDO-SHARED-LINUX-NOT: "-lc"
-// CHECK-SCUDO-SHARED-LINUX-NOT: libclang_rt.scudo_standalone-i386.a"
-// CHECK-SCUDO-SHARED-LINUX: libclang_rt.scudo_standalone-i386.so"
+// CHECK-SCUDO-SHARED-LINUX-NOT: libclang_rt.scudo-i386.a"
+// CHECK-SCUDO-SHARED-LINUX: libclang_rt.scudo-i386.so"
 // CHECK-SCUDO-SHARED-LINUX-NOT: "-lpthread"
 // CHECK-SCUDO-SHARED-LINUX-NOT: "-lrt"
 // CHECK-SCUDO-SHARED-LINUX-NOT: "-ldl"
@@ -885,7 +896,7 @@
 // CHECK-SCUDO-ANDROID: "-pie"
 // CHECK-SCUDO-ANDROID-NOT: "-lpthread"
 // CHECK-SCUDO-ANDROID-NOT: "-lresolv"
-// CHECK-SCUDO-ANDROID: libclang_rt.scudo_standalone-arm-android.so"
+// CHECK-SCUDO-ANDROID: libclang_rt.scudo-arm-android.so"
 // CHECK-SCUDO-ANDROID-NOT: "-lpthread"
 // CHECK-SCUDO-ANDROID-NOT: "-lresolv"
 
@@ -896,7 +907,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-SCUDO-ANDROID-STATIC %s
 // CHECK-SCUDO-ANDROID-STATIC: "{{(.*[^.0-9A-Z_a-z])?}}ld.lld{{(.exe)?}}"
 // CHECK-SCUDO-ANDROID-STATIC: "-pie"
-// CHECK-SCUDO-ANDROID-STATIC: "--whole-archive" "{{.*}}libclang_rt.scudo_standalone-arm-android.a" "--no-whole-archive"
+// CHECK-SCUDO-ANDROID-STATIC: "--whole-archive" "{{.*}}libclang_rt.scudo-arm-android.a" "--no-whole-archive"
 // CHECK-SCUDO-ANDROID-STATIC-NOT: "-lstdc++"
 // CHECK-SCUDO-ANDROID-STATIC-NOT: "-lpthread"
 // CHECK-SCUDO-ANDROID-STATIC-NOT: "-lrt"

diff  --git a/compiler-rt/cmake/config-ix.cmake b/compiler-rt/cmake/config-ix.cmake
index e16236fbb1b2d..f6190ee60e3c3 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -619,6 +619,9 @@ if(APPLE)
   list_intersect(CFI_SUPPORTED_ARCH
     ALL_CFI_SUPPORTED_ARCH
     SANITIZER_COMMON_SUPPORTED_ARCH)
+  list_intersect(SCUDO_SUPPORTED_ARCH
+    ALL_SCUDO_SUPPORTED_ARCH
+    SANITIZER_COMMON_SUPPORTED_ARCH)
   list_intersect(SCUDO_STANDALONE_SUPPORTED_ARCH
     ALL_SCUDO_STANDALONE_SUPPORTED_ARCH
     SANITIZER_COMMON_SUPPORTED_ARCH)
@@ -658,6 +661,7 @@ else()
   filter_available_targets(SAFESTACK_SUPPORTED_ARCH
     ${ALL_SAFESTACK_SUPPORTED_ARCH})
   filter_available_targets(CFI_SUPPORTED_ARCH ${ALL_CFI_SUPPORTED_ARCH})
+  filter_available_targets(SCUDO_SUPPORTED_ARCH ${ALL_SCUDO_SUPPORTED_ARCH})
   filter_available_targets(SCUDO_STANDALONE_SUPPORTED_ARCH ${ALL_SCUDO_STANDALONE_SUPPORTED_ARCH})
   filter_available_targets(XRAY_SUPPORTED_ARCH ${ALL_XRAY_SUPPORTED_ARCH})
   filter_available_targets(SHADOWCALLSTACK_SUPPORTED_ARCH
@@ -697,7 +701,7 @@ if(COMPILER_RT_SUPPORTED_ARCH)
 endif()
 message(STATUS "Compiler-RT supported architectures: ${COMPILER_RT_SUPPORTED_ARCH}")
 
-set(ALL_SANITIZERS asan;dfsan;msan;hwasan;tsan;safestack;cfi;ubsan_minimal;gwp_asan)
+set(ALL_SANITIZERS asan;dfsan;msan;hwasan;tsan;safestack;cfi;scudo;ubsan_minimal;gwp_asan)
 set(COMPILER_RT_SANITIZERS_TO_BUILD all CACHE STRING
     "sanitizers to build if supported on the target (all;${ALL_SANITIZERS})")
 list_replace(COMPILER_RT_SANITIZERS_TO_BUILD all "${ALL_SANITIZERS}")
@@ -826,6 +830,13 @@ else()
   set(COMPILER_RT_HAS_SCUDO_STANDALONE FALSE)
 endif()
 
+if (COMPILER_RT_HAS_SANITIZER_COMMON AND SCUDO_SUPPORTED_ARCH AND
+    OS_NAME MATCHES "Linux|Fuchsia")
+  set(COMPILER_RT_HAS_SCUDO TRUE)
+else()
+  set(COMPILER_RT_HAS_SCUDO FALSE)
+endif()
+
 if (COMPILER_RT_HAS_SANITIZER_COMMON AND XRAY_SUPPORTED_ARCH AND
     OS_NAME MATCHES "Darwin|Linux|FreeBSD|NetBSD|Fuchsia")
   set(COMPILER_RT_HAS_XRAY TRUE)

diff  --git a/compiler-rt/lib/CMakeLists.txt b/compiler-rt/lib/CMakeLists.txt
index 6b55b5f0f7705..18eed2446dc61 100644
--- a/compiler-rt/lib/CMakeLists.txt
+++ b/compiler-rt/lib/CMakeLists.txt
@@ -24,13 +24,12 @@ endif()
 function(compiler_rt_build_runtime runtime)
   string(TOUPPER ${runtime} runtime_uppercase)
   if(COMPILER_RT_HAS_${runtime_uppercase})
+    add_subdirectory(${runtime})
     if(${runtime} STREQUAL tsan)
       add_subdirectory(tsan/dd)
     endif()
-    if(${runtime} STREQUAL scudo_standalone)
+    if(${runtime} STREQUAL scudo)
       add_subdirectory(scudo/standalone)
-    else()
-      add_subdirectory(${runtime})
     endif()
   endif()
 endfunction()
@@ -49,8 +48,6 @@ if(COMPILER_RT_BUILD_SANITIZERS)
   foreach(sanitizer ${COMPILER_RT_SANITIZERS_TO_BUILD})
     compiler_rt_build_runtime(${sanitizer})
   endforeach()
-
-  compiler_rt_build_runtime(scudo_standalone)
 endif()
 
 if(COMPILER_RT_BUILD_PROFILE AND COMPILER_RT_HAS_PROFILE)

diff  --git a/compiler-rt/test/CMakeLists.txt b/compiler-rt/test/CMakeLists.txt
index 42eccb962bb95..e905b606c51c6 100644
--- a/compiler-rt/test/CMakeLists.txt
+++ b/compiler-rt/test/CMakeLists.txt
@@ -19,8 +19,6 @@ configure_compiler_rt_lit_site_cfg(
 # BlocksRuntime (and most of builtins) testsuites are not yet ported to lit.
 # add_subdirectory(BlocksRuntime)
 
-add_subdirectory(scudo/standalone)
-
 set(SANITIZER_COMMON_LIT_TEST_DEPS)
 
 if(COMPILER_RT_BUILD_PROFILE AND COMPILER_RT_HAS_PROFILE)

diff  --git a/compiler-rt/test/scudo/CMakeLists.txt b/compiler-rt/test/scudo/CMakeLists.txt
new file mode 100644
index 0000000000000..3a1982add9c49
--- /dev/null
+++ b/compiler-rt/test/scudo/CMakeLists.txt
@@ -0,0 +1,35 @@
+set(SCUDO_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(SCUDO_LIT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
+set(SCUDO_TESTSUITES)
+
+set(SCUDO_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
+if(NOT COMPILER_RT_STANDALONE_BUILD)
+  list(APPEND SCUDO_TEST_DEPS scudo)
+endif()
+
+configure_lit_site_cfg(
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
+  )
+
+set(SCUDO_TEST_ARCH ${SCUDO_SUPPORTED_ARCH})
+foreach(arch ${SCUDO_TEST_ARCH})
+  set(SCUDO_TEST_TARGET_ARCH ${arch})
+  string(TOLOWER "-${arch}" SCUDO_TEST_CONFIG_SUFFIX)
+  get_test_cc_for_arch(${arch} SCUDO_TEST_TARGET_CC SCUDO_TEST_TARGET_CFLAGS)
+  string(TOUPPER ${arch} ARCH_UPPER_CASE)
+  set(CONFIG_NAME ${ARCH_UPPER_CASE}${OS_NAME}Config)
+
+  configure_lit_site_cfg(
+    ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
+    ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg.py)
+  list(APPEND SCUDO_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
+endforeach()
+
+add_subdirectory(standalone)
+
+add_lit_testsuite(check-scudo "Running the Scudo Hardened Allocator tests"
+  ${SCUDO_TESTSUITES}
+  DEPENDS ${SCUDO_TEST_DEPS})
+set_target_properties(check-scudo PROPERTIES FOLDER "Compiler-RT Misc")

diff  --git a/llvm/docs/GwpAsan.rst b/llvm/docs/GwpAsan.rst
index 293f75d71ac3f..647c2a038a5a9 100644
--- a/llvm/docs/GwpAsan.rst
+++ b/llvm/docs/GwpAsan.rst
@@ -143,10 +143,9 @@ several aspects of GWP-ASan to be configured through the following methods:
   default visibility. This will override the compile time define;
 
 - Depending on allocator support (Scudo has support for this mechanism): Through
-  an environment variable, containing the options string to be parsed. In Scudo,
-  this is through `SCUDO_OPTIONS=GWP_ASAN_${OPTION_NAME}=${VALUE}` (e.g.
-  `SCUDO_OPTIONS=GWP_ASAN_SampleRate=100`). Options defined this way will
-  override any definition made through ``__gwp_asan_default_options``.
+  the environment variable ``GWP_ASAN_OPTIONS``, containing the options string
+  to be parsed. Options defined this way will override any definition made
+  through ``__gwp_asan_default_options``.
 
 The options string follows a syntax similar to ASan, where distinct options
 can be assigned in the same string, separated by colons.
@@ -217,9 +216,9 @@ and provide us a detailed error report:
 
 .. code:: console
 
-  $ clang++ -fsanitize=scudo -g buggy_code.cpp
-  $ for i in `seq 1 500`; do
-      SCUDO_OPTIONS="GWP_ASAN_SampleRate=100" ./a.out > /dev/null;
+  $ clang++ -fsanitize=scudo -std=c++17 -g buggy_code.cpp
+  $ for i in `seq 1 200`; do
+      GWP_ASAN_OPTIONS="SampleRate=100" ./a.out > /dev/null;
     done
   |
   | *** GWP-ASan detected a memory error ***


        


More information about the llvm-commits mailing list