[PATCH] D45306: PR36992 don't overwrite virtual bases in tail padding

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added a reviewer: rjmccall.
Herald added subscribers: cfe-commits, sanjoy.

When initializing a base class, we might accidentally overwrite a virtual base in its tail padding in some situations (in particular, when emitting a trivial copy constructor as a memcpy, we copy the full object size rather than only the dsize):

  struct A { char c; A(const A&) : c(1) {} };
  struct B { int n; char c[3]; ~B(); };
  struct C : B, virtual A {};
  C f(C c) { return c; }

(Here, Clang emits an 8-byte memcpy for the `B` base in `f`, overwriting the already-initialized `A` vbase.)

It's easy enough to prevent that from happening entirely, but we would still like to copy the padded-to-alignment size whenever we can (for example, see the tests in `test/CodeGenCXX/assign-construct-memcpy.cpp`). This patch adds a flag to `AggValueSlot` to indicate whether the tail padding of the aggregate might overlap some other object, and propagate that flag far enough that we can see it when emitting a trivial copy ctor call. When initializing a base class, we treat it as potentially-overlapping only if the base class lies partly outside the nvsize of the derived class -- otherwise, because we know non-virtual subobjects are initialized in address order, an overlarge store is valid.

This patch also lays some groundwork for P0840 <http://wg21.link/p0840>, which permits packing into the tail padding for fields marked with a new attribute, wherein the same situation can arise (a vbase can be constructed in the tail padding of the last field if it has the attribute):

  struct A { char c; A(const A&) : c(1) {} };
  struct B { int n; char c[3]; ~B(); };
  struct C : virtual A { [[no_unique_address]] B b; };
  C f(C c) { return c; }

(Copying a `C` object must not perform an 8-byte copy for the `B` subobject, even though it's a most-derived object, because its tail padding contains an `A`.)

It also prepares us for the possibility that WG21 might decide that guaranteed copy elision is supposed to apply to base class initialization (for classes without virtual bases), which would mean that we cannot assume that the tail padding of the return slot of a function has no overlap with other objects:

  struct A { char c; };
  struct B { int n; char c[3]; ~B(); };
  B f() { return /*...*/ }
  struct C : B, virtual A {
    C() : A{1}, B(f()) {}
  };

If `B(f())` is not permitted to make a copy, then `f()` cannot trample on its return slot's tail padding (which in this example might contain an already-initialized `A` object).


Repository:
  rC Clang

https://reviews.llvm.org/D45306

Files:
  CodeGen/CGAtomic.cpp
  CodeGen/CGBlocks.cpp
  CodeGen/CGCall.cpp
  CodeGen/CGClass.cpp
  CodeGen/CGDecl.cpp
  CodeGen/CGDeclCXX.cpp
  CodeGen/CGExpr.cpp
  CodeGen/CGExprAgg.cpp
  CodeGen/CGExprCXX.cpp
  CodeGen/CGObjC.cpp
  CodeGen/CGOpenMPRuntime.cpp
  CodeGen/CGStmt.cpp
  CodeGen/CGValue.h
  CodeGen/CodeGenFunction.h
  CodeGen/ItaniumCXXABI.cpp
  CodeGenCXX/tail-padding.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45306.141097.patch
Type: text/x-patch
Size: 38667 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180405/07a88ed8/attachment-0001.bin>