[PATCH] D47299: [CodeGenCXX] Emit strip.invariant.group with -fstrict-vtable-pointers

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1623
+      const CXXRecordDecl *SourceClassDecl =
+          E->getType().getTypePtr()->getPointeeCXXRecordDecl();
+      const CXXRecordDecl *DstClassDecl = DestTy->getPointeeCXXRecordDecl();
----------------
Prazek wrote:
> rjmccall wrote:
> > Unnecessary `getTypePtr()`.
> getType returns QualType and it does not have getPointeeCXXRecordDecl. Am I missing something?
`operator->` on `QualType` drills to the `Type*`.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1647
+      }
+    }
+
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D47299