From 4764ca65f5ac4e47166797801ff616a55b8a8ab3 Mon Sep 17 00:00:00 2001 From: Kenny Yu Date: Fri, 3 Nov 2017 17:32:47 -0700 Subject: [PATCH] Annotate LOG_EVERY_N macros as a benign race for TSAN Summary: Issue #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() #1 void std::_Bind_simple::_M_invoke<>(std::_Index_tuple<>) #2 std::_Bind_simple::operator()() #3 std::thread::_Impl >::_M_run() #4 execute_native_thread_routine Previous write of size 4 at 0x558de483f684 by thread T1: #0 logging() #1 void std::_Bind_simple::_M_invoke<>(std::_Index_tuple<>) #2 std::_Bind_simple::operator()() #3 std::thread::_Impl >::_M_run() #4 execute_native_thread_routine Location is global '' at 0x000000000000 (main+0x00000011c684) Thread T2 (tid=776857, running) created by main thread at: #0 pthread_create #1 __gthread_create #2 std::thread::_M_start_thread(std::shared_ptr, void (*)()) #3 main Thread T1 (tid=776856, running) created by main thread at: #0 pthread_create #1 __gthread_create #2 std::thread::_M_start_thread(std::shared_ptr, void (*)()) #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. --- src/glog/logging.h.in | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/glog/logging.h.in b/src/glog/logging.h.in index bf6d754e6..5c56ccf4d 100644 --- a/src/glog/logging.h.in +++ b/src/glog/logging.h.in @@ -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) \ @@ -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))) \ @@ -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) \ @@ -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) \