<div class="gmail_quote"><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><div class="im"><blockquote type="cite"><div class="gmail_quote">
<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></blockquote><div><br></div>
</div><div>Yes please! :)</div><div class="im"><br></div></div></div></blockquote><div><br></div><div>Ok (once we find out what's the right way). </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><div class="im"><blockquote type="cite"><div class="gmail_quote">
<div>The AddressSafety attribute is checked at least in one place outside of asan code (lib/Analysis/MemoryDependenceAnalysis.cpp).</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></blockquote><br></div></div><div>I never agreed with this patch, but let it go in to avoid blocking forward progress.</div>
</div></blockquote><div><br></div><div>Thanks!</div><div>I agree that the patch is not perfect (all problems that you mentioned, especially with inlining). </div><div><br></div><div><shameless plug></div><div>BTW, this particular patch unblocked the progress for mozilla (e.g. <a href="http://www.mozilla.org/security/announce/2012/mfsa2012-14.html">http://www.mozilla.org/security/announce/2012/mfsa2012-14.html</a>)</div>
<div>and clang+asan bootstrap (found ~5 clang/llvm bugs already, more are in my queue)</div><div></shameless plug><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><br></div><div>However, now that you ask :), GVN should never have checked for a "asan" attribute.  If you want an attribute to disable specific transformations, we should design this properly, and it should be a property of the load or store, not a property of the function.  </div>
</div></blockquote><div><br></div><div>I don't see how to attach the property to loads/stores.</div><div>If we use instruction-level metadata, it may be lost (as you just explained) above. </div><div>If we add an instruction-level attribute (are there such?), what value should be used if some phase creates a new load/store? </div>
<div>Again, what should be done when some code is copied (inlined, unrolled, etc)?</div><div><br></div><div> </div><div>--kcc  </div></div>