<div dir="ltr">+dvyukov<div><br></div><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 15, 2016 at 10:24 PM, Mehdi Amini via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word">Hello,<div><br></div><div>I trying TSAN on Darwin on LLVM itself (sanitizing multi-threaded ThinLTO link).</div><div>However I see two main issues on my debug build: </div><div><br></div><div>1) Statistics: the pre/post increment is not safe, it seems to be acknowledge in the code itself:</div><div><br></div><div><div style="margin:0px;font-size:11px;line-height:normal;font-family:menlo;color:rgb(0,143,0)"><span style="color:rgb(0,0,0)">    </span><span>// FIXME: This function and all those that follow carefully use an</span></div><div style="margin:0px;font-size:11px;line-height:normal;font-family:menlo;color:rgb(0,143,0)"><span style="color:rgb(0,0,0)">    </span><span>// atomic operation to update the value safely in the presence of</span></div><div style="margin:0px;font-size:11px;line-height:normal;font-family:menlo;color:rgb(0,143,0)"><span style="color:rgb(0,0,0)">    </span><span>// concurrent accesses, but not to read the return value, so the</span></div><div style="margin:0px;font-size:11px;line-height:normal;font-family:menlo;color:rgb(0,143,0)"><span style="color:rgb(0,0,0)">    </span><span>// return value is not thread safe.</span></div></div><div><span><br></span></div><div><span>Can we tell TSAN to ignore the statistics somehow? Alternatively, is there a fix someone can suggest?</span></div></div></blockquote><div><br></div><div>It's better to add proper synchronization, most likely an atomic read with memory_order_relaxed is enough. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><div><span><br></span></div><div>2) Pass initialization. This macro does not please TSAN (even though it seems written with TSAN in mind):</div><div><br></div><div>#define CALL_ONCE_INITIALIZATION(function) \<br>  static volatile sys::cas_flag initialized = 0; \<br>  sys::cas_flag old_val = sys::CompareAndSwap(&initialized, 1, 0); \<br>  if (old_val == 0) { \<br>    function(Registry); \<br>    sys::MemoryFence(); \<br>    TsanIgnoreWritesBegin(); \<br>    TsanHappensBefore(&initialized); \<br>    initialized = 2; \<br>    TsanIgnoreWritesEnd(); \<br>  } else { \<br>    sys::cas_flag tmp = initialized; \<br>    sys::MemoryFence(); \<br>    while (tmp != 2) { \<br>      tmp = initialized; \<br>      sys::MemoryFence(); \<br>    } \<br>  } \<br>  TsanHappensAfter(&initialized);</div></div></blockquote><div><br></div><div><br></div><div>This code might have been added to please the old valgrind-based tsan (which did not understand atomics). <br></div><div>For the current tsan we should just implement proper synchronization and remove the annotations. </div><div>Why can't we use std::call_once here? </div><div><br></div><div>Looks like you are serious about running llvm under tsan. Very cool! </div><div>Does it make sense to add a tsan build to our <a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast">sanitizer bot</a> (which already has asan, ubsan, lsan, and msan)? </div><div>What tests and what build mode would you suggest for the bot? </div><div><br></div><div>--kcc </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><div><br></div><div>The failure looks like:</div><div><br></div><div>==================<br>WARNING: ThreadSanitizer: data race (pid=17192)<br>  Atomic write of size 4 at 0x0001113619e8 by thread T13:<br>    #0 __tsan_atomic32_compare_exchange_val <null>:568340296 (libclang_rt.tsan_osx_dynamic.dylib+0x00000003e7aa)<br>    #1 llvm::sys::CompareAndSwap(unsigned int volatile*, unsigned int, unsigned int) Atomic.cpp:52 (libLTO.dylib+0x000000511914)<br>    #2 llvm::initializeSimpleInlinerPass(llvm::PassRegistry&) InlineSimple.cpp:82 (libLTO.dylib+0x000000ab2b55)<br>    #3 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:50 (libLTO.dylib+0x000000ab2e8e)<br>    #4 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:49 (libLTO.dylib+0x000000ab2d19)<br>    #5 llvm::createFunctionInliningPass() InlineSimple.cpp:85 (libLTO.dylib+0x000000ab2ce3)</div><div>...</div><div><br>  Previous read of size 4 at 0x0001113619e8 by thread T14:<br>    #0 llvm::initializeSimpleInlinerPass(llvm::PassRegistry&) InlineSimple.cpp:82 (libLTO.dylib+0x000000ab2b65)<br>    #1 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:50 (libLTO.dylib+0x000000ab2e8e)<br>    #2 (anonymous namespace)::SimpleInliner::SimpleInliner() InlineSimple.cpp:49 (libLTO.dylib+0x000000ab2d19)<br>    #3 llvm::createFunctionInliningPass() InlineSimple.cpp:85 (libLTO.dylib+0x000000ab2ce3)<br>...</div><div>  Location is global 'llvm::initializeSimpleInlinerPass(llvm::PassRegistry&)::initialized' at 0x0001113619e8 (libLTO.dylib+0x0000016389e8)<br><br></div><div><br></div><div>Thanks!</div><span class="gmail-HOEnZb"><font color="#888888"><div><br></div><div>Mehdi</div><div><br></div><div><br></div></font></span></div><br>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br></blockquote></div><br></div></div>