<br><br><div class="gmail_quote">On Thu, Mar 15, 2012 at 2:07 AM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com">clattner@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div class="im"><br><div><div>On Mar 14, 2012, at 6:08 PM, Kostya Serebryany wrote:</div><br><blockquote type="cite">Hello, <div><br></div><div>I would like to add one more function attribute: "ThreadSafety".</div>
<div>Functions with this attribute will be instrumented by run-time thread-safety analysis tools (e.g. with ThreadSanitizer).</div>
<div>This is similar to "AddressSafety" attribute used by AddressSanitizer (and SAFECode?).</div><div><br></div>Clang already has the attribute "__attribute__((no_thread_safety_analysis))" which is used to disable static thread-safety analysis. <div>

<a href="http://clang.llvm.org/docs/LanguageExtensions.html#threadsafety" target="_blank">http://clang.llvm.org/docs/LanguageExtensions.html#threadsafety</a><br></div><div>In a separate change we will teach clang to unset ThreadSafety when no_thread_safety_analysis is present. </div>

<div>(exactly as we do for no_address_safety_analysis and AddressSafety)</div></blockquote><br></div></div><div>Hi Kostya,</div><div><br></div><div>I'm skeptical of adding LLVM IR function attributes for each instrumentation pass.  Have you investigated using module flags metadata to track this information?  </div>
</div></blockquote><div><br></div><div>I didn't, thanks for the link. </div><div>It looks like this would work too, although an attribute still sounds more compelling to me.</div><div>It's just one bit (so it is free, as long as you have spare bits).</div>
<div>Do you dislike it because there are just too few Attribute bits available (31 left), or for other reasons? </div><div><br></div><div>My (perhaps incorrect) understanding of the metadata system is that if the metadata is removed, the correctness of the code is not affected.</div>
<div>In this case, loosing metadata will mean introducing incorrect instrumentation.  </div><div><br></div><div>Also, if I use metadata for tsan and an attribute for asan, it will look weird. </div><div>Would you suggest to use metadata for asan as well (instead of AddressSafety attribute)? </div>
<div><br></div><div>The AddressSafety attribute is checked at least in one place outside of asan code (lib/Analysis/MemoryDependenceAnalysis.cpp).<br></div><div>The ThreadSafety attribute (or equivalent metadata) will need to be checked in the same place too:<br>
</div><div>struct X {</div><div>   uint64_t for_alignment;</div><div>   char a, b, c, d;</div><div>};</div><div><br></div><div>Thread1(X *x) {</div><div>   ... = X.a + X.d</div><div>}</div><div><br></div><div>Thread2(X *x) {</div>
<div>   X.b = ...</div><div>}<br></div><div><br></div><div>If GVN widens X.a + X.d, tsan will start reporting a non-existing race. </div><div><br></div><div>Thanks, <br></div><div><br></div><div>--kcc </div><div><br></div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>It is designed to be extensible for these sorts of uses.</div><div><br>
</div><div><a href="http://llvm.org/docs/LangRef.html#module_flags" target="_blank">http://llvm.org/docs/LangRef.html#module_flags</a></div></div></blockquote><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><span class="HOEnZb"><font color="#888888"><div><br></div><div>-Chris</div></font></span></div>
</blockquote></div><br>