From a8574e5e11673ddc43fadaa55d3fd724cb2bcea9 Mon Sep 17 00:00:00 2001 From: Johan Engelen Date: Fri, 24 Jul 2020 14:55:18 +0200 Subject: [PATCH 1/4] Copy LLVM's ThreadSanitizer (TSan) library to LDC's libraries like we do for ASan, and set the correct linker flags. TSan is not supported on Windows. --- CMakeLists.txt | 2 + driver/linker-gcc.cpp | 46 ++++++++++++++++++-- tests/sanitizers/deflake.bash | 29 +++++++++++++ tests/sanitizers/lit.local.cfg | 7 +++ tests/sanitizers/tsan_noerror.d | 14 ++++++ tests/sanitizers/tsan_tiny_race.d | 61 +++++++++++++++++++++++++++ tests/sanitizers/tsan_tiny_race_TLS.d | 60 ++++++++++++++++++++++++++ 7 files changed, 216 insertions(+), 3 deletions(-) create mode 100755 tests/sanitizers/deflake.bash create mode 100644 tests/sanitizers/tsan_noerror.d create mode 100644 tests/sanitizers/tsan_tiny_race.d create mode 100644 tests/sanitizers/tsan_tiny_race_TLS.d diff --git a/CMakeLists.txt b/CMakeLists.txt index b527cbf565e..eab9f99cb21 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -763,6 +763,7 @@ if (LDC_INSTALL_LLVM_RUNTIME_LIBS) if(APPLE) copy_compilerrt_lib("darwin/libclang_rt.asan_osx_dynamic.dylib" "libldc_rt.asan.dylib" TRUE) + copy_compilerrt_lib("darwin/libclang_rt.tsan_osx_dynamic.dylib" "libldc_rt.tsan.dylib" TRUE) copy_compilerrt_lib("darwin/libclang_rt.osx.a" "libldc_rt.builtins.a" FALSE) copy_compilerrt_lib("darwin/libclang_rt.profile_osx.a" "libldc_rt.profile.a" FALSE) copy_compilerrt_lib("darwin/libclang_rt.fuzzer_osx.a" "libldc_rt.fuzzer.a" FALSE) @@ -777,6 +778,7 @@ if (LDC_INSTALL_LLVM_RUNTIME_LIBS) set(LDC_INSTALL_LLVM_RUNTIME_LIBS_ARCH "x86_64" CACHE STRING "Non-Mac Posix: architecture used as libname suffix for the compiler-rt source libraries, e.g., 'aarch64'.") copy_compilerrt_lib("${LDC_INSTALL_LLVM_RUNTIME_LIBS_OS}/libclang_rt.asan-${LDC_INSTALL_LLVM_RUNTIME_LIBS_ARCH}.a" "libldc_rt.asan.a" FALSE) + copy_compilerrt_lib("${LDC_INSTALL_LLVM_RUNTIME_LIBS_OS}/libclang_rt.tsan-${LDC_INSTALL_LLVM_RUNTIME_LIBS_ARCH}.a" "libldc_rt.tsan.a" FALSE) copy_compilerrt_lib("${LDC_INSTALL_LLVM_RUNTIME_LIBS_OS}/libclang_rt.builtins-${LDC_INSTALL_LLVM_RUNTIME_LIBS_ARCH}.a" "libldc_rt.builtins.a" FALSE) copy_compilerrt_lib("${LDC_INSTALL_LLVM_RUNTIME_LIBS_OS}/libclang_rt.profile-${LDC_INSTALL_LLVM_RUNTIME_LIBS_ARCH}.a" "libldc_rt.profile.a" FALSE) copy_compilerrt_lib("${LDC_INSTALL_LLVM_RUNTIME_LIBS_OS}/libclang_rt.xray-${LDC_INSTALL_LLVM_RUNTIME_LIBS_ARCH}.a" "libldc_rt.xray.a" FALSE) diff --git a/driver/linker-gcc.cpp b/driver/linker-gcc.cpp index 79a20a8cff1..1269aac1aca 100644 --- a/driver/linker-gcc.cpp +++ b/driver/linker-gcc.cpp @@ -57,6 +57,7 @@ class ArgsBuilder { private: virtual void addSanitizers(const llvm::Triple &triple); virtual void addASanLinkFlags(const llvm::Triple &triple); + virtual void addTSanLinkFlags(const llvm::Triple &triple); virtual void addFuzzLinkFlags(const llvm::Triple &triple); virtual void addCppStdlibLinkFlags(const llvm::Triple &triple); virtual void addProfileRuntimeLinkFlags(const llvm::Triple &triple); @@ -322,6 +323,47 @@ void ArgsBuilder::addASanLinkFlags(const llvm::Triple &triple) { args.push_back("-fsanitize=address"); } +void ArgsBuilder::addTSanLinkFlags(const llvm::Triple &triple) { + // Examples: "libclang_rt.tsan-x86_64.a" or "libclang_rt.tsan-arm.a" and + // "libclang_rt.tsan-x86_64.so" + + // TODO: let user choose to link with shared lib. + // In case of shared TSan, I think we also need to statically link with + // libclang_rt.tsan-preinit-.a on Linux. On Darwin, the only option is + // to use the shared library. + bool linkSharedTSan = triple.isOSDarwin(); + const auto searchPaths = + getFullCompilerRTLibPathCandidates("tsan", triple, linkSharedTSan); + + for (const auto &filepath : searchPaths) { + IF_LOG Logger::println("Searching TSan lib: %s", filepath.c_str()); + + if (llvm::sys::fs::exists(filepath) && + !llvm::sys::fs::is_directory(filepath)) { + IF_LOG Logger::println("Found, linking with %s", filepath.c_str()); + args.push_back(filepath); + + if (linkSharedTSan) { + // Add @executable_path to rpath to support having the shared lib copied + // with the executable. + args.push_back("-rpath"); + args.push_back("@executable_path"); + + // Add the path to the resource dir to rpath to support using the shared + // lib from the default location without copying. + args.push_back("-rpath"); + args.push_back(std::string(llvm::sys::path::parent_path(filepath))); + } + + return; + } + } + + // When we reach here, we did not find the TSan library. + // Fallback, requires Clang. + args.push_back("-fsanitize=thread"); +} + // Adds all required link flags for -fsanitize=fuzzer when libFuzzer library is // found. void ArgsBuilder::addFuzzLinkFlags(const llvm::Triple &triple) { @@ -459,10 +501,8 @@ void ArgsBuilder::addSanitizers(const llvm::Triple &triple) { args.push_back("-fsanitize=memory"); } - // TODO: instead of this, we should link with our own sanitizer libraries - // because LDC's LLVM version could be different from the system clang. if (opts::isSanitizerEnabled(opts::ThreadSanitizer)) { - args.push_back("-fsanitize=thread"); + addTSanLinkFlags(triple); } } diff --git a/tests/sanitizers/deflake.bash b/tests/sanitizers/deflake.bash new file mode 100755 index 00000000000..db48feecb05 --- /dev/null +++ b/tests/sanitizers/deflake.bash @@ -0,0 +1,29 @@ +#!/usr/bin/env bash + +# Copied from LLVM's testsuite: compiler-rt/test/tsan + +# This script is used to deflake inherently flaky tsan tests. +# It is invoked from lit tests as: +# %deflake $THRESHOLD mybinary +# which is then substituted by lit to: +# $(dirname %s)/deflake.bash $THRESHOLD mybinary +# - When TSAN_TEST_DEFLAKE_THRESHOLD is defined to a positive integer value, +# THRESHOLD will be the defined value. +# - When TSAN_TEST_DEFLAKE_THRESHOLD is not defined, THRESHOLD will be 10. +# The script runs the target program up to $THRESHOLD times, +# until it fails (i.e. produces a race report). + +THRESHOLD="${1}" +shift + +# Early exit if $THRESHOLD is not a non-negative integer +[[ "${THRESHOLD}" =~ ^[0-9]+$ ]] || exit 1 + +while (( THRESHOLD-- )); do + OUT=`$@ 2>&1` + if [[ $? != 0 ]]; then + echo "$OUT" + exit 0 + fi +done +exit 1 diff --git a/tests/sanitizers/lit.local.cfg b/tests/sanitizers/lit.local.cfg index 21c559927b6..3626cb42cc6 100644 --- a/tests/sanitizers/lit.local.cfg +++ b/tests/sanitizers/lit.local.cfg @@ -11,6 +11,10 @@ if (platform.system() == 'Darwin') or (platform.system() == 'Linux'): if m is not None: config.available_features.add('ASan') continue + m = re.match('.*tsan.*', file) + if m is not None: + config.available_features.add('TSan') + continue m = re.match('.*(F|f)uzzer.*', file) if m is not None: config.available_features.add('Fuzzer') @@ -25,3 +29,6 @@ if 'ASan' in config.available_features: config.substitutions.append(('%env_asan_opts=', 'env ASAN_OPTIONS=' + default_asan_options + ':')) +# Add the %deflake substitution, to help with flaky tests. +# Usage: "%deflake ", runs a maximum of times until a failure occurs. +config.substitutions.append( ("%deflake", os.path.join(os.path.dirname(__file__), "deflake.bash"))) diff --git a/tests/sanitizers/tsan_noerror.d b/tests/sanitizers/tsan_noerror.d new file mode 100644 index 00000000000..1c21b0fb9d7 --- /dev/null +++ b/tests/sanitizers/tsan_noerror.d @@ -0,0 +1,14 @@ +// Test that a simple program passes ThreadSanitizer without error + +// REQUIRES: TSan + +// XFAIL: * +// Druntime does not yet work with ThreadSanitizer. +// See Github issue 3519 (https://github.com/ldc-developers/ldc/issues/3519) + +// RUN: %ldc -fsanitize=thread %s -of=%t%exe +// RUN: %t%exe + +void main() +{ +} diff --git a/tests/sanitizers/tsan_tiny_race.d b/tests/sanitizers/tsan_tiny_race.d new file mode 100644 index 00000000000..ba8d99ee381 --- /dev/null +++ b/tests/sanitizers/tsan_tiny_race.d @@ -0,0 +1,61 @@ +// Test that ThreadSanitizer+LDC works on a very basic testcase. +// Note that -betterC is used, to avoid relying on druntime for this test. + +// REQUIRES: TSan + +// RUN: %ldc -betterC -g -fsanitize=thread %s -of=%t%exe +// RUN: %deflake 10 %t%exe | FileCheck %s + +// CHECK: WARNING: ThreadSanitizer: data race + +import core.sys.posix.pthread; + +shared int global; + +extern(C) +void *thread1(void *x) { + barrier_wait(&barrier); +// CHECK-DAG: thread1{{.*}}[[@LINE+1]] + global = 42; + return x; +} + +extern(C) +int main() { + barrier_init(&barrier, 2); + pthread_t t; + pthread_create(&t, null, &thread1, null); +// CHECK-DAG: main{{.*}}[[@LINE+1]] + global = 43; + barrier_wait(&barrier); + pthread_join(t, null); + return global; +} + +//---------------------------------------------------------------------------- +// Code to facilitate thread synchronization to make this test deterministic. +// See LLVM: compiler-rt/test/tsan/test.h + +// TSan-invisible barrier. +// Tests use it to establish necessary execution order in a way that does not +// interfere with tsan (does not establish synchronization between threads). +alias invisible_barrier_t = __c_ulonglong; +import core.stdc.config; +alias __c_unsigned = uint; +// Default instance of the barrier, but a test can declare more manually. +__gshared invisible_barrier_t barrier; + +extern (C) { + // These functions reside inside the tsan library. + void __tsan_testonly_barrier_init(invisible_barrier_t *barrier, __c_unsigned count); + void __tsan_testonly_barrier_wait(invisible_barrier_t *barrier); +} + +void barrier_init(invisible_barrier_t *barrier, __c_unsigned count) { + __tsan_testonly_barrier_init(barrier, count); +} + +void barrier_wait(invisible_barrier_t *barrier) { + __tsan_testonly_barrier_wait(barrier); +} +//---------------------------------------------------------------------------- diff --git a/tests/sanitizers/tsan_tiny_race_TLS.d b/tests/sanitizers/tsan_tiny_race_TLS.d new file mode 100644 index 00000000000..d2dbeee05af --- /dev/null +++ b/tests/sanitizers/tsan_tiny_race_TLS.d @@ -0,0 +1,60 @@ +// Test that ThreadSanitizer+LDC works on a very basic testcase. +// Note that -betterC is used, to avoid relying on druntime for this test. + +// REQUIRES: TSan + +// RUN: %ldc -betterC -g -fsanitize=thread %s -of=%t%exe +// RUN: %deflake 10 %t%exe | FileCheck %s + +// CHECK: WARNING: ThreadSanitizer: data race + +import core.sys.posix.pthread; + +extern(C) +void *thread1(void *x) { + barrier_wait(&barrier); +// CHECK-DAG: thread1{{.*}}[[@LINE+1]] + *cast(int*)x = 42; + return x; +} + +extern(C) +int main() { + int tls_variable; + barrier_init(&barrier, 2); + pthread_t t; + pthread_create(&t, null, &thread1, &tls_variable); +// CHECK-DAG: main{{.*}}[[@LINE+1]] + tls_variable = 43; + barrier_wait(&barrier); + pthread_join(t, null); + return 0; +} + +//---------------------------------------------------------------------------- +// Code to facilitate thread synchronization to make this test deterministic. +// See LLVM: compiler-rt/test/tsan/test.h + +// TSan-invisible barrier. +// Tests use it to establish necessary execution order in a way that does not +// interfere with tsan (does not establish synchronization between threads). +alias invisible_barrier_t = __c_ulonglong; +import core.stdc.config; +alias __c_unsigned = uint; +// Default instance of the barrier, but a test can declare more manually. +__gshared invisible_barrier_t barrier; + +extern (C) { + // These functions reside inside the tsan library. + void __tsan_testonly_barrier_init(invisible_barrier_t *barrier, __c_unsigned count); + void __tsan_testonly_barrier_wait(invisible_barrier_t *barrier); +} + +void barrier_init(invisible_barrier_t *barrier, __c_unsigned count) { + __tsan_testonly_barrier_init(barrier, count); +} + +void barrier_wait(invisible_barrier_t *barrier) { + __tsan_testonly_barrier_wait(barrier); +} +//---------------------------------------------------------------------------- From f65d79dad3a8a88a64b117d838441f0373960bc3 Mon Sep 17 00:00:00 2001 From: Johan Engelen Date: Thu, 30 Jul 2020 15:34:23 +0200 Subject: [PATCH 2/4] Require at least LLVM 8 for executable TSan test cases and increase max deflake repetition count to 20. --- tests/sanitizers/tsan_tiny_race.d | 3 ++- tests/sanitizers/tsan_tiny_race_TLS.d | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/sanitizers/tsan_tiny_race.d b/tests/sanitizers/tsan_tiny_race.d index ba8d99ee381..7bd0326af43 100644 --- a/tests/sanitizers/tsan_tiny_race.d +++ b/tests/sanitizers/tsan_tiny_race.d @@ -2,9 +2,10 @@ // Note that -betterC is used, to avoid relying on druntime for this test. // REQUIRES: TSan +// REQUIRES: atleast_llvm800 // RUN: %ldc -betterC -g -fsanitize=thread %s -of=%t%exe -// RUN: %deflake 10 %t%exe | FileCheck %s +// RUN: %deflake 20 %t%exe | FileCheck %s // CHECK: WARNING: ThreadSanitizer: data race diff --git a/tests/sanitizers/tsan_tiny_race_TLS.d b/tests/sanitizers/tsan_tiny_race_TLS.d index d2dbeee05af..0fbd616911f 100644 --- a/tests/sanitizers/tsan_tiny_race_TLS.d +++ b/tests/sanitizers/tsan_tiny_race_TLS.d @@ -2,9 +2,10 @@ // Note that -betterC is used, to avoid relying on druntime for this test. // REQUIRES: TSan +// REQUIRES: atleast_llvm800 // RUN: %ldc -betterC -g -fsanitize=thread %s -of=%t%exe -// RUN: %deflake 10 %t%exe | FileCheck %s +// RUN: %deflake 20 %t%exe | FileCheck %s // CHECK: WARNING: ThreadSanitizer: data race From c2cc27367cda9468e21813454c313b199f275119 Mon Sep 17 00:00:00 2001 From: Johan Engelen Date: Fri, 31 Jul 2020 17:11:03 +0200 Subject: [PATCH 3/4] Remove code duplication for ASan and TSan linker flags. --- driver/linker-gcc.cpp | 68 +++++++++---------------------------------- 1 file changed, 14 insertions(+), 54 deletions(-) diff --git a/driver/linker-gcc.cpp b/driver/linker-gcc.cpp index 1269aac1aca..f5021edb16d 100644 --- a/driver/linker-gcc.cpp +++ b/driver/linker-gcc.cpp @@ -56,8 +56,9 @@ class ArgsBuilder { private: virtual void addSanitizers(const llvm::Triple &triple); - virtual void addASanLinkFlags(const llvm::Triple &triple); - virtual void addTSanLinkFlags(const llvm::Triple &triple); + virtual void addSanitizerLinkFlags(const llvm::Triple &triple, + const llvm::StringRef sanitizerName, + const llvm::StringRef fallbackFlag); virtual void addFuzzLinkFlags(const llvm::Triple &triple); virtual void addCppStdlibLinkFlags(const llvm::Triple &triple); virtual void addProfileRuntimeLinkFlags(const llvm::Triple &triple); @@ -280,63 +281,22 @@ getFullCompilerRTLibPathCandidates(llvm::StringRef baseName, return r; } -void ArgsBuilder::addASanLinkFlags(const llvm::Triple &triple) { - // Examples: "libclang_rt.asan-x86_64.a" or "libclang_rt.asan-arm.a" and - // "libclang_rt.asan-x86_64.so" - - // TODO: let user choose to link with shared lib. - // In case of shared ASan, I think we also need to statically link with - // libclang_rt.asan-preinit-.a on Linux. On Darwin, the only option is - // to use the shared library. - bool linkSharedASan = triple.isOSDarwin(); - const auto searchPaths = - getFullCompilerRTLibPathCandidates("asan", triple, linkSharedASan); - - for (const auto &filepath : searchPaths) { - IF_LOG Logger::println("Searching ASan lib: %s", filepath.c_str()); - - if (llvm::sys::fs::exists(filepath) && - !llvm::sys::fs::is_directory(filepath)) { - IF_LOG Logger::println("Found, linking with %s", filepath.c_str()); - args.push_back(filepath); - - if (linkSharedASan) { - // Add @executable_path to rpath to support having the shared lib copied - // with the executable. - args.push_back("-rpath"); - args.push_back("@executable_path"); - - // Add the path to the resource dir to rpath to support using the shared - // lib from the default location without copying. - args.push_back("-rpath"); - args.push_back(std::string(llvm::sys::path::parent_path(filepath))); - } - - return; - } - } - - // When we reach here, we did not find the ASan library. - // Fallback, requires Clang. The asan library contains a versioned symbol - // name and a linker error will happen when the LDC-LLVM and Clang-LLVM - // versions don't match. - args.push_back("-fsanitize=address"); -} - -void ArgsBuilder::addTSanLinkFlags(const llvm::Triple &triple) { +void ArgsBuilder::addSanitizerLinkFlags(const llvm::Triple &triple, + const llvm::StringRef sanitizerName, + const llvm::StringRef fallbackFlag) { // Examples: "libclang_rt.tsan-x86_64.a" or "libclang_rt.tsan-arm.a" and // "libclang_rt.tsan-x86_64.so" // TODO: let user choose to link with shared lib. - // In case of shared TSan, I think we also need to statically link with - // libclang_rt.tsan-preinit-.a on Linux. On Darwin, the only option is + // In case of shared ASan, I think we also need to statically link with + // libclang_rt.asan-preinit-.a on Linux. On Darwin, the only option is // to use the shared library. bool linkSharedTSan = triple.isOSDarwin(); const auto searchPaths = - getFullCompilerRTLibPathCandidates("tsan", triple, linkSharedTSan); + getFullCompilerRTLibPathCandidates(sanitizerName, triple, linkSharedTSan); for (const auto &filepath : searchPaths) { - IF_LOG Logger::println("Searching TSan lib: %s", filepath.c_str()); + IF_LOG Logger::println("Searching sanitizer lib: %s", filepath.c_str()); if (llvm::sys::fs::exists(filepath) && !llvm::sys::fs::is_directory(filepath)) { @@ -359,9 +319,9 @@ void ArgsBuilder::addTSanLinkFlags(const llvm::Triple &triple) { } } - // When we reach here, we did not find the TSan library. + // When we reach here, we did not find the sanitizer library. // Fallback, requires Clang. - args.push_back("-fsanitize=thread"); + args.push_back(fallbackFlag); } // Adds all required link flags for -fsanitize=fuzzer when libFuzzer library is @@ -488,7 +448,7 @@ void ArgsBuilder::addProfileRuntimeLinkFlags(const llvm::Triple &triple) { void ArgsBuilder::addSanitizers(const llvm::Triple &triple) { if (opts::isSanitizerEnabled(opts::AddressSanitizer)) { - addASanLinkFlags(triple); + addSanitizerLinkFlags(triple, "asan", "-fsanitize=address"); } if (opts::isSanitizerEnabled(opts::FuzzSanitizer)) { @@ -502,7 +462,7 @@ void ArgsBuilder::addSanitizers(const llvm::Triple &triple) { } if (opts::isSanitizerEnabled(opts::ThreadSanitizer)) { - addTSanLinkFlags(triple); + addSanitizerLinkFlags(triple, "tsan", "-fsanitize=thread"); } } From 073246f8cfc1f0dc3ecb62335044f1369d93585c Mon Sep 17 00:00:00 2001 From: Johan Engelen Date: Sat, 1 Aug 2020 17:48:58 +0200 Subject: [PATCH 4/4] Fix up variable name --- driver/linker-gcc.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/driver/linker-gcc.cpp b/driver/linker-gcc.cpp index f5021edb16d..20c69aabb0f 100644 --- a/driver/linker-gcc.cpp +++ b/driver/linker-gcc.cpp @@ -291,9 +291,9 @@ void ArgsBuilder::addSanitizerLinkFlags(const llvm::Triple &triple, // In case of shared ASan, I think we also need to statically link with // libclang_rt.asan-preinit-.a on Linux. On Darwin, the only option is // to use the shared library. - bool linkSharedTSan = triple.isOSDarwin(); + bool linkSharedLibrary = triple.isOSDarwin(); const auto searchPaths = - getFullCompilerRTLibPathCandidates(sanitizerName, triple, linkSharedTSan); + getFullCompilerRTLibPathCandidates(sanitizerName, triple, linkSharedLibrary); for (const auto &filepath : searchPaths) { IF_LOG Logger::println("Searching sanitizer lib: %s", filepath.c_str()); @@ -303,7 +303,7 @@ void ArgsBuilder::addSanitizerLinkFlags(const llvm::Triple &triple, IF_LOG Logger::println("Found, linking with %s", filepath.c_str()); args.push_back(filepath); - if (linkSharedTSan) { + if (linkSharedLibrary) { // Add @executable_path to rpath to support having the shared lib copied // with the executable. args.push_back("-rpath");