search for: vptr

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

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). &gt...

...ifier was to support > cases where the "invariant" region's value changes and then changes back > (remember that invariant.group does not imply the storage doesn't change, > just that a particular set of loads and stores witness the same value): > > struct A { void *vptr; /*...*/ }; > struct B { void *vptr; /*...*/ }; > union U { A a; B b; }; > > void f(U *u) { > // #1, load A vptr > load u->a.vptr, !invariant "A::vptr" > // #2, change union member to B ... > store u->b.vptr, !invariant "B::vptr" > // #...

On Mon, Mar 19, 2012 at 2:52 PM, Kostya Serebryany <kcc at google.com> 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_o...

...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_...

I'm looking into how we can improve devirtualization in clang, and there a language in C++ feature I'd like to take advantage of which would let us perform elimination of more vptr loads. In this code: Cls *p = new Cls; p->virtual_method1(); p->method_changing_vptr(); // uses placement new to legally change the vptr p->virtual_method2(); // invalid! Cls *q = p; q->virtual_method2(); // this must get a new vptr lookup. there is no need to reload p&...

...>> cases where the "invariant" region's value changes and then changes back >> (remember that invariant.group does not imply the storage doesn't change, >> just that a particular set of loads and stores witness the same value): >> >> struct A { void *vptr; /*...*/ }; >> struct B { void *vptr; /*...*/ }; >> union U { A a; B b; }; >> >> void f(U *u) { >> // #1, load A vptr >> load u->a.vptr, !invariant "A::vptr" >> // #2, change union member to B ... >> store u->b.vptr, !invariant...

...n 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_o...

...i32 %call, 0 br i1 %tobool, label %while.end.loopexit, label %while.body while.end.loopexit: ; preds = %while.body br label %while.end while.end: ; preds = %while.end.loopexit, %entry ret void } We know that the load of vptr and virtual function will not change in this loop, which is indicated by !invariant.group. Hoisting invariant.group load is legal because %b is dereferenceable, but hoisting next load is illegal by LICM because it can't prove that %vtable is dereferenceable. But if I add dereferenceable metadat...

...which is being destroyed strikes me as a construct nobody would intentionally write. > We could, as an implementation, decide to emit these as relaxed atomic > stores, making the code well defined without changing the semantics (or > optimization) in any meaningful way, right? Making all vptr loads and stores atomic would block some optimizations (specifically, we can't perform certain optimizations involving memcpy, and IIRC some optimizers have incomplete atomics handling). Not sure if it would have much practical impact, though. Specifically just making vptr stores in destructor...

...me as a construct nobody > would intentionally write. > > > We could, as an implementation, decide to emit these as relaxed atomic > > stores, making the code well defined without changing the semantics (or > > optimization) in any meaningful way, right? > > Making all vptr loads and stores atomic would block some optimizations > .. and will not solve my problem -- I still need to distinguish between "benign-for-practical-purposes" and "definitely-harmful" vptr races. The only difference between those two cases lies outside of the instrumented...

..." region's value changes >> and then changes back (remember that invariant.group does not >> imply the storage doesn't change, just that a particular set >> of loads and stores witness the same value): >> >> struct A { void *vptr; /*...*/ }; >> struct B { void *vptr; /*...*/ }; >> union U { A a; B b; }; >> >> void f(U *u) { >> // #1, load A vptr >> load u->a.vptr, !invariant "A::vptr" >> // #2, change union member to...

Hi Piotr, I think makes sense. Modulo bitcasts, the invariant is identified by a particular pointer SSA value. Given that you can't sensibly have two nonequivalent invariants associated with the same pointer SSA value simultaneously, there's no need to also identify the invariant with a metadata string as well. When we need a new "identifier" for the pointed-to value, we

...gt; > > > On the other hand, after performing my LICM of !invariant.group load, GVN > > hoists the second load and I am not sure why it is legal then. > > I suspect what's going on is that we first canonicalize the loop to: > > if (precondition) { > do { > vptr = load vtable; > fptr = *vptr; > ... > } while (backedge_condition); > } > > after which it is safe to transform the program to (modulo aliasing): > > if (precondition) { > vptr = load vtable; > fptr = *vptr; > do { > ... > } while (backe...

On Mon, Mar 19, 2012 at 4:46 PM, Eli Friedman <eli.friedman at gmail.com>wrote: > 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. > I users expect this to "just work", why not extend the language and make it just work? We could, as

On May 9, 2013, at 6:34 PM, Nick Lewycky <nlewycky at google.com> wrote: > I'm looking into how we can improve devirtualization in clang, and there a language in C++ feature I'd like to take advantage of which would let us perform elimination of more vptr loads. In this code: > > Cls *p = new Cls; > p->virtual_method1(); > p->method_changing_vptr(); // uses placement new to legally change the vptr > p->virtual_method2(); // invalid! > Cls *q = p; > q->virtual_method2(); // this must get a new vptr loo...

...apple.com> wrote: > On May 9, 2013, at 6:34 PM, Nick Lewycky <nlewycky at google.com> wrote: > > I'm looking into how we can improve devirtualization in clang, and there a language in C++ feature I'd like to take advantage of which would let us perform elimination of more vptr loads. In this code: > > > > Cls *p = new Cls; > > p->virtual_method1(); > > p->method_changing_vptr(); // uses placement new to legally change the vptr > > p->virtual_method2(); // invalid! > > Cls *q = p; > > q->virtual_method2()...

...gt; wrote: > On May 9, 2013, at 6:34 PM, Nick Lewycky <nlewycky at google.com> wrote: > > I'm looking into how we can improve devirtualization in clang, and there > a language in C++ feature I'd like to take advantage of which would let us > perform elimination of more vptr loads. In this code: > > > > Cls *p = new Cls; > > p->virtual_method1(); > > p->method_changing_vptr(); // uses placement new to legally change > the vptr > > p->virtual_method2(); // invalid! > > Cls *q = p; > > q->virtual_meth...

...if ( CallInst *ci = dyn_cast<CallInst>(Inst) ){ ci->dump(); for(int i = 0; i < ci->getNumArgOperands(); i++){ Value *v = ci->getArgOperand(i); if (GetElementPtrInst *vi = dyn_cast<GetElementPtrInst>(v)){ Value *vPtr = vi->getPointerOperand(); vPtr->dump(); if ( AllocaInst *allo = dyn_cast<AllocaInst>(vPtr) ) { const Type *t = allo->getAllocatedType(); if ( const ArrayType *at = dyn_cast<ArrayType>(t) ) { int64_t...

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