glog should use ThreadSanitizer (TSAN) dynamic annotations #80
Labels
Comments
|
Another example in stacktrace_libunwind-inl.h -- the 'g_now_entering' variable should be stored using a 'Release_Store' at the end of GetStackTrace(...). Probably many more, so I won't bother listing them all. I know that Google uses TSAN internally heavily. Does Google have any intention to open source newer versions of glog, or should we assume that any improvements here will have to be done by the external community? |
|
One more: there should be an ANNOTATE_BENIGN_RACE on fatal_msg_data_shared to prevent a TSAN warning when multiple threads FATAL at the same time. (or it should be made thread-safe) |
dwayneberry
pushed a commit
to heavyai/heavydb
that referenced
this issue
Jun 23, 2017
TSAN reported some issues due to the non-constness of data_dir. It also reported some issues with glog which appear to be false positives, but removing for now instead of adding to the exclusion list. See google/glog#80
kennyyu
added a commit
to kennyyu/glog
that referenced
this issue
Nov 4, 2017
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.
|
@toddlipcon This PR will mark the LOG_EVERY_N macros as benign races: #263 |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Trying to build glog with TSAN ends up causing a bunch of false positives due to probably-benign races. For example, RawLog_SetLastTime operates unprotected on global variables. This is questionable (since 'struct tm' is relatively large) and generates a TSAN warning. Fixing this to use a seqlock, or at least annotating it as benign, would be helpful for those trying to run glog in TSAN builds.
Another example is the thread-unsafe counters for LOG_EVERY_N and similar macros. These need to be annotated as benign.
The text was updated successfully, but these errors were encountered: