Skip to content

Commit

Permalink
Standardize on GCC for TSAN conditional compilation (facebook#8543)
Browse files Browse the repository at this point in the history
Summary:
In facebook#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: facebook#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 facebook#8538

Reviewed By: zhichao-cao

Differential Revision: D29728483

Pulled By: pdillinger

fbshipit-source-id: 8a3b8015c2ed48078214c3ee17146a2c3f11c9f7
  • Loading branch information
pdillinger authored and facebook-github-bot committed Jul 16, 2021
1 parent b678cb1 commit aeb913d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 17 deletions.
17 changes: 17 additions & 0 deletions port/lang.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#endif
#endif

// ASAN (Address sanitizer)

#if defined(__clang__)
#if defined(__has_feature)
#if __has_feature(address_sanitizer)
Expand Down Expand Up @@ -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
6 changes: 3 additions & 3 deletions port/stack_trace.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ void* SaveStack(int* /*num_frames*/, int /*first_frames_to_skip*/) {
#include <sys/sysctl.h>
#endif

#include "port/lang.h"

namespace ROCKSDB_NAMESPACE {
namespace port {

Expand Down Expand Up @@ -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"
Expand All @@ -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...)
Expand Down
15 changes: 1 addition & 14 deletions trace_replay/io_tracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <fstream>

#include "monitoring/instrumented_mutex.h"
#include "port/lang.h"
#include "rocksdb/file_system.h"
#include "rocksdb/options.h"
#include "trace_replay/trace_replay.h"
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit aeb913d

Please sign in to comment.