<br><br><div class="gmail_quote">On Mon, Mar 19, 2012 at 4:46 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Mon, Mar 19, 2012 at 4:30 PM, Chris Lattner <<a href="mailto:clattner@apple.com">clattner@apple.com</a>> wrote:<br>
><br>
> On Mar 19, 2012, at 2:52 PM, Kostya Serebryany wrote:<br>
><br>
> Hello,<br>
><br>
> While instrumenting LLVM IR in ThreadSanitizer (race detector), I need<br>
> to distinguish between a store to vtable pointer (vptr) and any other<br>
> regular store.<br>
> This special treatment should be limited to class DTORs, so I should also<br>
> know when a function is a DTOR.<br>
> Rationale: need to distinguish benign and harmful races on vptr<br>
> (<a href="http://code.google.com/p/data-race-test/wiki/PopularDataRaces#Data_race_on_vptr" target="_blank">http://code.google.com/p/data-race-test/wiki/PopularDataRaces#Data_race_on_vptr</a>).<br>
><br>
> Currently, I can figure out when a function is a DTOR and when a store<br>
> touches vptr by analyzing mangled names.<br>
> _ZN1BD1Ev=="B::~B()"<br>
> _ZTV1B=="vtable for B"<br>
><br>
> define linkonce_odr void @_ZN1BD1Ev(%struct.B* %this) unnamed_addr nounwind<br>
> uwtable align 2 {<br>
> entry:<br>
>   ....<br>
>   store i32 (...)** bitcast (i8** getelementptr inbounds ([5 x i8*]*<br>
> @_ZTV1B, i64 0, i64 2) to i32 (...)**), i32 (...)*** %0, align 8<br>
><br>
> However, this does not sound right.<br>
> What would be the right way to pass this information from clang to LLVM?<br>
> Will using metadata for this purpose be a right solution?<br>
> (insn-level metadata for vptr store and module-level metadata for DTORs)<br>
><br>
><br>
> Using instruction level metadata for this would be appropriate.  However, I<br>
> also don't understand why a race on this is truly benign.<br>
<br>
</div></div>It isn't, really; calling it "benign" is deceptive.</blockquote><div><br></div><div>Well, yes. Generally, I agree with you here. </div><div>But then there are tsan users who have all that legacy code and want to find races that will harm them for sure and don't want to see "noise".</div>
<div>These vptr races are hard to suppress w/o risking to hide some other races. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">  It's just that<br>

storing a pointer which is equal to the existing pointer stored at a<br>
given address almost always makes the optimizer/codegen generate code<br>
which can't trigger the race in a way which visibly misbehaves.<br>
Therefore, as a heuristic users apparently want ThreadSanitizer to<br>
ignore (or list separately) such races.<br></blockquote><div><br></div><div>Yep. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Given that, I'm not sure I really see the issue with just<br>
special-casing any store where the value stored is a pointer to a<br>
global... but it could be argued either way, I guess.<br></blockquote><div><br></div><div>That will hide too many real races, I afraid. </div><div>Including those "harmful" vptr races.</div><div><br></div><div>
<blockquote class="gmail_quote" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span style><br class="Apple-interchange-newline">
 I'm also concerned that you're adding even more knobs to clang and IR for special case situations.  How many more special cases like this are you going to require?</span></blockquote></div><div><span style><br></span></div>
<div><span style><br></span></div><div><span style>I don't remember more special cases off the top of my head. valgrind-based variant has this special case and nothing else, I believe.</span></div><div><span style>We've run our race detector unit tests (</span><a href="http://code.google.com/p/data-race-test/source/browse/trunk/unittest/racecheck_unittest.cc">http://code.google.com/p/data-race-test/source/browse/trunk/unittest/racecheck_unittest.cc</a><span style>)</span></div>
<div><span style>under the current </span><span style>LLVM-TSAN and this is the only thing we found so far.</span></div><div><span style>But we did not run anything heavy under LLVM-TSAN yet, so something else may be hiding from us. </span></div>
<div><span style><br></span></div><div><span style>--kcc </span></div></div>