From aeb913dd01b0dc4ccdf5f9e05316802a94c1d6f1 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Thu, 15 Jul 2021 23:49:12 -0700 Subject: [PATCH] Standardize on GCC for TSAN conditional compilation (#8543) Summary: In https://github.com/facebook/rocksdb/issues/8539 I accidentally only checked for GCC TSAN, which is what I tested locally, while CircleCI and FB CI use clang TSAN. Related: other existing code like in stack_trace.cc only check for clang TSAN. I've now standardized these to the GCC convention in port/lang.h, so now #ifdef __SANITIZE_THREAD__ can check for any TSAN (assuming lang.h include) Pull Request resolved: https://github.com/facebook/rocksdb/pull/8543 Test Plan: Put an assert(false) in slice_test and look for the NOTE about "signal-unsafe call", both GCC and clang. Eventually, CircleCI TSAN in https://github.com/facebook/rocksdb/issues/8538 Reviewed By: zhichao-cao Differential Revision: D29728483 Pulled By: pdillinger fbshipit-source-id: 8a3b8015c2ed48078214c3ee17146a2c3f11c9f7 --- port/lang.h | 17 +++++++++++++++++ port/stack_trace.cc | 6 +++--- trace_replay/io_tracer.h | 15 +-------------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/port/lang.h b/port/lang.h index eabe44b69..4a601d41c 100644 --- a/port/lang.h +++ b/port/lang.h @@ -15,6 +15,8 @@ #endif #endif +// ASAN (Address sanitizer) + #if defined(__clang__) #if defined(__has_feature) #if __has_feature(address_sanitizer) @@ -43,3 +45,18 @@ #else #define STATIC_AVOID_DESTRUCTION(Type, name) static Type& name = *new Type #endif + +// TSAN (Thread sanitizer) + +// For simplicity, standardize on the GCC define +#if defined(__clang__) +#if defined(__has_feature) && __has_feature(thread_sanitizer) +#define __SANITIZE_THREAD__ 1 +#endif // __has_feature(thread_sanitizer) +#endif // __clang__ + +#ifdef __SANITIZE_THREAD__ +#define TSAN_SUPPRESSION __attribute__((no_sanitize("thread"))) +#else +#define TSAN_SUPPRESSION +#endif // TSAN_SUPPRESSION diff --git a/port/stack_trace.cc b/port/stack_trace.cc index c82da2a20..15a36bb71 100644 --- a/port/stack_trace.cc +++ b/port/stack_trace.cc @@ -36,6 +36,8 @@ void* SaveStack(int* /*num_frames*/, int /*first_frames_to_skip*/) { #include #endif +#include "port/lang.h" + namespace ROCKSDB_NAMESPACE { namespace port { @@ -163,8 +165,7 @@ static void StackTraceHandler(int sig) { // Efforts to fix or suppress TSAN warnings "signal-unsafe call inside of // a signal" have failed, so just warn the user about them. -#if defined(__clang__) && defined(__has_feature) -#if __has_feature(thread_sanitizer) +#ifdef __SANITIZE_THREAD__ fprintf(stderr, "==> NOTE: any above warnings about \"signal-unsafe call\" are\n" "==> ignorable, as they are expected when generating a stack\n" @@ -173,7 +174,6 @@ static void StackTraceHandler(int sig) { "==> in the TSAN warning can be useful for that. (The stack\n" "==> trace printed by the signal handler is likely obscured\n" "==> by TSAN output.)\n"); -#endif #endif // re-signal to default handler (so we still get core dump if needed...) diff --git a/trace_replay/io_tracer.h b/trace_replay/io_tracer.h index 83bcaee9d..8db6e2a27 100644 --- a/trace_replay/io_tracer.h +++ b/trace_replay/io_tracer.h @@ -9,6 +9,7 @@ #include #include "monitoring/instrumented_mutex.h" +#include "port/lang.h" #include "rocksdb/file_system.h" #include "rocksdb/options.h" #include "trace_replay/trace_replay.h" @@ -157,20 +158,6 @@ class IOTracer { // mutex and ignore the operation if writer_is null. So its ok if // tracing_enabled shows non updated value. -#if defined(__clang__) -#if defined(__has_feature) && __has_feature(thread_sanitizer) -#define TSAN_SUPPRESSION __attribute__((no_sanitize("thread"))) -#endif // __has_feature(thread_sanitizer) -#else // __clang__ -#ifdef __SANITIZE_THREAD__ -#define TSAN_SUPPRESSION __attribute__((no_sanitize("thread"))) -#endif // __SANITIZE_THREAD__ -#endif // __clang__ - -#ifndef TSAN_SUPPRESSION -#define TSAN_SUPPRESSION -#endif // TSAN_SUPPRESSION - // Start writing IO operations to the trace_writer. TSAN_SUPPRESSION Status StartIOTrace(SystemClock* clock, const TraceOptions& trace_options,