Fix x64/Debug build on MSVS #2
Conversation
|
I cannot find a CLA (contribution license agreement) on file for you? We cannot accept your contribution without one, unfortunately... Could you make sure you have one by going here? https://cla.developers.google.com/ Thanks! |
|
@pphaneuf I submitted it just now. |
|
And the newfangled CLA robot saw it. Thanks! I'm not reviewing the code change, I have no idea about this Windows stuff, just dropping by for the CLA stuff, leaving it to the other maintainers to merge this in. |
| @@ -1459,7 +1459,7 @@ static void logging_fail() { | |||
| #if defined(_DEBUG) && defined(_MSC_VER) | |||
| // When debugging on windows, avoid the obnoxious dialog and make | |||
| // it possible to continue past a LOG(FATAL) in the debugger | |||
| _asm int 3 | |||
| __debugbreak(); | |||
There was a problem hiding this comment.
no need to include <intrin.h> ?
https://msdn.microsoft.com/en-us/library/f408b4et.aspx
There was a problem hiding this comment.
It compiles without it.
As far as I know, <windows.h> includes it, so maybe windows/port.h is included somehow.
To be sure, the function "DebugBreak()" from windows.h can be used:
https://msdn.microsoft.com/library/windows/desktop/ms679297.aspx
However, the official replacement (according to https://msdn.microsoft.com/en-us/library/45yd4tzz.aspx) for _asm int 3 is __debugbreak().
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.
Fix x64/Debug build on MSVS
This fixes building glog on Windows using the Debug configuration on the x64 platform.