[llvm-dev] [TSAN] LLVM statistics and pass initialization trigger race detection

[Kostya Serebryany via llvm-dev]

+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>