[LLVMdev] recognizing DTORs and vptr updates in LLVM.

Kostya Serebryany kcc at google.com
Mon Mar 19 17:01:46 PDT 2012


On Mon, Mar 19, 2012 at 4:46 PM, Eli Friedman <eli.friedman at gmail.com>wrote:

> On Mon, Mar 19, 2012 at 4:30 PM, Chris Lattner <clattner at apple.com> wrote:
> >
> > On Mar 19, 2012, at 2:52 PM, Kostya Serebryany wrote:
> >
> > Hello,
> >
> > While instrumenting LLVM IR in ThreadSanitizer (race detector), I need
> > to distinguish between a store to vtable pointer (vptr) and any other
> > regular store.
> > This special treatment should be limited to class DTORs, so I should also
> > know when a function is a DTOR.
> > Rationale: need to distinguish benign and harmful races on vptr
> > (
> http://code.google.com/p/data-race-test/wiki/PopularDataRaces#Data_race_on_vptr
> ).
> >
> > Currently, I can figure out when a function is a DTOR and when a store
> > touches vptr by analyzing mangled names.
> > _ZN1BD1Ev=="B::~B()"
> > _ZTV1B=="vtable for B"
> >
> > define linkonce_odr void @_ZN1BD1Ev(%struct.B* %this) unnamed_addr
> nounwind
> > uwtable align 2 {
> > entry:
> >   ....
> >   store i32 (...)** bitcast (i8** getelementptr inbounds ([5 x i8*]*
> > @_ZTV1B, i64 0, i64 2) to i32 (...)**), i32 (...)*** %0, align 8
> >
> > However, this does not sound right.
> > What would be the right way to pass this information from clang to LLVM?
> > Will using metadata for this purpose be a right solution?
> > (insn-level metadata for vptr store and module-level metadata for DTORs)
> >
> >
> > Using instruction level metadata for this would be appropriate.
>  However, I
> > also don't understand why a race on this is truly benign.
>
> It isn't, really; calling it "benign" is deceptive.


Well, yes. Generally, I agree with you here.
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".
These vptr races are hard to suppress w/o risking to hide some other races.


>  It's just that
> storing a pointer which is equal to the existing pointer stored at a
> given address almost always makes the optimizer/codegen generate code
> which can't trigger the race in a way which visibly misbehaves.
> Therefore, as a heuristic users apparently want ThreadSanitizer to
> ignore (or list separately) such races.
>

Yep.


>
> Given that, I'm not sure I really see the issue with just
> special-casing any store where the value stored is a pointer to a
> global... but it could be argued either way, I guess.
>

That will hide too many real races, I afraid.
Including those "harmful" vptr races.


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



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.
We've run our race detector unit tests (
http://code.google.com/p/data-race-test/source/browse/trunk/unittest/racecheck_unittest.cc
)
under the current LLVM-TSAN and this is the only thing we found so far.
But we did not run anything heavy under LLVM-TSAN yet, so something else
may be hiding from us.

--kcc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120319/b53259f5/attachment.html>


More information about the llvm-dev mailing list