Skip to content

Commit

Permalink
Annotate LOG_EVERY_N macros as a benign race for TSAN
Browse files Browse the repository at this point in the history
Summary:
Issue google#80 points out several places in glog where TSAN discovers
false positives. One of these places is in the `LOG_EVERY_N` macros.
These macros are implemented by maintaining a static unprotected
integer counter, and TSAN will report data races on these counters.

Here is a minimum example to reproduce the data race:

```

void logging() {
  for (int i = 0; i < 300; ++i) {
    LOG_EVERY_N(INFO, 2) << "foo";
  }
}

int main() {
  auto t1 = std::thread(logging);
  auto t2 = std::thread(logging);
  t1.join();
  t2.join();
  return 0;
}
```

And here is the TSAN report:

```
WARNING: ThreadSanitizer: data race (pid=776850)
  Write of size 4 at 0x558de483f684 by thread T2:
    #0 logging()
    google#1 void std::_Bind_simple<void (*())()>::_M_invoke<>(std::_Index_tuple<>)
    google#2 std::_Bind_simple<void (*())()>::operator()()
    google#3 std::thread::_Impl<std::_Bind_simple<void (*())()> >::_M_run()
    google#4 execute_native_thread_routine

  Previous write of size 4 at 0x558de483f684 by thread T1:
    #0 logging()
    google#1 void std::_Bind_simple<void (*())()>::_M_invoke<>(std::_Index_tuple<>)
    google#2 std::_Bind_simple<void (*())()>::operator()()
    google#3 std::thread::_Impl<std::_Bind_simple<void (*())()> >::_M_run()
    google#4 execute_native_thread_routine

  Location is global '<null>' at 0x000000000000 (main+0x00000011c684)

  Thread T2 (tid=776857, running) created by main thread at:
    #0 pthread_create
    google#1 __gthread_create
    google#2 std::thread::_M_start_thread(std::shared_ptr<std::thread::_Impl_base>, void (*)())
    google#3 main

  Thread T1 (tid=776856, running) created by main thread at:
    #0 pthread_create
    google#1 __gthread_create
    google#2 std::thread::_M_start_thread(std::shared_ptr<std::thread::_Impl_base>, void (*)())
    google#3 main

SUMMARY: ThreadSanitizer: data race in logging()
```

To avoid noisy TSAN reports and also avoid adding a performance hit, this
change will mark these counters as benign races so that TSAN will not report
them. This change will only have an effect if we are compiling with TSAN;
there are no changes if we are not building with TSAN.

With this change, the above example no longer reports a data race when built
and run with TSAN.
  • Loading branch information
kennyyu committed Nov 4, 2017
1 parent d72e3be commit 4764ca6
Showing 1 changed file with 38 additions and 0 deletions.
38 changes: 38 additions & 0 deletions src/glog/logging.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -888,8 +888,41 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \
#define LOG_OCCURRENCES LOG_EVERY_N_VARNAME(occurrences_, __LINE__)
#define LOG_OCCURRENCES_MOD_N LOG_EVERY_N_VARNAME(occurrences_mod_n_, __LINE__)

#if defined(__has_feature)
#define _GLOG_HAS_FEATURE(...) __has_feature(__VA_ARGS__)
#else
#define _GLOG_HAS_FEATURE(...) 0
#endif

#if _GLOG_HAS_FEATURE(thread_sanitizer) || __SANITIZE_THREAD__
#define _GLOG_SANITIZE_THREAD 1
#endif

#if defined(_GLOG_SANITIZE_THREAD)
#define _GLOG_IFDEF_THREAD_SANITIZER(X) X
#else
#define _GLOG_IFDEF_THREAD_SANITIZER(X)
#endif

#if defined(_GLOG_SANITIZE_THREAD)
} // namespace google

// We need to identify the static variables as "benign" races
// to avoid noisy reports from TSAN.
extern "C" void AnnotateBenignRaceSized(
const char *file,
int line,
const volatile void *mem,
long size,
const char *description);

namespace google {
#endif

#define SOME_KIND_OF_LOG_EVERY_N(severity, n, what_to_do) \
static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \
_GLOG_IFDEF_THREAD_SANITIZER(AnnotateBenignRaceSized(__FILE__, __LINE__, &LOG_OCCURRENCES, sizeof(int), "")); \
_GLOG_IFDEF_THREAD_SANITIZER(AnnotateBenignRaceSized(__FILE__, __LINE__, &LOG_OCCURRENCES_MOD_N, sizeof(int), "")); \
++LOG_OCCURRENCES; \
if (++LOG_OCCURRENCES_MOD_N > n) LOG_OCCURRENCES_MOD_N -= n; \
if (LOG_OCCURRENCES_MOD_N == 1) \
Expand All @@ -899,6 +932,8 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \

#define SOME_KIND_OF_LOG_IF_EVERY_N(severity, condition, n, what_to_do) \
static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \
_GLOG_IFDEF_THREAD_SANITIZER(AnnotateBenignRaceSized(__FILE__, __LINE__, &LOG_OCCURRENCES, sizeof(int), "")); \
_GLOG_IFDEF_THREAD_SANITIZER(AnnotateBenignRaceSized(__FILE__, __LINE__, &LOG_OCCURRENCES_MOD_N, sizeof(int), "")); \
++LOG_OCCURRENCES; \
if (condition && \
((LOG_OCCURRENCES_MOD_N=(LOG_OCCURRENCES_MOD_N + 1) % n) == (1 % n))) \
Expand All @@ -908,6 +943,8 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \

#define SOME_KIND_OF_PLOG_EVERY_N(severity, n, what_to_do) \
static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \
_GLOG_IFDEF_THREAD_SANITIZER(AnnotateBenignRaceSized(__FILE__, __LINE__, &LOG_OCCURRENCES, sizeof(int), "")); \
_GLOG_IFDEF_THREAD_SANITIZER(AnnotateBenignRaceSized(__FILE__, __LINE__, &LOG_OCCURRENCES_MOD_N, sizeof(int), "")); \
++LOG_OCCURRENCES; \
if (++LOG_OCCURRENCES_MOD_N > n) LOG_OCCURRENCES_MOD_N -= n; \
if (LOG_OCCURRENCES_MOD_N == 1) \
Expand All @@ -917,6 +954,7 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \

#define SOME_KIND_OF_LOG_FIRST_N(severity, n, what_to_do) \
static int LOG_OCCURRENCES = 0; \
_GLOG_IFDEF_THREAD_SANITIZER(AnnotateBenignRaceSized(__FILE__, __LINE__, &LOG_OCCURRENCES, sizeof(int), "")); \
if (LOG_OCCURRENCES <= n) \
++LOG_OCCURRENCES; \
if (LOG_OCCURRENCES <= n) \
Expand Down

0 comments on commit 4764ca6

Please sign in to comment.