[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

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

rsmith added a comment.

In D99790#2681487 <https://reviews.llvm.org/D99790#2681487>, @lebedev.ri wrote:

> I think it should be fixed in `CodeGenVTables::maybeEmitThunk()`, likely in `arrangeGlobalDeclaration()`.
> Or is this wrong on the AST level, too?

The AST-level `ThunkInfo` doesn't track the `this` parameter type at all, so I think this is probably a CodeGen-specific concern. I think this is a bug in `maybeEmitThunk`:

  const CGFunctionInfo &FnInfo =
      IsUnprototyped ? CGM.getTypes().arrangeUnprototypedMustTailThunk(MD)
                     : CGM.getTypes().arrangeGlobalDeclaration(GD);

The problem here is that `GD` refers to the derived-class declaration (which takes a `Derived*`) but what we actually want is the base class declaration corresponding to the vtable entry for which we're trying to emit a thunk. So I think we want to find the class that introduced the vtable slot, look up the corresponding function in that class (`CXXRecordDecl::getCorrespondingMethodInClass(ForVTable, true)`), and arrange a declaration of *that* function.

I don't think there's an easy way to find the vtable slot owner from `addVTableComponent`, so perhaps the easiest thing to do would be to track either the effective `this` class or that class's corresponding method declaration in `ThunkInfo`.

Note that this is also wrong for covariant return types. For example:

  struct A {};
  struct alignas(32) B : virtual A { char c[32]; };
  struct Pad { char c[7]; };
  struct C : B, Pad, virtual A {};
  
  struct X { virtual A &f(); };
  struct U { virtual ~U(); };
  C c;
  struct Y : U, X {
      virtual B &f() override { return c; }
  };
  
  Y y;

... generates a thunk that claims its return value is `align 32 dereferenceable(40) %struct.B*`, which is nonsense; the return value should be `align(1) dereferenceable(1)` because (as happens in this example) the `A` virtual base subobject could be at a different offset and and alignment from the `B` object. Using `X::f` instead of `Y::f` as the method whose declaration we arrange should fix that too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99790