From 0bfa6efb0ee5b292455503d47fd1394f02226d5b Mon Sep 17 00:00:00 2001 From: Denis Borisov Date: Thu, 22 Aug 2024 11:36:13 +0300 Subject: [PATCH 1/2] Add ThreadSanitizer support - restore SEASTAR_NO_EXCEPTION_HACK behavior - detect TSan and its fiber support - do not handle SIGSEGV, use increased stack size for TSan as for ASan - use TSan fiber api in context switch code - add sanitize-thread build mode with TSan and SEASTAR_NO_EXCEPTION_HACK --- CMakeLists.txt | 76 +++++++++++++++++-- README.md | 13 ++-- cmake/FindSanitizers.cmake | 26 +++++++ .../code_tests/ThreadSanitizer_fiber_test.cc | 14 ++++ configure.py | 2 +- include/seastar/core/thread_impl.hh | 11 ++- include/seastar/util/std-compat.hh | 4 + seastar_cmake.py | 2 +- src/core/exception_hacks.cc | 12 ++- src/core/posix.cc | 2 +- src/core/reactor.cc | 8 +- src/core/thread.cc | 74 ++++++++++++++++-- src/testing/entry_point.cc | 4 +- 13 files changed, 214 insertions(+), 34 deletions(-) create mode 100644 cmake/code_tests/ThreadSanitizer_fiber_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 7aa90ce31bf..c242c35047c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -182,11 +182,45 @@ mark_as_advanced ( CMAKE_EXE_LINKER_FLAGS_SANITIZE CMAKE_SHARED_LINKER_FLAGS_SANITIZE) +set (CMAKE_CXX_FLAGS_SANITIZETHREAD + "-Os -g" + CACHE + STRING + "Flags used by the C++ compiler during sanitize-thread builds." + FORCE) + +set (CMAKE_C_FLAGS_SANITIZETHREAD + "-Os -g" + CACHE + STRING + "Flags used by the C compiler during sanitize-thread builds." + FORCE) + +set (CMAKE_EXE_LINKER_FLAGS_SANITIZETHREAD + "" + CACHE + STRING + "Flags used for linking binaries during sanitize-thread builds." + FORCE) + +set (CMAKE_SHARED_LINKER_FLAGS_SANITIZETHREAD + "" + CACHE + STRING + "Flags used by the shared libraries linker during sanitize-thread builds." + FORCE) + +mark_as_advanced ( + CMAKE_CXX_FLAGS_SANITIZETHREAD + CMAKE_C_FLAGS_SANITIZETHREAD + CMAKE_EXE_LINKER_FLAGS_SANITIZETHREAD + CMAKE_SHARED_LINKER_FLAGS_SANITIZETHREAD) + set (CMAKE_BUILD_TYPE "${CMAKE_BUILD_TYPE}" CACHE STRING - "Choose the type of build, options are: None Debug Release RelWithDebInfo MinSizeRel Dev Sanitize." + "Choose the type of build, options are: None Debug Release RelWithDebInfo Dev Sanitize SanitizeThread." FORCE) if (NOT CMAKE_BUILD_TYPE) @@ -356,7 +390,7 @@ endif () set (Seastar_TEST_ENVIRONMENT - "ASAN_OPTIONS=${Seastar_ASAN_OPTIONS};UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1;BOOST_TEST_CATCH_SYSTEM_ERRORS=no" + "ASAN_OPTIONS=${Seastar_ASAN_OPTIONS};UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1;TSAN_OPTIONS=halt_on_error=1;BOOST_TEST_CATCH_SYSTEM_ERRORS=no" CACHE STRING "Environment variables for running tests") @@ -377,6 +411,12 @@ set (Seastar_SANITIZE STRING "Enable ASAN and UBSAN. Can be ON, OFF or DEFAULT (which enables it for Debug and Sanitize)") +set (Seastar_SANITIZE_THREAD + "DEFAULT" + CACHE + STRING + "Enable TSAN. Can be ON, OFF or DEFAULT (which enables it for SanitizeThread)") + set (Seastar_DEBUG_SHARED_PTR "DEFAULT" CACHE @@ -888,6 +928,26 @@ if (condition) $<${condition}:Sanitizers::undefined_behavior>) endif () +tri_state_option (${Seastar_SANITIZE_THREAD} + DEFAULT_BUILD_TYPES "SanitizeThread" + CONDITION condition) +if (condition) + if (NOT ThreadSanitizer_FOUND) + message (FATAL_ERROR "Thread sanitizer not found!") + endif () + set (Seastar_Sanitizers_OPTIONS ${ThreadSanitizer_COMPILER_OPTIONS}) + target_link_libraries (seastar + PUBLIC + $<${condition}:Sanitizers::thread>) + target_compile_definitions(seastar + PUBLIC + SEASTAR_NO_EXCEPTION_HACK) + if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # Disabled because tsan doesn't support std::atomic_thread_fence + list (APPEND Seastar_PRIVATE_CXX_FLAGS -Wno-error=tsan) + endif () +endif () + # We only need valgrind to find uninitialized memory uses, so disable # the leak sanitizer. # To test with valgrind run "ctest -T memcheck" @@ -963,6 +1023,10 @@ if (Sanitizers_FIBER_SUPPORT) list (APPEND Seastar_PRIVATE_COMPILE_DEFINITIONS SEASTAR_HAVE_ASAN_FIBER_SUPPORT) endif () +if (ThreadSanitizer_FIBER_SUPPORT) + list (APPEND Seastar_PRIVATE_COMPILE_DEFINITIONS SEASTAR_HAVE_TSAN_FIBER_SUPPORT) +endif () + if (Seastar_ALLOC_PAGE_SIZE) target_compile_definitions (seastar PUBLIC SEASTAR_OVERRIDE_ALLOCATOR_PAGE_SIZE=${Seastar_ALLOC_PAGE_SIZE}) @@ -1079,11 +1143,11 @@ foreach (definition SEASTAR_SHUFFLE_TASK_QUEUE) target_compile_definitions (seastar PUBLIC - $<$,Debug;Sanitize>:${definition}>) + $<$,Debug;Sanitize;SanitizeThread>:${definition}>) endforeach () tri_state_option (${Seastar_DEBUG_SHARED_PTR} - DEFAULT_BUILD_TYPES "Debug" "Sanitize" + DEFAULT_BUILD_TYPES "Debug" "Sanitize" "SanitizeThread" CONDITION condition) if (condition) target_compile_definitions (seastar @@ -1092,7 +1156,7 @@ if (condition) endif () tri_state_option (${Seastar_DEBUG_SHARED_PTR} - DEFAULT_BUILD_TYPES "Debug" "Sanitize" + DEFAULT_BUILD_TYPES "Debug" "Sanitize" "SanitizeThread" CONDITION condition) if (condition) target_compile_definitions (seastar @@ -1103,7 +1167,7 @@ endif () include (CheckLibc) tri_state_option (${Seastar_STACK_GUARDS} - DEFAULT_BUILD_TYPES "Debug" "Sanitize" "Dev" + DEFAULT_BUILD_TYPES "Debug" "Sanitize" "SanitizeThread" "Dev" CONDITION condition) if (condition) # check for -fstack-clash-protection together with -Werror, because diff --git a/README.md b/README.md index e6570ec1235..dae07231448 100644 --- a/README.md +++ b/README.md @@ -56,12 +56,13 @@ Build modes The configure.py script is a wrapper around cmake. The --mode argument maps to CMAKE_BUILD_TYPE, and supports the following modes -| | CMake mode | Debug info | Optimi­zations | Sanitizers | Allocator | Checks | Use for | -| -------- | ------------------- | ---------- | ------------------ |------------- | --------- | -------- | -------------------------------------- | -| debug | `Debug` | Yes | `-O0` | ASAN, UBSAN | System | All | gdb | -| release | `RelWithDebInfo` | Yes | `-O3` | None | Seastar | Asserts | production | -| dev | `Dev` (Custom) | No | `-O1` | None | Seastar | Asserts | build and test cycle | -| sanitize | `Sanitize` (Custom) | Yes | `-Os` | ASAN, UBSAN | System | All | second level of tests, track down bugs | +| | CMake mode | Debug info | Optimi­zations | Sanitizers | Allocator | Checks | Use for | +| --------------- | ------------------------- | ---------- | ------------------ | ----------- | --------- | ------- | -------------------------------------- | +| debug | `Debug` | Yes | `-O0` | ASAN, UBSAN | System | All | gdb | +| release | `RelWithDebInfo` | Yes | `-O3` | None | Seastar | Asserts | production | +| dev | `Dev` (Custom) | No | `-O1` | None | Seastar | Asserts | build and test cycle | +| sanitize | `Sanitize` (Custom) | Yes | `-Os` | ASAN, UBSAN | System | All | second level of tests, track down bugs | +| sanitize-thread | `SanitizeThread` (Custom) | Yes | `-Os` | TSAN | System | All | race detection | Note that seastar is more sensitive to allocators and optimizations than usual. A quick rule of the thumb of the relative performances is that diff --git a/cmake/FindSanitizers.cmake b/cmake/FindSanitizers.cmake index 316f88ea2e0..5c8f3269a00 100644 --- a/cmake/FindSanitizers.cmake +++ b/cmake/FindSanitizers.cmake @@ -37,6 +37,13 @@ if (Sanitizers_UNDEFINED_BEHAVIOR_FOUND) set (Sanitizers_UNDEFINED_BEHAVIOR_COMPILER_OPTIONS "-fsanitize=undefined;-fno-sanitize=vptr") endif () +set (CMAKE_REQUIRED_FLAGS -fsanitize=thread) +check_cxx_source_compiles ("int main() {}" Sanitizers_THREAD_FOUND) + +if (Sanitizers_THREAD_FOUND) + set (ThreadSanitizer_COMPILER_OPTIONS -fsanitize=thread) +endif () + set (Sanitizers_COMPILER_OPTIONS ${Sanitizers_ADDRESS_COMPILER_OPTIONS} ${Sanitizers_UNDEFINED_BEHAVIOR_COMPILER_OPTIONS}) @@ -45,6 +52,10 @@ file (READ ${CMAKE_CURRENT_LIST_DIR}/code_tests/Sanitizers_fiber_test.cc _saniti set (CMAKE_REQUIRED_FLAGS ${Sanitizers_COMPILER_OPTIONS}) check_cxx_source_compiles ("${_sanitizers_fiber_test_code}" Sanitizers_FIBER_SUPPORT) +file (READ ${CMAKE_CURRENT_LIST_DIR}/code_tests/ThreadSanitizer_fiber_test.cc _thread_sanitizer_fiber_test_code) +set (CMAKE_REQUIRED_FLAGS ${ThreadSanitizer_COMPILER_OPTIONS}) +check_cxx_source_compiles ("${_thread_sanitizer_fiber_test_code}" ThreadSanitizer_FIBER_SUPPORT) + include (FindPackageHandleStandardArgs) find_package_handle_standard_args (Sanitizers @@ -52,6 +63,10 @@ find_package_handle_standard_args (Sanitizers Sanitizers_ADDRESS_COMPILER_OPTIONS Sanitizers_UNDEFINED_BEHAVIOR_COMPILER_OPTIONS) +find_package_handle_standard_args (ThreadSanitizer + REQUIRED_VARS + ThreadSanitizer_COMPILER_OPTIONS) + if (Sanitizers_FOUND) if (NOT (TARGET Sanitizers::address)) add_library (Sanitizers::address INTERFACE IMPORTED) @@ -71,3 +86,14 @@ if (Sanitizers_FOUND) INTERFACE_LINK_LIBRARIES "${Sanitizers_UNDEFINED_BEHAVIOR_COMPILER_OPTIONS}") endif () endif () + +if (ThreadSanitizer_FOUND) + if (NOT (TARGET Sanitizers::thread)) + add_library (Sanitizers::thread INTERFACE IMPORTED) + + set_target_properties (Sanitizers::thread + PROPERTIES + INTERFACE_COMPILE_OPTIONS ${ThreadSanitizer_COMPILER_OPTIONS} + INTERFACE_LINK_LIBRARIES ${ThreadSanitizer_COMPILER_OPTIONS}) + endif () +endif () diff --git a/cmake/code_tests/ThreadSanitizer_fiber_test.cc b/cmake/code_tests/ThreadSanitizer_fiber_test.cc new file mode 100644 index 00000000000..f043ee87ef0 --- /dev/null +++ b/cmake/code_tests/ThreadSanitizer_fiber_test.cc @@ -0,0 +1,14 @@ +extern "C" { + void* __tsan_get_current_fiber(); + void* __tsan_create_fiber(unsigned flags); + void __tsan_destroy_fiber(void* fiber); + void __tsan_switch_to_fiber(void* fiber, unsigned flags); +} + +int main() { + void* new_fiber = __tsan_create_fiber(0); + void* main_fiber = __tsan_get_current_fiber(); + __tsan_switch_to_fiber(new_fiber, 0); + __tsan_switch_to_fiber(main_fiber, 0); + __tsan_destroy_fiber(new_fiber); +} diff --git a/configure.py b/configure.py index c1fba65ad13..4b76f41d80e 100755 --- a/configure.py +++ b/configure.py @@ -169,7 +169,7 @@ def identify_best_standard(cpp_standards, compiler): # For convenience. tr = seastar_cmake.translate_arg -MODE_TO_CMAKE_BUILD_TYPE = {'release': 'RelWithDebInfo', 'debug': 'Debug', 'dev': 'Dev', 'sanitize': 'Sanitize' } +MODE_TO_CMAKE_BUILD_TYPE = {'release': 'RelWithDebInfo', 'debug': 'Debug', 'dev': 'Dev', 'sanitize': 'Sanitize', 'sanitize-thread' : 'SanitizeThread'} def configure_mode(mode): diff --git a/include/seastar/core/thread_impl.hh b/include/seastar/core/thread_impl.hh index eb34d0e0bcc..bb81f266452 100644 --- a/include/seastar/core/thread_impl.hh +++ b/include/seastar/core/thread_impl.hh @@ -36,11 +36,17 @@ class thread_context; class scheduling_group; struct jmp_buf_link { -#ifdef SEASTAR_ASAN_ENABLED +#if defined(SEASTAR_ASAN_ENABLED) || defined(SEASTAR_TSAN_ENABLED) ucontext_t context; +#ifdef SEASTAR_ASAN_ENABLED void* fake_stack = nullptr; const void* stack_bottom; size_t stack_size; +#endif +#ifdef SEASTAR_TSAN_ENABLED + void* fiber = nullptr; + bool destroy_tsan_fiber = false; +#endif #else jmp_buf jmpbuf; #endif @@ -52,6 +58,9 @@ public: void switch_out(); void initial_switch_in_completed(); void final_switch_out(); +#ifdef SEASTAR_TSAN_ENABLED + ~jmp_buf_link(); +#endif }; extern thread_local jmp_buf_link* g_current_context; diff --git a/include/seastar/util/std-compat.hh b/include/seastar/util/std-compat.hh index 7c58cb68143..cfc2385d2d5 100644 --- a/include/seastar/util/std-compat.hh +++ b/include/seastar/util/std-compat.hh @@ -47,6 +47,10 @@ namespace std::pmr { #define SEASTAR_ASAN_ENABLED #endif +#if __has_feature(thread_sanitizer) || defined(__SANITIZE_THREAD__) +#define SEASTAR_TSAN_ENABLED +#endif + #if __has_include() #include #endif diff --git a/seastar_cmake.py b/seastar_cmake.py index 105c01c50c0..3bd510268b2 100644 --- a/seastar_cmake.py +++ b/seastar_cmake.py @@ -17,7 +17,7 @@ import os -SUPPORTED_MODES = ['release', 'debug', 'dev', 'sanitize'] +SUPPORTED_MODES = ['release', 'debug', 'dev', 'sanitize', 'sanitize-thread'] ROOT_PATH = os.path.realpath(os.path.dirname(__file__)) diff --git a/src/core/exception_hacks.cc b/src/core/exception_hacks.cc index 9ac7ff0d10e..75df739a704 100644 --- a/src/core/exception_hacks.cc +++ b/src/core/exception_hacks.cc @@ -71,6 +71,8 @@ module seastar; #endif namespace seastar { + +#ifndef SEASTAR_NO_EXCEPTION_HACK using dl_iterate_fn = int (*) (int (*callback) (struct dl_phdr_info *info, size_t size, void *data), void *data); [[gnu::no_sanitize_address]] @@ -102,6 +104,9 @@ void init_phdr_cache() { return 0; }, nullptr); } +#else +void init_phdr_cache() {} +#endif void internal::increase_thrown_exceptions_counter() noexcept { seastar::engine()._cxx_exceptions++; @@ -124,6 +129,7 @@ void log_exception_trace() noexcept {} } +#ifndef SEASTAR_NO_EXCEPTION_HACK extern "C" [[gnu::visibility("default")]] [[gnu::used]] @@ -146,6 +152,7 @@ int dl_iterate_phdr(int (*callback) (struct dl_phdr_info *info, size_t size, voi } return r; } +#endif #ifndef NO_EXCEPTION_INTERCEPT extern "C" @@ -153,11 +160,8 @@ extern "C" [[gnu::used]] int _Unwind_RaiseException(struct ::_Unwind_Exception *h) { using throw_fn = int (*)(void *); - static throw_fn org = nullptr; + static throw_fn org = (throw_fn)dlsym (RTLD_NEXT, "_Unwind_RaiseException"); - if (!org) { - org = (throw_fn)dlsym (RTLD_NEXT, "_Unwind_RaiseException"); - } if (seastar::local_engine) { seastar::internal::increase_thrown_exceptions_counter(); seastar::log_exception_trace(); diff --git a/src/core/posix.cc b/src/core/posix.cc index cf2e95f8d96..8f1f04e66d0 100644 --- a/src/core/posix.cc +++ b/src/core/posix.cc @@ -103,7 +103,7 @@ posix_thread::posix_thread(attr a, std::function func) throw std::system_error(r, std::system_category()); } -#ifndef SEASTAR_ASAN_ENABLED +#if !defined(SEASTAR_ASAN_ENABLED) && !defined(SEASTAR_TSAN_ENABLED) auto stack_size = a._stack_size.size; if (!stack_size) { stack_size = 2 << 20; diff --git a/src/core/reactor.cc b/src/core/reactor.cc index c47d688a399..b415baa5e57 100644 --- a/src/core/reactor.cc +++ b/src/core/reactor.cc @@ -867,7 +867,7 @@ static void print_with_backtrace(const char* cause, bool oneline = false) noexce print_with_backtrace(buf, oneline); } -#ifndef SEASTAR_ASAN_ENABLED +#if !defined(SEASTAR_ASAN_ENABLED) && !defined(SEASTAR_TSAN_ENABLED) // Installs signal handler stack for current thread. // The stack remains installed as long as the returned object is kept alive. // When it goes out of scope the previous handler is restored. @@ -893,7 +893,7 @@ static decltype(auto) install_signal_handler_stack() { } #else // SIGSTKSZ is too small when using asan. We also don't need to -// handle SIGSEGV ourselves when using asan, so just don't install +// handle SIGSEGV ourselves when using asan/tsan, so just don't install // a signal handler stack. auto install_signal_handler_stack() { struct nothing { ~nothing() {} }; @@ -3960,8 +3960,8 @@ static void sigabrt_action() noexcept { reraise_signal(SIGABRT); } -// We don't need to handle SIGSEGV when asan is enabled. -#ifdef SEASTAR_ASAN_ENABLED +// We don't need to handle SIGSEGV when asan/tsan is enabled. +#if defined(SEASTAR_ASAN_ENABLED) || defined(SEASTAR_TSAN_ENABLED) template<> void install_oneshot_signal_handler() { (void)sigsegv_action; diff --git a/src/core/thread.cc b/src/core/thread.cc index eb9b6d6cc31..c25b300c78a 100644 --- a/src/core/thread.cc +++ b/src/core/thread.cc @@ -24,7 +24,7 @@ module; #endif #include -#ifndef SEASTAR_ASAN_ENABLED +#if !defined(SEASTAR_ASAN_ENABLED) && !defined(SEASTAR_TSAN_ENABLED) #include #endif #include @@ -48,11 +48,11 @@ namespace seastar { thread_local jmp_buf_link g_unthreaded_context; thread_local jmp_buf_link* g_current_context; -#ifdef SEASTAR_ASAN_ENABLED +#if defined(SEASTAR_ASAN_ENABLED) || defined(SEASTAR_TSAN_ENABLED) namespace { -#ifdef SEASTAR_HAVE_ASAN_FIBER_SUPPORT +#if defined(SEASTAR_ASAN_ENABLED) && defined(SEASTAR_HAVE_ASAN_FIBER_SUPPORT) // ASan provides two functions as a means of informing it that user context // switch has happened. First __sanitizer_start_switch_fiber() needs to be // called with a place to store the fake stack pointer and the new stack @@ -60,16 +60,29 @@ namespace { // __sanitizer_finish_switch_fiber() needs to be called with a pointer to the // current context fake stack and a place to store stack information of the // previous ucontext. - extern "C" { void __sanitizer_start_switch_fiber(void** fake_stack_save, const void* stack_bottom, size_t stack_size); void __sanitizer_finish_switch_fiber(void* fake_stack_save, const void** stack_bottom_old, size_t* stack_size_old); } -#else +#elif defined(SEASTAR_ASAN_ENABLED) static inline void __sanitizer_start_switch_fiber(...) { } static inline void __sanitizer_finish_switch_fiber(...) { } #endif +#if defined(SEASTAR_TSAN_ENABLED) && defined(SEASTAR_HAVE_TSAN_FIBER_SUPPORT) +extern "C" { +void* __tsan_get_current_fiber(); +void* __tsan_create_fiber(unsigned flags); +void __tsan_destroy_fiber(void* fiber); +void __tsan_switch_to_fiber(void* fiber, unsigned flags); +} +#elif defined(SEASTAR_TSAN_ENABLED) +static inline void* __tsan_get_current_fiber(...) { return nullptr; } +static inline void* __tsan_create_fiber(...) { return nullptr; } +static inline void __tsan_destroy_fiber(...) { } +static inline void __tsan_switch_to_fiber(...) { } +#endif + thread_local jmp_buf_link* g_previous_context; } @@ -79,10 +92,20 @@ void jmp_buf_link::initial_switch_in(ucontext_t* initial_context, const void* st auto prev = std::exchange(g_current_context, this); link = prev; g_previous_context = prev; +#ifdef SEASTAR_ASAN_ENABLED __sanitizer_start_switch_fiber(&prev->fake_stack, stack_bottom, stack_size); +#endif +#ifdef SEASTAR_TSAN_ENABLED + g_current_context->destroy_tsan_fiber = true; + g_current_context->fiber = __tsan_create_fiber(0); + g_previous_context->fiber = __tsan_get_current_fiber(); + __tsan_switch_to_fiber(g_current_context->fiber, 0); +#endif swapcontext(&prev->context, initial_context); +#ifdef SEASTAR_ASAN_ENABLED __sanitizer_finish_switch_fiber(g_current_context->fake_stack, &g_previous_context->stack_bottom, &g_previous_context->stack_size); +#endif } void jmp_buf_link::switch_in() @@ -90,40 +113,75 @@ void jmp_buf_link::switch_in() auto prev = std::exchange(g_current_context, this); link = prev; g_previous_context = prev; +#ifdef SEASTAR_ASAN_ENABLED __sanitizer_start_switch_fiber(&prev->fake_stack, stack_bottom, stack_size); +#endif +#ifdef SEASTAR_TSAN_ENABLED + g_previous_context->fiber = __tsan_get_current_fiber(); + __tsan_switch_to_fiber(g_current_context->fiber, 0); +#endif swapcontext(&prev->context, &context); +#ifdef SEASTAR_ASAN_ENABLED __sanitizer_finish_switch_fiber(g_current_context->fake_stack, &g_previous_context->stack_bottom, &g_previous_context->stack_size); +#endif } void jmp_buf_link::switch_out() { g_current_context = link; g_previous_context = this; +#ifdef SEASTAR_ASAN_ENABLED __sanitizer_start_switch_fiber(&fake_stack, g_current_context->stack_bottom, g_current_context->stack_size); +#endif +#ifdef SEASTAR_TSAN_ENABLED + g_previous_context->fiber = __tsan_get_current_fiber(); + __tsan_switch_to_fiber(g_current_context->fiber, 0); +#endif swapcontext(&context, &g_current_context->context); +#ifdef SEASTAR_ASAN_ENABLED __sanitizer_finish_switch_fiber(g_current_context->fake_stack, &g_previous_context->stack_bottom, &g_previous_context->stack_size); +#endif } void jmp_buf_link::initial_switch_in_completed() { +#ifdef SEASTAR_ASAN_ENABLED // This is a new thread and it doesn't have the fake stack yet. ASan will // create it lazily, for now just pass nullptr. __sanitizer_finish_switch_fiber(nullptr, &g_previous_context->stack_bottom, &g_previous_context->stack_size); +#endif } void jmp_buf_link::final_switch_out() { g_current_context = link; g_previous_context = this; +#ifdef SEASTAR_ASAN_ENABLED // Since the thread is about to die we pass nullptr as fake_stack_save argument // so that ASan knows it can destroy the fake stack if it exists. __sanitizer_start_switch_fiber(nullptr, g_current_context->stack_bottom, g_current_context->stack_size); +#endif +#ifdef SEASTAR_TSAN_ENABLED + g_previous_context->fiber = __tsan_get_current_fiber(); + __tsan_switch_to_fiber(g_current_context->fiber, 0); +#endif setcontext(&g_current_context->context); } +#ifdef SEASTAR_TSAN_ENABLED +jmp_buf_link::~jmp_buf_link() +{ + // Do not destroy main context + if (destroy_tsan_fiber) + { + __tsan_destroy_fiber(fiber); + } +} +#endif + #else inline void jmp_buf_link::initial_switch_in(ucontext_t* initial_context, const void*, size_t) @@ -164,16 +222,16 @@ inline void jmp_buf_link::final_switch_out() #endif -// Both asan and optimizations can increase the stack used by a +// Both asan/tsan and optimizations can increase the stack used by a // function. When both are used, we need more than 128 KiB. -#if defined(SEASTAR_ASAN_ENABLED) +#if defined(SEASTAR_ASAN_ENABLED) || defined(SEASTAR_TSAN_ENABLED) static constexpr size_t base_stack_size = 256 * 1024; #else static constexpr size_t base_stack_size = 128 * 1024; #endif static size_t get_stack_size(thread_attributes attr) { -#if defined(__OPTIMIZE__) && defined(SEASTAR_ASAN_ENABLED) +#if defined(__OPTIMIZE__) && (defined(SEASTAR_ASAN_ENABLED) || defined(SEASTAR_TSAN_ENABLED)) return std::max(base_stack_size, attr.stack_size); #else return attr.stack_size ? attr.stack_size : base_stack_size; diff --git a/src/testing/entry_point.cc b/src/testing/entry_point.cc index a84b41698c1..bd4879f8b5f 100644 --- a/src/testing/entry_point.cc +++ b/src/testing/entry_point.cc @@ -45,13 +45,13 @@ static void install_dummy_handler(int sig) { } int entry_point(int argc, char** argv) { -#ifndef SEASTAR_ASAN_ENABLED +#if !defined(SEASTAR_ASAN_ENABLED) && !defined(SEASTAR_TSAN_ENABLED) // Before we call into boost, install some dummy signal // handlers. This seems to be the only way to stop boost from // installing its own handlers, which disables our backtrace // printer. The real handler will be installed when the reactor is // constructed. - // If we are using ASAN, it has already installed a signal handler + // If we are using asan/tsan, it has already installed a signal handler // that does its own stack printing. for (int sig : {SIGSEGV, SIGABRT}) { install_dummy_handler(sig); From 4506cd354ca53ccfb93b6c448550c92154e58d7f Mon Sep 17 00:00:00 2001 From: Denis Borisov Date: Thu, 22 Aug 2024 15:14:17 +0300 Subject: [PATCH 2/2] tests: fix data races with BOOST_REQUIRE BOOST_REQUIRE macros are not thread safe, so replace them with asserts in multithreaded environment --- tests/unit/distributed_test.cc | 18 +++++++-------- tests/unit/scheduling_group_test.cc | 34 ++++++++++++++--------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/tests/unit/distributed_test.cc b/tests/unit/distributed_test.cc index 59ee48b2863..7fe60cc8499 100644 --- a/tests/unit/distributed_test.cc +++ b/tests/unit/distributed_test.cc @@ -142,7 +142,7 @@ SEASTAR_TEST_CASE(test_map_reduce_lifetime) { } auto operator()(const X& x) { return yield().then([this, &x] { - BOOST_REQUIRE(!destroyed); + assert(!destroyed); return x.cpu_id_squared(); }); } @@ -158,7 +158,7 @@ SEASTAR_TEST_CASE(test_map_reduce_lifetime) { } auto operator()(int x) { return yield().then([this, x] { - BOOST_REQUIRE(!destroyed); + assert(!destroyed); res += x; }); } @@ -169,7 +169,7 @@ SEASTAR_TEST_CASE(test_map_reduce_lifetime) { return x.map_reduce(reduce{result}, map{}).then([&result] { long n = smp::count - 1; long expected = (n * (n + 1) * (2*n + 1)) / 6; - BOOST_REQUIRE_EQUAL(result, expected); + assert(result == expected); }); }); }); @@ -186,7 +186,7 @@ SEASTAR_TEST_CASE(test_map_reduce0_lifetime) { } auto operator()(const X& x) const { return yield().then([this, &x] { - BOOST_REQUIRE(!destroyed); + assert(!destroyed); return x.cpu_id_squared(); }); } @@ -199,7 +199,7 @@ SEASTAR_TEST_CASE(test_map_reduce0_lifetime) { destroyed = true; } auto operator()(long res, int x) { - BOOST_REQUIRE(!destroyed); + assert(!destroyed); return res + x; } }; @@ -208,7 +208,7 @@ SEASTAR_TEST_CASE(test_map_reduce0_lifetime) { return x.map_reduce0(map{}, 0L, reduce{}).then([] (long result) { long n = smp::count - 1; long expected = (n * (n + 1) * (2*n + 1)) / 6; - BOOST_REQUIRE_EQUAL(result, expected); + assert(result == expected); }); }); }); @@ -224,7 +224,7 @@ SEASTAR_TEST_CASE(test_map_lifetime) { } auto operator()(const X& x) const { return yield().then([this, &x] { - BOOST_REQUIRE(!destroyed); + assert(!destroyed); return x.cpu_id_squared(); }); } @@ -232,9 +232,9 @@ SEASTAR_TEST_CASE(test_map_lifetime) { return do_with_distributed([] (distributed& x) { return x.start().then([&x] { return x.map(map{}).then([] (std::vector result) { - BOOST_REQUIRE_EQUAL(result.size(), smp::count); + assert(result.size() == smp::count); for (size_t i = 0; i < (size_t)smp::count; i++) { - BOOST_REQUIRE_EQUAL(result[i], i * i); + assert(std::cmp_equal(result[i], i * i)); } }); }); diff --git a/tests/unit/scheduling_group_test.cc b/tests/unit/scheduling_group_test.cc index 380f8d53f71..ad38d7b3bd3 100644 --- a/tests/unit/scheduling_group_test.cc +++ b/tests/unit/scheduling_group_test.cc @@ -71,8 +71,8 @@ SEASTAR_THREAD_TEST_CASE(sg_specific_values_define_after_sg_create) { } for (int i=0; i < num_scheduling_groups; i++) { - BOOST_REQUIRE_EQUAL(sgs[i].get_specific(key1) = (i + 1) * factor, (i + 1) * factor); - BOOST_REQUIRE_EQUAL(sgs[i].get_specific(key2)[0], (i + 1) * factor); + assert(sgs[i].get_specific(key1) == (i + 1) * factor); + assert(sgs[i].get_specific(key2)[0] == (i + 1) * factor); } }).get(); @@ -81,7 +81,7 @@ SEASTAR_THREAD_TEST_CASE(sg_specific_values_define_after_sg_create) { return reduce_scheduling_group_specific(std::plus(), int(0), key1).then([] (int sum) { int factor = this_shard_id() + 1; int expected_sum = ((1 + num_scheduling_groups)*num_scheduling_groups) * factor /2; - BOOST_REQUIRE_EQUAL(expected_sum, sum); + assert(expected_sum == sum); }). then([key2] { auto ivec_to_int = [] (ivec& v) { return v.size() ? v[0] : 0; @@ -90,7 +90,7 @@ SEASTAR_THREAD_TEST_CASE(sg_specific_values_define_after_sg_create) { return map_reduce_scheduling_group_specific(ivec_to_int, std::plus(), int(0), key2).then([] (int sum) { int factor = this_shard_id() + 1; int expected_sum = ((1 + num_scheduling_groups)*num_scheduling_groups) * factor /2; - BOOST_REQUIRE_EQUAL(expected_sum, sum); + assert(expected_sum == sum); }); }); @@ -129,8 +129,8 @@ SEASTAR_THREAD_TEST_CASE(sg_specific_values_define_before_sg_create) { } for (int i=0; i < num_scheduling_groups; i++) { - BOOST_REQUIRE_EQUAL(sgs[i].get_specific(key1) = (i + 1) * factor, (i + 1) * factor); - BOOST_REQUIRE_EQUAL(sgs[i].get_specific(key2)[0], (i + 1) * factor); + assert(sgs[i].get_specific(key1) == (i + 1) * factor); + assert(sgs[i].get_specific(key2)[0] == (i + 1) * factor); } }).get(); @@ -139,7 +139,7 @@ SEASTAR_THREAD_TEST_CASE(sg_specific_values_define_before_sg_create) { return reduce_scheduling_group_specific(std::plus(), int(0), key1).then([] (int sum) { int factor = this_shard_id() + 1; int expected_sum = ((1 + num_scheduling_groups)*num_scheduling_groups) * factor /2; - BOOST_REQUIRE_EQUAL(expected_sum, sum); + assert(expected_sum == sum); }). then([key2] { auto ivec_to_int = [] (ivec& v) { return v.size() ? v[0] : 0; @@ -148,7 +148,7 @@ SEASTAR_THREAD_TEST_CASE(sg_specific_values_define_before_sg_create) { return map_reduce_scheduling_group_specific(ivec_to_int, std::plus(), int(0), key2).then([] (int sum) { int factor = this_shard_id() + 1; int expected_sum = ((1 + num_scheduling_groups)*num_scheduling_groups) * factor /2; - BOOST_REQUIRE_EQUAL(expected_sum, sum); + assert(expected_sum == sum); }); }); @@ -191,8 +191,8 @@ SEASTAR_THREAD_TEST_CASE(sg_specific_values_define_before_and_after_sg_create) { } for (int i=0; i < num_scheduling_groups; i++) { - BOOST_REQUIRE_EQUAL(sgs[i].get_specific(key1) = (i + 1) * factor, (i + 1) * factor); - BOOST_REQUIRE_EQUAL(sgs[i].get_specific(key2)[0], (i + 1) * factor); + assert(sgs[i].get_specific(key1) == (i + 1) * factor); + assert(sgs[i].get_specific(key2)[0] == (i + 1) * factor); } }).get(); @@ -201,7 +201,7 @@ SEASTAR_THREAD_TEST_CASE(sg_specific_values_define_before_and_after_sg_create) { return reduce_scheduling_group_specific(std::plus(), int(0), key1).then([] (int sum) { int factor = this_shard_id() + 1; int expected_sum = ((1 + num_scheduling_groups)*num_scheduling_groups) * factor /2; - BOOST_REQUIRE_EQUAL(expected_sum, sum); + assert(expected_sum == sum); }). then([key2] { auto ivec_to_int = [] (ivec& v) { return v.size() ? v[0] : 0; @@ -210,7 +210,7 @@ SEASTAR_THREAD_TEST_CASE(sg_specific_values_define_before_and_after_sg_create) { return map_reduce_scheduling_group_specific(ivec_to_int, std::plus(), int(0), key2).then([] (int sum) { int factor = this_shard_id() + 1; int expected_sum = ((1 + num_scheduling_groups)*num_scheduling_groups) * factor /2; - BOOST_REQUIRE_EQUAL(expected_sum, sum); + assert(expected_sum == sum); }); }); @@ -234,7 +234,7 @@ SEASTAR_THREAD_TEST_CASE(sg_scheduling_group_inheritance_in_seastar_async_test) internal::scheduling_group_index(*(attr.sched_group))); smp::invoke_on_all([sched_group_idx = internal::scheduling_group_index(*(attr.sched_group))] () { - BOOST_REQUIRE_EQUAL(internal::scheduling_group_index(current_scheduling_group()), sched_group_idx); + assert(internal::scheduling_group_index(current_scheduling_group()) == sched_group_idx); }).get(); }).get(); }).get(); @@ -326,7 +326,7 @@ SEASTAR_THREAD_TEST_CASE(sg_rename_callback) { smp::invoke_on_all([&sgs, &keys] () { for (size_t s = 0; s < std::size(sgs); ++s) { for (size_t k = 0; k < std::size(keys); ++k) { - BOOST_REQUIRE_EQUAL(sgs[s].get_specific(keys[k])._sg_name, fmt::format("sg-old-{}", s)); + assert(sgs[s].get_specific(keys[k])._sg_name == fmt::format("sg-old-{}", s)); sgs[s].get_specific(keys[k])._id = 1; } } @@ -339,9 +339,9 @@ SEASTAR_THREAD_TEST_CASE(sg_rename_callback) { smp::invoke_on_all([&sgs, &keys] () { for (size_t s = 0; s < std::size(sgs); ++s) { for (size_t k = 0; k < std::size(keys); ++k) { - BOOST_REQUIRE_EQUAL(sgs[s].get_specific(keys[k])._sg_name, fmt::format("sg-new-{}", s)); + assert(sgs[s].get_specific(keys[k])._sg_name == fmt::format("sg-new-{}", s)); // Checks that the value only saw a rename(), not a reconstruction. - BOOST_REQUIRE_EQUAL(sgs[s].get_specific(keys[k])._id, 1); + assert(sgs[s].get_specific(keys[k])._id == 1); } } }).get(); @@ -364,7 +364,7 @@ SEASTAR_THREAD_TEST_CASE(sg_create_check_unique_constructor_invocation) { check() { auto sg = current_scheduling_group(); auto id = internal::scheduling_group_index(sg); - BOOST_REQUIRE(groups.count(id) == 0); + assert(groups.count(id) == 0); groups.emplace(id); } };