Data race reports from ThreadSanitizer #13025
Comments
|
Thanks, I looked at the ones in dart/runtime/vm/parser.cc and dart/runtime/vm/parser.h: They come from global variables used for compiler statistics and debug printing. We can remove those, or hide them under a flag. The result of the data race in this case would be that those counters are incorrect. They count compiler-internal performance statistics for profiling. |
|
Thanks for the report and quick triage. I'll take a deeper look and will mark those counters as potentially lossy. Set owner to @iposva-google. |
|
This comment was originally written by gli...@chromium.org You'll need to use atomic operations for those counters, otherwise the |
|
I have a CL that hides those counters behind the corresponding flag: https://codereview.chromium.org/23819018/ The assumption is that when running with --compiler-stats which is off by default we don't care about those data races. |
|
Florian, thanks for that CL. But soon I would like these counters to always be accessible regardless of the state of --compiler-stats. So this would only be a short-term fix which will be reverted soon, so I am not sure it is worth the effort. |
|
If on by default, the counters are not useful if they don't work correctly with multiple threads. I think of them as a debugging tool for VM developers only. As mentioned above, anything else would require atomic operations. If those don't cost too much, we can surely do atomic increment/decrement there. |
|
This comment was originally written by gli...@chromium.org If those counters are intended to be lossy (e.g. they're used for collecting the stats), it's ok to use nobarrier atomics for them (and my comment referring to https://codereview.chromium.org/23450016 is still applicable) |
|
This comment was originally written by gli...@chromium.org Hi Ivan, any progress with these issue? Should we perhaps split the known bugs into separate issues? Do you need any help with analyzing the rest? |
larsbak
added
Type-Defect
area-vm
|
This bug is stale. The race in compiler stat counters has been removed some time ago by introducing per-isolate counters. |
Hi Dart engs,
I've tried ThreadSanitizer (a compiler instrumentation based data race
detector, clang.llvm.org/docs/ThreadSanitizer.html) on Dart tests and
got a number of race reports, particularly related to dart::Parser
(although there's a number of other races).
Can someone please help to verify whether these races are true positives or not?
The common sources of false positives are:
- custom thread implementations (looks like the mentioned code uses
pthreads, so this shouldn't be the case)
- custom synchronization primitives (TSan knows about pthread mutexes
and condvars and C++11 atomics, but other implementations of
atomics/locks/etc. may require annotations).
Unless these are used, the reported races are likely to be true.
In order to reproduce the reports you'll need to run the following commands:
export CC=/path/to/clang
export CXX=/path/to/clang++
export LD=/path/to/clang++
export CFLAGS="-fsanitize=thread -w -fPIE"
export CXXLAGS="-fsanitize=thread -w -fPIE"
export LDLAGS="-fsanitize=thread -pie"
./tools/build.py --arch=x64
The next line may hang your machine. If this happens, try running
tests one by one
./tools/test.py --arch=x64 --runtime=vm
Attached is a log from the invocation of test.py. There are 18
distinct race reports:
SUMMARY: ThreadSanitizer: data race
dart/runtime/bin/dbg_connection.cc:438
dart::bin::DebuggerConnectionHandler::AddNewDebuggerConnection(int)
SUMMARY: ThreadSanitizer: data race
dart/runtime/bin/process_linux.cc:156
dart::bin::ExitCodeHandler::WakeUpFd()
SUMMARY: ThreadSanitizer: data race dart/runtime/platform/assert.h:101
void dart::DynamicAssertionHelper::Equals<long, long>(long const&,
long const&)
SUMMARY: ThreadSanitizer: data race
dart/runtime/vm/message_handler_test.cc:72
dart::TestMessageHandler::port_buffer() const
SUMMARY: ThreadSanitizer: data race
dart/runtime/vm/message_handler_test.cc:74
dart::TestMessageHandler::message_count() const
SUMMARY: ThreadSanitizer: data race
dart/runtime/vm/message_handler_test.cc:75
dart::TestMessageHandler::start_called() const
SUMMARY: ThreadSanitizer: data race dart/runtime/vm/parser.cc:348
dart::Parser::SetPosition(long)
SUMMARY: ThreadSanitizer: data race dart/runtime/vm/parser.cc:371
dart::Parser::CurrentToken()
SUMMARY: ThreadSanitizer: data race dart/runtime/vm/parser.cc:377
dart::Parser::LookaheadToken(int)
SUMMARY: ThreadSanitizer: data race dart/runtime/vm/parser.cc:67 ~TraceParser
SUMMARY: ThreadSanitizer: data race dart/runtime/vm/parser.h:273
dart::Parser::ConsumeToken()
SUMMARY: ThreadSanitizer: data race dart/runtime/vm/thread_pool.cc:200
dart::ThreadPool::ReleaseIdleWorker(dart::ThreadPool::Worker*)
SUMMARY: ThreadSanitizer: data race
dart/runtime/vm/thread_pool_test.cc:159
dart::Dart_TestThreadPool_WorkerTimeout()
SUMMARY: ThreadSanitizer: data race
dart/third_party/net_nss/ssl/sslsock.c:270 ssl_FindSocket
SUMMARY: ThreadSanitizer: data race
dart/third_party/nss/nspr/pr/src/pthreads/ptsynch.c:149 PR_NewLock
SUMMARY: ThreadSanitizer: data race
dart/third_party/nss/nspr/pr/src/pthreads/ptsynch.c:165 PR_DestroyLock
SUMMARY: ThreadSanitizer: data race
dart/third_party/nss/nspr/pr/src/pthreads/ptsynch.c:186 PR_Lock
SUMMARY: ThreadSanitizer: data race
dart/third_party/nss/nspr/pr/src/pthreads/ptsynch.c:211 PR_Unlock
Attachment:
log.gz (312.24 KB)
The text was updated successfully, but these errors were encountered: