Mehdi Amini via llvm-dev
2016-Apr-16 05:24 UTC
[llvm-dev] [TSAN] LLVM statistics and pass initialization trigger race detection
Hello, I trying TSAN on Darwin on LLVM itself (sanitizing multi-threaded ThinLTO link). However I see two main issues on my debug build: 1) Statistics: the pre/post increment is not safe, it seems to be acknowledge in the code itself: // FIXME: This function and all those that follow carefully use an // atomic operation to update the value safely in the presence of // concurrent accesses, but not to read the return value, so the // return value is not thread safe. Can we tell TSAN to ignore the statistics somehow? Alternatively, is there a fix someone can suggest? 2) Pass initialization. This macro does not please TSAN (even though it seems written with TSAN in mind): #define CALL_ONCE_INITIALIZATION(function) \ static volatile sys::cas_flag initialized = 0; \ sys::cas_flag old_val = sys::CompareAndSwap(&initialized, 1, 0); \ if (old_val == 0) { \ function(Registry); \ sys::MemoryFence(); \ TsanIgnoreWritesBegin(); \ TsanHappensBefore(&initialized); \ initialized = 2; \ TsanIgnoreWritesEnd(); \ } else { \ sys::cas_flag tmp = initialized; \ sys::MemoryFence(); \ while (tmp != 2) { \ tmp = initialized; \ sys::MemoryFence(); \ } \ } \ TsanHappensAfter(&initialized); The failure looks like: =================WARNING: ThreadSanitizer: data race (pid=17192) Atomic write of size 4 at 0x0001113619e8 by thread T13: #0 __tsan_atomic32_compare_exchange_val <null>:568340296 (libclang_rt.tsan_osx_dynamic.dylib+0x00000003e7aa) #1 llvm::sys::CompareAndSwap(unsigned int volatile*, unsigned int, unsigned int) Atomic.cpp:52 (libLTO.dylib+0x000000511914) #2 llvm::initializeSimpleInlinerPass(llvm::PassRegistry&) InlineSimple.cpp:82 (libLTO.dylib+0x000000ab2b55) #3 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:50 (libLTO.dylib+0x000000ab2e8e) #4 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:49 (libLTO.dylib+0x000000ab2d19) #5 llvm::createFunctionInliningPass() InlineSimple.cpp:85 (libLTO.dylib+0x000000ab2ce3) ... Previous read of size 4 at 0x0001113619e8 by thread T14: #0 llvm::initializeSimpleInlinerPass(llvm::PassRegistry&) InlineSimple.cpp:82 (libLTO.dylib+0x000000ab2b65) #1 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:50 (libLTO.dylib+0x000000ab2e8e) #2 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:49 (libLTO.dylib+0x000000ab2d19) #3 llvm::createFunctionInliningPass() InlineSimple.cpp:85 (libLTO.dylib+0x000000ab2ce3) ... Location is global 'llvm::initializeSimpleInlinerPass(llvm::PassRegistry&)::initialized' at 0x0001113619e8 (libLTO.dylib+0x0000016389e8) Thanks! Mehdi -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160415/b34ec2a8/attachment.html>
Kostya Serebryany via llvm-dev
2016-Apr-18 17:38 UTC
[llvm-dev] [TSAN] LLVM statistics and pass initialization trigger race detection
+dvyukov On Fri, Apr 15, 2016 at 10:24 PM, Mehdi Amini via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Hello, > > I trying TSAN on Darwin on LLVM itself (sanitizing multi-threaded ThinLTO > link). > However I see two main issues on my debug build: > > 1) Statistics: the pre/post increment is not safe, it seems to be > acknowledge in the code itself: > > // FIXME: This function and all those that follow carefully use an > // atomic operation to update the value safely in the presence of > // concurrent accesses, but not to read the return value, so the > // return value is not thread safe. > > Can we tell TSAN to ignore the statistics somehow? Alternatively, is there > a fix someone can suggest? >It's better to add proper synchronization, most likely an atomic read with memory_order_relaxed is enough.> > 2) Pass initialization. This macro does not please TSAN (even though it > seems written with TSAN in mind): > > #define CALL_ONCE_INITIALIZATION(function) \ > static volatile sys::cas_flag initialized = 0; \ > sys::cas_flag old_val = sys::CompareAndSwap(&initialized, 1, 0); \ > if (old_val == 0) { \ > function(Registry); \ > sys::MemoryFence(); \ > TsanIgnoreWritesBegin(); \ > TsanHappensBefore(&initialized); \ > initialized = 2; \ > TsanIgnoreWritesEnd(); \ > } else { \ > sys::cas_flag tmp = initialized; \ > sys::MemoryFence(); \ > while (tmp != 2) { \ > tmp = initialized; \ > sys::MemoryFence(); \ > } \ > } \ > TsanHappensAfter(&initialized); >This code might have been added to please the old valgrind-based tsan (which did not understand atomics). For the current tsan we should just implement proper synchronization and remove the annotations. Why can't we use std::call_once here? Looks like you are serious about running llvm under tsan. Very cool! Does it make sense to add a tsan build to our sanitizer bot <http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast> (which already has asan, ubsan, lsan, and msan)? What tests and what build mode would you suggest for the bot? --kcc> > The failure looks like: > > =================> WARNING: ThreadSanitizer: data race (pid=17192) > Atomic write of size 4 at 0x0001113619e8 by thread T13: > #0 __tsan_atomic32_compare_exchange_val > <null>:568340296 (libclang_rt.tsan_osx_dynamic.dylib+0x00000003e7aa) > #1 llvm::sys::CompareAndSwap(unsigned int volatile*, unsigned > int, unsigned int) Atomic.cpp:52 (libLTO.dylib+0x000000511914) > #2 > llvm::initializeSimpleInlinerPass(llvm::PassRegistry&) InlineSimple.cpp:82 > (libLTO.dylib+0x000000ab2b55) > #3 (anonymous > namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:50 > (libLTO.dylib+0x000000ab2e8e) > #4 (anonymous > namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:49 > (libLTO.dylib+0x000000ab2d19) > #5 llvm::createFunctionInliningPass() > InlineSimple.cpp:85 (libLTO.dylib+0x000000ab2ce3) > ... > > Previous read of size 4 at 0x0001113619e8 by thread T14: > #0 > llvm::initializeSimpleInlinerPass(llvm::PassRegistry&) InlineSimple.cpp:82 > (libLTO.dylib+0x000000ab2b65) > #1 (anonymous > namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:50 > (libLTO.dylib+0x000000ab2e8e) > #2 (anonymous > namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:49 > (libLTO.dylib+0x000000ab2d19) > #3 llvm::createFunctionInliningPass() > InlineSimple.cpp:85 (libLTO.dylib+0x000000ab2ce3) > ... > Location is > global 'llvm::initializeSimpleInlinerPass(llvm::PassRegistry&)::initialized' at > 0x0001113619e8 (libLTO.dylib+0x0000016389e8) > > > Thanks! > > Mehdi > > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160418/98502b18/attachment-0001.html>
Mehdi Amini via llvm-dev
2016-Apr-18 17:43 UTC
[llvm-dev] [TSAN] LLVM statistics and pass initialization trigger race detection
Hi, Thanks for the answers.> On Apr 18, 2016, at 10:38 AM, Kostya Serebryany <kcc at google.com> wrote: > > +dvyukov > > > > On Fri, Apr 15, 2016 at 10:24 PM, Mehdi Amini via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > Hello, > > I trying TSAN on Darwin on LLVM itself (sanitizing multi-threaded ThinLTO link). > However I see two main issues on my debug build: > > 1) Statistics: the pre/post increment is not safe, it seems to be acknowledge in the code itself: > > // FIXME: This function and all those that follow carefully use an > // atomic operation to update the value safely in the presence of > // concurrent accesses, but not to read the return value, so the > // return value is not thread safe. > > Can we tell TSAN to ignore the statistics somehow? Alternatively, is there a fix someone can suggest? > > It's better to add proper synchronization, most likely an atomic read with memory_order_relaxed is enough.Ok, will look into this!> > > 2) Pass initialization. This macro does not please TSAN (even though it seems written with TSAN in mind): > > #define CALL_ONCE_INITIALIZATION(function) \ > static volatile sys::cas_flag initialized = 0; \ > sys::cas_flag old_val = sys::CompareAndSwap(&initialized, 1, 0); \ > if (old_val == 0) { \ > function(Registry); \ > sys::MemoryFence(); \ > TsanIgnoreWritesBegin(); \ > TsanHappensBefore(&initialized); \ > initialized = 2; \ > TsanIgnoreWritesEnd(); \ > } else { \ > sys::cas_flag tmp = initialized; \ > sys::MemoryFence(); \ > while (tmp != 2) { \ > tmp = initialized; \ > sys::MemoryFence(); \ > } \ > } \ > TsanHappensAfter(&initialized); > > > This code might have been added to please the old valgrind-based tsan (which did not understand atomics). > For the current tsan we should just implement proper synchronization and remove the annotations. > Why can't we use std::call_once here?Maybe some pre c++11 thing? I'll try to upgrade std::call_once.> > Looks like you are serious about running llvm under tsan. Very cool! > Does it make sense to add a tsan build to our sanitizer bot <http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast> (which already has asan, ubsan, lsan, and msan)? > What tests and what build mode would you suggest for the bot?I plan on adding a TSan bot on Green Dragon in the near future. My idea is to perform a ThinLTO bootstrap of clang (i.e. stage 1 being a Release+Assert+TSAN and stage2 Release-ThinLTO). This involves running the optimizer and the codegen in multiple threads. -- Mehdi> > --kcc > > > The failure looks like: > > =================> WARNING: ThreadSanitizer: data race (pid=17192) > Atomic write of size 4 at 0x0001113619e8 by thread T13: > #0 __tsan_atomic32_compare_exchange_val <null>:568340296 (libclang_rt.tsan_osx_dynamic.dylib+0x00000003e7aa) > #1 llvm::sys::CompareAndSwap(unsigned int volatile*, unsigned int, unsigned int) Atomic.cpp:52 (libLTO.dylib+0x000000511914) > #2 llvm::initializeSimpleInlinerPass(llvm::PassRegistry&) InlineSimple.cpp:82 (libLTO.dylib+0x000000ab2b55) > #3 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:50 (libLTO.dylib+0x000000ab2e8e) > #4 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:49 (libLTO.dylib+0x000000ab2d19) > #5 llvm::createFunctionInliningPass() InlineSimple.cpp:85 (libLTO.dylib+0x000000ab2ce3) > ... > > Previous read of size 4 at 0x0001113619e8 by thread T14: > #0 llvm::initializeSimpleInlinerPass(llvm::PassRegistry&) InlineSimple.cpp:82 (libLTO.dylib+0x000000ab2b65) > #1 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:50 (libLTO.dylib+0x000000ab2e8e) > #2 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:49 (libLTO.dylib+0x000000ab2d19) > #3 llvm::createFunctionInliningPass() InlineSimple.cpp:85 (libLTO.dylib+0x000000ab2ce3) > ... > Location is global 'llvm::initializeSimpleInlinerPass(llvm::PassRegistry&)::initialized' at 0x0001113619e8 (libLTO.dylib+0x0000016389e8) > > > Thanks! > > Mehdi > > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160418/abc82446/attachment.html>