Skip to content

Commit

Permalink
[Scudo] Make -fsanitize=scudo use standalone. Migrate tests.
Browse files Browse the repository at this point in the history
This patch moves -fsanitize=scudo to link the standalone scudo library,
rather than the original compiler-rt based library. This is one of the
major remaining roadblocks to deleting the compiler-rt based scudo,
which should not be used any more. The standalone Scudo is better in
pretty much every way and is much more suitable for production usage.

As well as patching the litmus tests for checking that the
scudo_standalone lib is linked instead of the scudo lib, this patch also
ports all the scudo lit tests to run under scudo standalone.

This patch also adds a feature to scudo standalone that was under test
in the original scudo - that arguments passed to an aligned operator new
were checked that the alignment was a power of two.

Some lit tests could not be migrated, due to the following issues:
 1. Features that aren't supported in scudo standalone, like the rss
 limit.
 2. Different quarantine implementation where the test needs some more
 thought.
 3. Small bugs in scudo standalone that should probably be fixed, like
 the Secondary allocator having a full page on the LHS of an allocation
 that only contains the chunk header, so underflows by <= a page aren't
 caught.
 4. Slight differences in behaviour that's technically correct, like
 'realloc(malloc(1), 0)' returns nullptr in standalone, but a real
 pointer in old scudo.
 5. Some tests that might be migratable, but not easily.

Tests that are obviously not applicable to scudo standalone (like
testing that no sanitizer symbols made it into the DSO) have been
deleted.

After this patch, the remaining work is:
 1. Update the Scudo documentation. The flags have changed, etc.
 2. Delete the old version of scudo.
 3. Patch up the tests in lit-unmigrated, or fix Scudo standalone.

Reviewed By: cryptoad, vitalybuka

Differential Revision: https://reviews.llvm.org/D102543
  • Loading branch information
hctim committed May 26, 2021
1 parent d63d662 commit 6911114
Show file tree
Hide file tree
Showing 37 changed files with 171 additions and 297 deletions.
17 changes: 4 additions & 13 deletions clang/lib/Driver/ToolChains/CommonArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -809,10 +809,7 @@ collectSanitizerRuntimes(const ToolChain &TC, const ArgList &Args,
SharedRuntimes.push_back("ubsan_standalone");
}
if (SanArgs.needsScudoRt() && SanArgs.linkRuntimes()) {
if (SanArgs.requiresMinimalRuntime())
SharedRuntimes.push_back("scudo_minimal");
else
SharedRuntimes.push_back("scudo");
SharedRuntimes.push_back("scudo_standalone");
}
if (SanArgs.needsTsanRt() && SanArgs.linkRuntimes())
SharedRuntimes.push_back("tsan");
Expand Down Expand Up @@ -903,15 +900,9 @@ collectSanitizerRuntimes(const ToolChain &TC, const ArgList &Args,
RequiredSymbols.push_back("__sanitizer_stats_register");
}
if (!SanArgs.needsSharedRt() && SanArgs.needsScudoRt() && SanArgs.linkRuntimes()) {
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");
}
StaticRuntimes.push_back("scudo_standalone");
if (SanArgs.linkCXXRuntimes())
StaticRuntimes.push_back("scudo_standalone_cxx");
}
}

Expand Down
6 changes: 3 additions & 3 deletions clang/test/Driver/fuchsia.c
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,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{{/|\\\\}}x86_64-unknown-fuchsia{{/|\\\\}}libclang_rt.scudo.so"
// CHECK-SCUDO-X86: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}x86_64-unknown-fuchsia{{/|\\\\}}libclang_rt.scudo_standalone.so"

// RUN: %clang %s -### --target=aarch64-unknown-fuchsia \
// RUN: -fsanitize=scudo 2>&1 \
Expand All @@ -163,7 +163,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{{/|\\\\}}aarch64-unknown-fuchsia{{/|\\\\}}libclang_rt.scudo.so"
// CHECK-SCUDO-AARCH64: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}aarch64-unknown-fuchsia{{/|\\\\}}libclang_rt.scudo_standalone.so"

// RUN: %clang %s -### --target=x86_64-unknown-fuchsia \
// RUN: -fsanitize=scudo -fPIC -shared 2>&1 \
Expand All @@ -172,7 +172,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{{/|\\\\}}x86_64-unknown-fuchsia{{/|\\\\}}libclang_rt.scudo.so"
// CHECK-SCUDO-SHARED: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}x86_64-unknown-fuchsia{{/|\\\\}}libclang_rt.scudo_standalone.so"

// RUN: %clang %s -### --target=aarch64-unknown-fuchsia \
// RUN: -fsanitize=leak 2>&1 \
Expand Down
20 changes: 5 additions & 15 deletions clang/test/Driver/sanitizer-ld.c
Original file line number Diff line number Diff line change
Expand Up @@ -773,21 +773,11 @@
// 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-i386.a" "--no-whole-archive"
// CHECK-SCUDO-LINUX: "--whole-archive" "{{.*}}libclang_rt.scudo_standalone-i386.a" "--no-whole-archive"
// CHECK-SCUDO-LINUX-NOT: "-lstdc++"
// CHECK-SCUDO-LINUX: "-lpthread"
// CHECK-SCUDO-LINUX: "-ldl"

// RUN: %clang -fsanitize=scudo -fsanitize-minimal-runtime %s -### -o %t.o 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"

// RUN: %clang -no-canonical-prefixes %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 \
Expand All @@ -796,8 +786,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-i386.a"
// CHECK-SCUDO-SHARED-LINUX: libclang_rt.scudo-i386.so"
// 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: "-lpthread"
// CHECK-SCUDO-SHARED-LINUX-NOT: "-lrt"
// CHECK-SCUDO-SHARED-LINUX-NOT: "-ldl"
Expand All @@ -813,7 +803,7 @@
// CHECK-SCUDO-ANDROID-NOT: "-lc"
// CHECK-SCUDO-ANDROID: "-pie"
// CHECK-SCUDO-ANDROID-NOT: "-lpthread"
// CHECK-SCUDO-ANDROID: libclang_rt.scudo-arm-android.so"
// CHECK-SCUDO-ANDROID: libclang_rt.scudo_standalone-arm-android.so"
// CHECK-SCUDO-ANDROID-NOT: "-lpthread"

// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
Expand All @@ -823,7 +813,7 @@
// RUN: | FileCheck --check-prefix=CHECK-SCUDO-ANDROID-STATIC %s
// CHECK-SCUDO-ANDROID-STATIC: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
// CHECK-SCUDO-ANDROID-STATIC: "-pie"
// CHECK-SCUDO-ANDROID-STATIC: "--whole-archive" "{{.*}}libclang_rt.scudo-arm-android.a" "--no-whole-archive"
// CHECK-SCUDO-ANDROID-STATIC: "--whole-archive" "{{.*}}libclang_rt.scudo_standalone-arm-android.a" "--no-whole-archive"
// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lstdc++"
// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lpthread"
// CHECK-SCUDO-ANDROID-STATIC-NOT: "-lrt"
Expand Down
25 changes: 25 additions & 0 deletions compiler-rt/lib/scudo/standalone/wrappers_cpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,27 @@ struct nothrow_t {};
enum class align_val_t : size_t {};
} // namespace std

// It's not valid for a non-throwing non-noreturn operator new to return
// nullptr. In these cases, just terminate.
#define CHECK_ALIGN_NORETURN(align) \
do { \
scudo::uptr alignment = static_cast<scudo::uptr>(align); \
if (UNLIKELY(!scudo::isPowerOfTwo(alignment))) { \
scudo::reportAlignmentNotPowerOfTwo(alignment); \
} \
} while (0)

#define CHECK_ALIGN(align) \
do { \
scudo::uptr alignment = static_cast<scudo::uptr>(align); \
if (UNLIKELY(!scudo::isPowerOfTwo(alignment))) { \
if (Allocator.canReturnNull()) { \
return nullptr; \
} \
scudo::reportAlignmentNotPowerOfTwo(alignment); \
} \
} while (0)

INTERFACE WEAK void *operator new(size_t size) {
return Allocator.allocate(size, scudo::Chunk::Origin::New);
}
Expand All @@ -38,20 +59,24 @@ INTERFACE WEAK void *operator new[](size_t size,
return Allocator.allocate(size, scudo::Chunk::Origin::NewArray);
}
INTERFACE WEAK void *operator new(size_t size, std::align_val_t align) {
CHECK_ALIGN_NORETURN(align);
return Allocator.allocate(size, scudo::Chunk::Origin::New,
static_cast<scudo::uptr>(align));
}
INTERFACE WEAK void *operator new[](size_t size, std::align_val_t align) {
CHECK_ALIGN_NORETURN(align);
return Allocator.allocate(size, scudo::Chunk::Origin::NewArray,
static_cast<scudo::uptr>(align));
}
INTERFACE WEAK void *operator new(size_t size, std::align_val_t align,
std::nothrow_t const &) NOEXCEPT {
CHECK_ALIGN(align);
return Allocator.allocate(size, scudo::Chunk::Origin::New,
static_cast<scudo::uptr>(align));
}
INTERFACE WEAK void *operator new[](size_t size, std::align_val_t align,
std::nothrow_t const &) NOEXCEPT {
CHECK_ALIGN(align);
return Allocator.allocate(size, scudo::Chunk::Origin::NewArray,
static_cast<scudo::uptr>(align));
}
Expand Down
34 changes: 0 additions & 34 deletions compiler-rt/test/scudo/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,35 +1 @@
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")
91 changes: 0 additions & 91 deletions compiler-rt/test/scudo/interface.cpp

This file was deleted.

64 changes: 0 additions & 64 deletions compiler-rt/test/scudo/lit.cfg.py

This file was deleted.

11 changes: 0 additions & 11 deletions compiler-rt/test/scudo/lit.site.cfg.py.in

This file was deleted.

18 changes: 18 additions & 0 deletions compiler-rt/test/scudo/standalone/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
set(SCUDO_STANDALONE_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
set(SCUDO_STANDALONE_LIT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
set(SCUDO_STANDALONE_TESTSUITES)
set(SCUDO_STANDALONE_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS} scudo_standalone)

foreach(arch ${SCUDO_STANDALONE_SUPPORTED_ARCH})
set(SCUDO_STANDALONE_TEST_TARGET_ARCH ${arch})
string(TOLOWER "-${arch}" SCUDO_STANDALONE_TEST_CONFIG_SUFFIX)
get_target_flags_for_arch(${arch} SCUDO_STANDALONE_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_STANDALONE_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
endforeach()

if(COMPILER_RT_INCLUDE_TESTS AND COMPILER_RT_HAS_SCUDO_STANDALONE)
configure_lit_site_cfg(
${CMAKE_CURRENT_SOURCE_DIR}/unit/lit.site.cfg.py.in
Expand Down
Loading

0 comments on commit 6911114

Please sign in to comment.