diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp index 655fc3dc954c81..00f21e393a27f3 100644 --- a/clang/lib/CodeGen/CGExprConstant.cpp +++ b/clang/lib/CodeGen/CGExprConstant.cpp @@ -733,6 +733,7 @@ bool ConstStructBuilder::Build(const InitListExpr *ILE, bool AllowOverwrite) { for (FieldDecl *Field : RD->fields()) { ++FieldNo; + QualType FieldTy = Field->getType(); // If this is a union, skip all the fields that aren't being initialized. if (RD->isUnion() && @@ -773,23 +774,23 @@ bool ConstStructBuilder::Build(const InitListExpr *ILE, bool AllowOverwrite) { // represents additional overwriting of our current constant value, and not // a new constant to emit independently. if (AllowOverwrite && - (Field->getType()->isArrayType() || Field->getType()->isRecordType())) { + (FieldTy->isArrayType() || FieldTy->isRecordType())) { if (auto *SubILE = dyn_cast(Init)) { CharUnits Offset = CGM.getContext().toCharUnitsFromBits( Layout.getFieldOffset(FieldNo)); if (!EmitDesignatedInitUpdater(Emitter, Builder, StartOffset + Offset, - Field->getType(), SubILE)) + FieldTy, SubILE)) return false; // If we split apart the field's value, try to collapse it down to a // single value now. Builder.condense(StartOffset + Offset, - CGM.getTypes().ConvertTypeForMem(Field->getType())); + CGM.getTypes().ConvertTypeForMem(FieldTy)); continue; } } llvm::Constant *EltInit = - Init ? Emitter.tryEmitPrivateForMemory(Init, Field->getType()) + Init ? Emitter.tryEmitPrivateForMemory(Init, FieldTy) : Emitter.emitNullForMemory(Field->getType()); if (!EltInit) return false; @@ -803,10 +804,16 @@ bool ConstStructBuilder::Build(const InitListExpr *ILE, bool AllowOverwrite) { if (!AppendField(Field, Layout.getFieldOffset(FieldNo), EltInit, AllowOverwrite)) return false; - // After emitting a non-empty field with [[no_unique_address]], we may - // need to overwrite its tail padding. - if (Field->hasAttr()) - AllowOverwrite = true; + + // Allow overwrites after a field with tail padding. This allows + // overwriting tail padding of fields carrying [[no_unique_address]] + // without checking for it, since it is not necessarily present in debug + // info. + if (const CXXRecordDecl *FieldRD = FieldTy->getAsCXXRecordDecl()) { + const ASTRecordLayout &Layout = CGM.getContext().getASTRecordLayout(FieldRD); + if (Layout.getDataSize() < Layout.getSize()) + AllowOverwrite = true; + } } else { // Otherwise we have a bitfield. if (!AppendBitField(Field, Layout.getFieldOffset(FieldNo), EltInit, diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp index ea44e6f21f3c86..9b36ff4e0beadb 100644 --- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp +++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp @@ -198,7 +198,7 @@ struct CGRecordLowering { const CXXRecordDecl *Query) const; void calculateZeroInit(); CharUnits calculateTailClippingOffset(bool isNonVirtualBaseType) const; - void checkBitfieldClipping(bool isNonVirtualBaseType) const; + void checkForOverlap(bool isNonVirtualBaseType); /// Determines if we need a packed llvm struct. void determinePacked(bool NVBaseType); /// Inserts padding everywhere it's needed. @@ -300,7 +300,7 @@ void CGRecordLowering::lower(bool NVBaseType) { accumulateVBases(); } llvm::stable_sort(Members); - checkBitfieldClipping(NVBaseType); + checkForOverlap(NVBaseType); Members.push_back(StorageInfo(Size, getIntNType(8))); determinePacked(NVBaseType); insertPadding(); @@ -389,14 +389,28 @@ void CGRecordLowering::accumulateFields(bool isNonVirtualBaseType) { // Empty fields have no storage. ++Field; } else { - // Use base subobject layout for the potentially-overlapping field, - // as it is done in RecordLayoutBuilder - Members.push_back(MemberInfo( - bitsToCharUnits(getFieldBitOffset(*Field)), MemberInfo::Field, - Field->isPotentiallyOverlapping() - ? getStorageType(Field->getType()->getAsCXXRecordDecl()) - : getStorageType(*Field), - *Field)); + CharUnits CurOffset = bitsToCharUnits(getFieldBitOffset(*Field)); + llvm::Type *StorageType = getStorageType(*Field); + + // Detect cases when the next field needs to be packed into tail padding + // of a record field. This is typically caused by [[no_unique_address]], + // but we try to infer when that is the case rather than checking for the + // attribute explicitly because the attribute is typically not present in + // debug info. Use the base subobject LLVM struct type in these cases, + // which will be less than data size bytes. + if (const CXXRecordDecl *FieldRD = + Field->getType()->getAsCXXRecordDecl()) { + CharUnits NextOffset = Layout.getNonVirtualSize(); + auto NextField = std::next(Field); + if (NextField != FieldEnd) + NextOffset = bitsToCharUnits(getFieldBitOffset(*NextField)); + if (CurOffset + getSize(StorageType) > NextOffset) { + StorageType = getStorageType(FieldRD); + } + } + + Members.push_back( + MemberInfo(CurOffset, MemberInfo::Field, StorageType, *Field)); ++Field; } } @@ -948,19 +962,19 @@ void CGRecordLowering::calculateZeroInit() { } // Verify accumulateBitfields computed the correct storage representations. -void CGRecordLowering::checkBitfieldClipping(bool IsNonVirtualBaseType) const { +void CGRecordLowering::checkForOverlap(bool IsNonVirtualBaseType) { #ifndef NDEBUG auto ScissorOffset = calculateTailClippingOffset(IsNonVirtualBaseType); auto Tail = CharUnits::Zero(); - for (const auto &M : Members) { + for (MemberInfo &M : Members) { // Only members with data could possibly overlap. if (!M.Data) continue; - assert(M.Offset >= Tail && "Bitfield access unit is not clipped"); + assert(M.Offset >= Tail && "LLVM struct member types overlap"); Tail = M.Offset + getSize(M.Data); assert((Tail <= ScissorOffset || M.Offset >= ScissorOffset) && - "Bitfield straddles scissor offset"); + "Member straddles scissor offset"); } #endif } diff --git a/clang/test/CodeGenCXX/no-unique-address-3.cpp b/clang/test/CodeGenCXX/no-unique-address-3.cpp index c1e84f1d312768..17480030ef7ba2 100644 --- a/clang/test/CodeGenCXX/no-unique-address-3.cpp +++ b/clang/test/CodeGenCXX/no-unique-address-3.cpp @@ -65,8 +65,8 @@ Bar J; // CHECK-NEXT: | [sizeof=8, dsize=8, align=4, // CHECK-NEXT: | nvsize=8, nvalign=4] -// CHECK-LABEL: LLVMType:%class.IntFieldClass = type { [2 x i8], %class.Second.base, i32 } -// CHECK-NEXT: NonVirtualBaseLLVMType:%class.IntFieldClass = type { [2 x i8], %class.Second.base, i32 } +// CHECK-LABEL: LLVMType:%class.IntFieldClass = type { [2 x i8], %class.Second, i32 } +// CHECK-NEXT: NonVirtualBaseLLVMType:%class.IntFieldClass = type { [2 x i8], %class.Second, i32 } class IntFieldClass : Empty { [[no_unique_address]] Second Field; int C; diff --git a/clang/test/CodeGenCXX/override-layout.cpp b/clang/test/CodeGenCXX/override-layout.cpp index 07cd31580726e3..de5c18966688f0 100644 --- a/clang/test/CodeGenCXX/override-layout.cpp +++ b/clang/test/CodeGenCXX/override-layout.cpp @@ -1,15 +1,19 @@ // RUN: %clang_cc1 %std_cxx98-14 -w -fdump-record-layouts-simple %s > %t.layouts // RUN: %clang_cc1 %std_cxx98-14 -w -fdump-record-layouts-simple %s > %t.before -// RUN: %clang_cc1 %std_cxx98-14 -w -DPACKED= -DALIGNED16= -fdump-record-layouts-simple -foverride-record-layout=%t.layouts %s > %t.after +// RUN: %clang_cc1 %std_cxx98-14 -w -DPACKED= -DALIGNED16= -DNO_UNIQUE_ADDRESS= -fdump-record-layouts-simple -foverride-record-layout=%t.layouts %s > %t.after // RUN: diff -u %t.before %t.after // RUN: FileCheck --check-prefixes=CHECK,PRE17 %s < %t.after // RUN: %clang_cc1 -std=c++17 -w -fdump-record-layouts-simple %s > %t.layouts // RUN: %clang_cc1 -std=c++17 -w -fdump-record-layouts-simple %s > %t.before -// RUN: %clang_cc1 -std=c++17 -w -DPACKED= -DALIGNED16= -fdump-record-layouts-simple -foverride-record-layout=%t.layouts %s > %t.after +// RUN: %clang_cc1 -std=c++17 -w -DPACKED= -DALIGNED16= -DNO_UNIQUE_ADDRESS= -fdump-record-layouts-simple -foverride-record-layout=%t.layouts %s > %t.after // RUN: diff -u %t.before %t.after // RUN: FileCheck --check-prefixes=CHECK,CXX17 %s < %t.after +// Generate IR with the externally defined layout to exercise IR generation the +// way LLDB does. +// RUN: %clang_cc1 -std=c++17 -w -DPACKED= -DALIGNED16= -DNO_UNIQUE_ADDRESS= -foverride-record-layout=%t.layouts %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-IR + // CXX17: Type: struct X6 // If not explicitly disabled, set PACKED to the packed attribute. @@ -102,3 +106,43 @@ void use_structs() { x4s[1].a = 1; x5s[1].a = 17; } + +// Test various uses of no_unique_address. +#ifndef NO_UNIQUE_ADDRESS +#define NO_UNIQUE_ADDRESS [[no_unique_address]] +#endif + +struct HasTailPadding { + HasTailPadding() = default; + constexpr HasTailPadding(int x, short y) : x(x), y(y) {} +private: + int x; + short y; +}; +static_assert(sizeof(HasTailPadding) == 8); +// CHECK: Type: struct HasTailPadding + +struct NoTailPacking { + HasTailPadding xy; + char z; +}; +static_assert(sizeof(NoTailPacking) == 12); +// CHECK: Type: struct NoTailPacking + +struct NUAUsesTailPadding { + NO_UNIQUE_ADDRESS HasTailPadding xy; + NO_UNIQUE_ADDRESS char z; +}; + +// CHECK: Type: struct NUAUsesTailPadding +static_assert(sizeof(NUAUsesTailPadding) == 8); + +NUAUsesTailPadding nua_gv_zeroinit; +NUAUsesTailPadding nua_gv_braces{}; +NUAUsesTailPadding nua_gv_123{{1, 2}, 3}; +// CHECK-IR: %struct.NUAUsesTailPadding = type { %struct.HasTailPadding.base, i8, i8 } +// CHECK-IR: %struct.HasTailPadding.base = type <{ i32, i16 }> + +// CHECK-IR: @nua_gv_zeroinit = global %struct.NUAUsesTailPadding zeroinitializer, align 4 +// CHECK-IR: @nua_gv_braces = global { [6 x i8], i8, [1 x i8] } zeroinitializer, align 4 +// CHECK-IR: @nua_gv_123 = global { i32, i16, i8 } { i32 1, i16 2, i8 3 }, align 4