[PATCH] D124571: Avoid strict aliasing violation on type punning inside llvm::PointerIntPair

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a subscriber: rsmith.
efriedma added a comment.

I think the placement new call needs to be in the operator=, not the constructor, to have any effect.

If we assign a value with the low bit set, the object must be implicitly destroyed, and the memcpy must implicitly create a new object: otherwise, we have an object with an invalid value, which I don't think is valid.  So the sequence "placement new, then memcpy a value with a low bit set, then memcpy a value without the low bit set, then call getPointerAddress()" doesn't work.  (Not that a compiler is likely to ever exploit that, but I'm pretty sure that's how the model works.)

On the other hand, if we placement new in the assignment operator, then it's fine if the memcpy implicitly destroys the object; we'll placement new again before we need it. The sequence "placement new, then memcpy a value with a low bit set, then placement new, then memcpy a value without the low bit set, then call getPointerAddress()" works fine.

----

That said, looking again, I don't think the whole dance with placement new is necessary, given the restrictions you're placing on the contained type.  A type which is trivially destructible and trivially movable must be an implicit-lifetime type.  From [class.prop]: "A class S is an implicit-lifetime class if it is an aggregate or has at least one trivial eligible constructor and a trivial, non-deleted destructor."  The type is allowed to have non-trivial constructors, as long as there's at least one trivial constructor (including a move constructor).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124571/new/

https://reviews.llvm.org/D124571