[no_unique_address] Member not stored in previous member's padding

[zoecarver]

Bugzilla Link	51424
Version	trunk
OS	All
CC	@dwblaikie,@zygoloid

Extended Description

It seems like a missed optimization not to store members in a previous member's padding when the previous member is marked as [[no_unique_address]].

GCC implements this. The difference in codegen can be seen here: https://godbolt.org/z/zGs97xKG7

[zygoloid]

Slightly further reduced:

struct A {
int *p;
[[no_unique_address]] char a;
};

struct B {
[[no_unique_address]] A cache;
char b;
};

static_assert(sizeof(B) == 16);
static_assert(alignof(B) == 8);

It looks like Clang doesn't properly implement the rule that a class with a potentially-overlapping member is not POD for the purpose of layout. (If the attribute is removed from A::a, then struct A is POD for the purpose of layout and we're not allowed to reuse its tail padding per the ABI rule.)

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Zoe Carver (zoecarver)

| | | | --- | --- | | Bugzilla Link | [51424](https://llvm.org/bz51424) | | Version | trunk | | OS | All | | CC | @dwblaikie,@zygoloid |

Extended Description

It seems like a missed optimization not to store members in a previous member's padding when the previous member is marked as [[no_unique_address]].

GCC implements this. The difference in codegen can be seen here: https://godbolt.org/z/zGs97xKG7

[SainoNamkho]

Slightly further reduced:

struct A { int *p; [[no_unique_address]] char a; };

struct B { [[no_unique_address]] A cache; char b; };

static_assert(sizeof(B) == 16); static_assert(alignof(B) == 8);

It looks like Clang doesn't properly implement the rule that a class with a potentially-overlapping member is not POD for the purpose of layout. (If the attribute is removed from A::a, then struct A is POD for the purpose of layout and we're not allowed to reuse its tail padding per the ABI rule.)

I don't quite understand why sizeof(B) should be 16. As in https://itanium-cxx-abi.github.io/cxx-abi/abi.html#class-types:

1. After A::p, sizeof(A)=align(A)=dsize(A)=nvsize(A)=nvalign(A)=8
2. A::a is not an empty base class and is not an empty data member,
   • nvsize(A)=sizeof(A)=max(sizeof(A), offset(A::a)+max(nvsize(char), dsize(char)))=9
   • dsize(A)=offset(A::a)+max(nvsize(char), dsize(char))=9
   • nvalign(A)=align(A)=max(align(A),align(char))=8
3. Falls into IV Finalization,
   • sizeof(A)=ceil(sizeof(A)/align(A))*align(A)=16
   • A is a POD, but not a POD for the purpose of layout, dsize(A)=nvsize(A)=sizeof(A)=16
4. B::cache is not an empty base class and is not an empty data member,
   • nvsize(B)=sizeof(B)=max(sizeof(B), offset(B::cache)+max(nvsize(A), dsize(A)))=16
   • dsize(B)=offset(B::cache)+max(nvsize(A), dsize(A))=16
   • nvalign(B)=align(B)=max(1,align(A))=8
5. B::b is not an empty base class and is not an empty data member,
   • nvsize(B)=sizeof(B)=max(sizeof(B), offset(B::b)+sizeof(char))=17
   • dsize(B)=offset(B::b)+sizeof(char))=17
   • nvalign(B)=align(B)=max(align(B),align(char))=8
6. Falls into IV Finalization,
   • sizeof(B)=ceil(sizeof(B)/align(B))*align(B)=24
   • B is a POD, but not a POD for the purpose of layout, dsize(B)=nvsize(B)=sizeof(B)=24

A is still a POD, though not a POD for the purpose of layout, so is considered as having no tail-padding.

The cluase is strange, but explains https://godbolt.org/z/7jEnYGqYj

struct E {};
struct A
{
    struct X {
        E e;
        [[no_unique_address]] E f;
    } ;
    [[no_unique_address]] X x;
    [[no_unique_address]] char c;
};

static_assert(sizeof(A) == 3);

[zygoloid]

A is still a POD, though not a POD for the purpose of layout, so is considered as having no tail-padding.

I don't think that's clear. [[no_unique_address]] didn't exist in C++03, so it's not really meaningful to ask whether a class using it on a member would be POD using the C++03 rules. This seems like a bug / underspecification in the ABI -- [[no_unique_address]] on a member should make a type not-POD just like it makes it not-POD-for-the-purpose-of-layout.

[SainoNamkho]

A is still a POD, though not a POD for the purpose of layout, so is considered as having no tail-padding.

I don't think that's clear. [[no_unique_address]] didn't exist in C++03, so it's not really meaningful to ask whether a class using it on a member would be POD using the C++03 rules. This seems like a bug / underspecification in the ABI -- [[no_unique_address]] on a member should make a type not-POD just like it makes it not-POD-for-the-purpose-of-layout.

I agree with this. There should have been no difference with POD but not for the purpose of layout and non-POD in the itanium abi spec. May the abi spec should be updated before clang fixes.