Prazek marked 10 inline comments as done.
Prazek added inline comments.

================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1647
+      }
+    }
+
----------------
rsmith wrote:
> Prazek wrote:
> > rjmccall wrote:
> > > Prazek wrote:
> > > > rjmccall wrote:
> > > > > Incidentally, how do you protect against code like this?
> > > > > 
> > > > >   A *ptr;
> > > > >   reinterpret_cast<B *&>(ptr) = new B();
> > > > >   ptr->foo();
> > > > > 
> > > > > Presumably there needs to be a launder/strip here, but I guess it 
> > > > > would have to be introduced by the middle-end when forwarding the 
> > > > > store?  The way I've written this is an aliasing violation, but (1) I 
> > > > > assume your pass isn't disabled whenever strict-aliasing is disabled 
> > > > > and (2) you can do this with a memcpy and still pretty reliably 
> > > > > expect that LLVM will be able to eventually forward the store.
> > > > Can you add more info on what is A and B so I can make sure I 
> > > > understand it correctly?
> > > > Is the prefix of the layout the same for both, but they are not in the 
> > > > hierarchy?
> > > > 
> > > > I haven't thought about the strict aliasing. I think the only sane way 
> > > > would be to require strict aliasing for the strict vtable pointers.
> > > It's whatever case you're worried about such that you have to launder 
> > > member accesses and bitcasts.
> > > 
> > > And like I mentioned, relying on strict aliasing isn't enough because you 
> > > can do it legally with memcpy.  Maybe it's okay to consider it UB?  I'm 
> > > not sure about that.
> > AFAIK reinterpreting one class as another is UB if they are not in 
> > hierarchy (especially calling virtual function on reinterpreted class), not 
> > sure if strict aliasing should allow it anyway (if it would be a hand 
> > written custom vptr then it should be ok with strict aliasing turned off, 
> > but with vptr I don't think it is legal).
> > @rsmith Can you comment on that?
> OK, here's how I think about what we're doing here:
> 
> We view the IR-level pointer value for a pointer to a dynamic class type as 
> being a fat pointer, containing the actual pointer value and also a tag 
> indicating the dynamic type of the object (only notionally, though -- the 
> actual bit representation of the pointer doesn't include the extra 
> information, but we don't ever emit IR that inspects the bit representation 
> of the fat pointer to avoid exposing that fact). In that model, if you try to 
> type pun between a pointer to a dynamic class type and a pointer to a 
> non-dynamic-class type, that can't be expected to work because the (notional) 
> value is different, much as type punning between a derived and base class 
> pointer wouldn't work for a pointer to something other than a base class at 
> offset zero.
> 
> I think @rjmccall's example is OK, because both `A` and `B` would need to be 
> dynamic class types in a hierarchy to work, and that means we'd be using the 
> same notional pointer representation. A slight variation of that example:
> 
> ```
> struct A {};
> struct B : A { virtual void f(); };
> struct C : B { void f(); } c;
> A *p = &c;
> B *q;
> memcpy(&q, &p, sizeof(B*)); // or q = std::bit_cast<B*>(p);
> q->f();
> ```
> 
> ... would be UB, because the representation of an `A*` and a `B*` are 
> different (a `B*` contains a tag and an `A*` does not).
Does this answer satisfy you John? Can I push it to trunk?


Repository:
  rL LLVM

https://reviews.llvm.org/D47299



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to