https://github.com/amykhuang updated https://github.com/llvm/llvm-project/pull/65675:
>From 923a43cd6386f6e57023fd8928eed0dc0ab04d57 Mon Sep 17 00:00:00 2001 From: Amy Huang <akhu...@google.com> Date: Fri, 21 Jul 2023 16:30:30 -0700 Subject: [PATCH 1/2] Implement [[msvc::no_unique_address]] This attribute should match the behavior of MSVC's [[msvc::no_unique_address]] attribute. Bug: https://github.com/llvm/llvm-project/issues/49358 Differential Revision: https://reviews.llvm.org/D157762 --- clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Basic/AttrDocs.td | 4 + clang/lib/AST/Decl.cpp | 8 + clang/lib/AST/RecordLayoutBuilder.cpp | 191 +++++++++-- clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 1 + clang/lib/Sema/SemaDeclAttr.cpp | 17 + clang/test/Layout/ms-no-unique-address.cpp | 338 ++++++++++++++++++++ 7 files changed, 540 insertions(+), 27 deletions(-) create mode 100644 clang/test/Layout/ms-no-unique-address.cpp diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c95db7e8049d47a..23e56cda0f67e9d 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1798,11 +1798,13 @@ def ArmMveStrictPolymorphism : TypeAttr, TargetSpecificAttr<TargetARM> { let Documentation = [ArmMveStrictPolymorphismDocs]; } -def NoUniqueAddress : InheritableAttr, TargetSpecificAttr<TargetItaniumCXXABI> { - let Spellings = [CXX11<"", "no_unique_address", 201803>]; +def NoUniqueAddress : InheritableAttr { + let Spellings = [CXX11<"", "no_unique_address", 201803>, + CXX11<"msvc", "no_unique_address", 201803>]; + let Accessors = [Accessor<"isDefault", [CXX11<"", "no_unique_address", 201803>]>, + Accessor<"isMSVC", [CXX11<"msvc", "no_unique_address", 201803>]>]; let Subjects = SubjectList<[NonBitField], ErrorDiag>; let Documentation = [NoUniqueAddressDocs]; - let SimpleHandler = 1; } def ReturnsTwice : InheritableAttr { diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index f11ea89d14bad0d..21e6373611272b5 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -1405,6 +1405,10 @@ Example usage: ``[[no_unique_address]]`` is a standard C++20 attribute. Clang supports its use in C++11 onwards. + +On MSVC targets, ``[[no_unique_address]]`` is ignored; use +``[[msvc::no_unique_address]]`` instead. Currently there is no guarantee of ABI +compatibility or stability. }]; } diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 60c80f2b075336b..d1770330070a519 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext &Ctx) const { if (!CXXRD->isEmpty()) return false; + // MS ABI: nonzero if class type with class type fields + if (Ctx.getTargetInfo().getCXXABI().isMicrosoft() && + llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) { + return Field->getType()->getAs<RecordType>(); + })) { + return false; + } + // Otherwise, [...] the circumstances under which the object has zero size // are implementation-defined. // FIXME: This might be Itanium ABI specific; we don't yet know what the MS diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 8afd88ae7be27b3..2152e69732d65c9 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2545,7 +2545,10 @@ struct MicrosoftRecordLayoutBuilder { CharUnits Alignment; }; typedef llvm::DenseMap<const CXXRecordDecl *, CharUnits> BaseOffsetsMapTy; - MicrosoftRecordLayoutBuilder(const ASTContext &Context) : Context(Context) {} + MicrosoftRecordLayoutBuilder(const ASTContext &Context, + EmptySubobjectMap *EmptySubobjects) + : Context(Context), EmptySubobjects(EmptySubobjects) {} + private: MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete; void operator=(const MicrosoftRecordLayoutBuilder &) = delete; @@ -2562,7 +2565,11 @@ struct MicrosoftRecordLayoutBuilder { void layoutNonVirtualBase(const CXXRecordDecl *RD, const CXXRecordDecl *BaseDecl, const ASTRecordLayout &BaseLayout, - const ASTRecordLayout *&PreviousBaseLayout); + const ASTRecordLayout *&PreviousBaseLayout, + BaseSubobjectInfo *Base); + BaseSubobjectInfo *computeBaseSubobjectInfo(const CXXRecordDecl *RD, + bool IsVirtual, + BaseSubobjectInfo *Derived); void injectVFPtr(const CXXRecordDecl *RD); void injectVBPtr(const CXXRecordDecl *RD); /// Lays out the fields of the record. Also rounds size up to @@ -2595,6 +2602,12 @@ struct MicrosoftRecordLayoutBuilder { llvm::SmallPtrSetImpl<const CXXRecordDecl *> &HasVtorDispSet, const CXXRecordDecl *RD) const; const ASTContext &Context; + EmptySubobjectMap *EmptySubobjects; + llvm::SpecificBumpPtrAllocator<BaseSubobjectInfo> BaseSubobjectInfoAllocator; + typedef llvm::DenseMap<const CXXRecordDecl *, BaseSubobjectInfo *> + BaseSubobjectInfoMapTy; + BaseSubobjectInfoMapTy VirtualBaseInfo; + /// The size of the record being laid out. CharUnits Size; /// The non-virtual size of the record layout. @@ -2818,6 +2831,8 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { // Iterate through the bases and lay out the non-virtual ones. for (const CXXBaseSpecifier &Base : RD->bases()) { const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl(); + BaseSubobjectInfo *BaseInfo = + computeBaseSubobjectInfo(BaseDecl, Base.isVirtual(), nullptr); HasPolymorphicBaseClass |= BaseDecl->isPolymorphic(); const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl); // Mark and skip virtual bases. @@ -2839,7 +2854,8 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { LeadsWithZeroSizedBase = BaseLayout.leadsWithZeroSizedBase(); } // Lay out the base. - layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout); + layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout, + BaseInfo); } // Figure out if we need a fresh VFPtr for this class. if (RD->isPolymorphic()) { @@ -2864,10 +2880,21 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { bool CheckLeadingLayout = !PrimaryBase; // Iterate through the bases and lay out the non-virtual ones. for (const CXXBaseSpecifier &Base : RD->bases()) { - if (Base.isVirtual()) - continue; const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl(); const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl); + BaseSubobjectInfo *BaseInfo = + computeBaseSubobjectInfo(BaseDecl, Base.isVirtual(), nullptr); + + if (Base.isVirtual()) { + // Mark offset for virtual base. + CharUnits Offset = CharUnits::Zero(); + while (!EmptySubobjects->CanPlaceBaseAtOffset(BaseInfo, Offset)) { + ElementInfo Info = getAdjustedElementInfo(BaseLayout); + Offset += Info.Alignment; + } + continue; + } + // Only lay out bases without extendable VFPtrs on the second pass. if (BaseLayout.hasExtendableVFPtr()) { VBPtrOffset = Bases[BaseDecl] + BaseLayout.getNonVirtualSize(); @@ -2880,7 +2907,8 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { LeadsWithZeroSizedBase = BaseLayout.leadsWithZeroSizedBase(); } // Lay out the base. - layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout); + layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout, + BaseInfo); VBPtrOffset = Bases[BaseDecl] + BaseLayout.getNonVirtualSize(); } // Set our VBPtroffset if we know it at this point. @@ -2908,10 +2936,9 @@ static bool recordUsesEBO(const RecordDecl *RD) { } void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase( - const CXXRecordDecl *RD, - const CXXRecordDecl *BaseDecl, + const CXXRecordDecl *RD, const CXXRecordDecl *BaseDecl, const ASTRecordLayout &BaseLayout, - const ASTRecordLayout *&PreviousBaseLayout) { + const ASTRecordLayout *&PreviousBaseLayout, BaseSubobjectInfo *Base) { // Insert padding between two bases if the left first one is zero sized or // contains a zero sized subobject and the right is zero sized or one leads // with a zero sized base. @@ -2927,7 +2954,7 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase( if (UseExternalLayout) { FoundBase = External.getExternalNVBaseOffset(BaseDecl, BaseOffset); if (BaseOffset > Size) { - Size = BaseOffset; + DataSize = Size = BaseOffset; } } @@ -2937,14 +2964,97 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase( BaseOffset = CharUnits::Zero(); } else { // Otherwise, lay the base out at the end of the MDC. - BaseOffset = Size = Size.alignTo(Info.Alignment); + BaseOffset = DataSize = Size = Size.alignTo(Info.Alignment); } + + // Place in EmptySubobjects map but don't check the position? MSVC seems to + // not allow fields to overlap at the end of a virtual base, but they can + // overlap with other bass. + EmptySubobjects->CanPlaceBaseAtOffset(Base, BaseOffset); } + Bases.insert(std::make_pair(BaseDecl, BaseOffset)); Size += BaseLayout.getNonVirtualSize(); + DataSize = Size; PreviousBaseLayout = &BaseLayout; } +BaseSubobjectInfo *MicrosoftRecordLayoutBuilder::computeBaseSubobjectInfo( + const CXXRecordDecl *RD, bool IsVirtual, BaseSubobjectInfo *Derived) { + // This is copied directly from ItaniumRecordLayoutBuilder::ComputeBaseSubobjectInfo. + BaseSubobjectInfo *Info; + + if (IsVirtual) { + // Check if we already have info about this virtual base. + BaseSubobjectInfo *&InfoSlot = VirtualBaseInfo[RD]; + if (InfoSlot) { + assert(InfoSlot->Class == RD && "Wrong class for virtual base info!"); + return InfoSlot; + } + + // We don't, create it. + InfoSlot = new (BaseSubobjectInfoAllocator.Allocate()) BaseSubobjectInfo; + Info = InfoSlot; + } else { + Info = new (BaseSubobjectInfoAllocator.Allocate()) BaseSubobjectInfo; + } + + Info->Class = RD; + Info->IsVirtual = IsVirtual; + Info->Derived = nullptr; + Info->PrimaryVirtualBaseInfo = nullptr; + + const CXXRecordDecl *PrimaryVirtualBase = nullptr; + BaseSubobjectInfo *PrimaryVirtualBaseInfo = nullptr; + + // Check if this base has a primary virtual base. + if (RD->getNumVBases()) { + const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD); + if (Layout.isPrimaryBaseVirtual()) { + // This base does have a primary virtual base. + PrimaryVirtualBase = Layout.getPrimaryBase(); + assert(PrimaryVirtualBase && "Didn't have a primary virtual base!"); + + // Now check if we have base subobject info about this primary base. + PrimaryVirtualBaseInfo = VirtualBaseInfo.lookup(PrimaryVirtualBase); + + if (PrimaryVirtualBaseInfo) { + if (PrimaryVirtualBaseInfo->Derived) { + // We did have info about this primary base, and it turns out that it + // has already been claimed as a primary virtual base for another + // base. + PrimaryVirtualBase = nullptr; + } else { + // We can claim this base as our primary base. + Info->PrimaryVirtualBaseInfo = PrimaryVirtualBaseInfo; + PrimaryVirtualBaseInfo->Derived = Info; + } + } + } + } + + // Now go through all direct bases. + for (const auto &I : RD->bases()) { + bool IsVirtual = I.isVirtual(); + + const CXXRecordDecl *BaseDecl = I.getType()->getAsCXXRecordDecl(); + + Info->Bases.push_back(computeBaseSubobjectInfo(BaseDecl, IsVirtual, Info)); + } + + if (PrimaryVirtualBase && !PrimaryVirtualBaseInfo) { + // Traversing the bases must have created the base info for our primary + // virtual base. + PrimaryVirtualBaseInfo = VirtualBaseInfo.lookup(PrimaryVirtualBase); + assert(PrimaryVirtualBaseInfo && "Did not create a primary virtual base!"); + + // Claim the primary virtual base as our primary virtual base. + Info->PrimaryVirtualBaseInfo = PrimaryVirtualBaseInfo; + PrimaryVirtualBaseInfo->Derived = Info; + } + return Info; +} + void MicrosoftRecordLayoutBuilder::layoutFields(const RecordDecl *RD) { LastFieldIsNonZeroWidthBitfield = false; for (const FieldDecl *Field : RD->fields()) @@ -2956,18 +3066,48 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) { layoutBitField(FD); return; } + LastFieldIsNonZeroWidthBitfield = false; ElementInfo Info = getAdjustedElementInfo(FD); Alignment = std::max(Alignment, Info.Alignment); - CharUnits FieldOffset; - if (UseExternalLayout) + + const CXXRecordDecl *FieldClass = FD->getType()->getAsCXXRecordDecl(); + bool IsOverlappingEmptyField = FD->isPotentiallyOverlapping() && + FieldClass->isEmpty() && + FieldClass->fields().empty(); + const CXXRecordDecl *ParentClass = cast<CXXRecordDecl>(FD->getParent()); + bool HasBases = + !ParentClass->bases().empty() || !ParentClass->vbases().empty(); + + CharUnits FieldOffset = CharUnits::Zero(); + + if (UseExternalLayout) { FieldOffset = Context.toCharUnitsFromBits(External.getExternalFieldOffset(FD)); - else if (IsUnion) + } else if (IsUnion) { FieldOffset = CharUnits::Zero(); - else + } else if (EmptySubobjects) { + if (!IsOverlappingEmptyField) + FieldOffset = DataSize.alignTo(Info.Alignment); + + while (!EmptySubobjects->CanPlaceFieldAtOffset(FD, FieldOffset)) { + if (FieldOffset == CharUnits::Zero() && DataSize != CharUnits::Zero() && + HasBases) { + // MSVC appears to only do this when there are base classes; + // otherwise it overlaps no_unique_address fields in non-zero offsets. + FieldOffset = DataSize.alignTo(Info.Alignment); + } else { + FieldOffset += Info.Alignment; + } + } + } else { FieldOffset = Size.alignTo(Info.Alignment); + } placeFieldAtOffset(FieldOffset); + + if (!IsOverlappingEmptyField) + DataSize = std::max(DataSize, FieldOffset + Info.Size); + Size = std::max(Size, FieldOffset + Info.Size); } @@ -2999,17 +3139,17 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { auto NewSize = Context.toCharUnitsFromBits( llvm::alignDown(FieldBitOffset, Context.toBits(Info.Alignment)) + Context.toBits(Info.Size)); - Size = std::max(Size, NewSize); + DataSize = Size = std::max(Size, NewSize); Alignment = std::max(Alignment, Info.Alignment); } else if (IsUnion) { placeFieldAtOffset(CharUnits::Zero()); - Size = std::max(Size, Info.Size); + DataSize = Size = std::max(Size, Info.Size); // TODO: Add a Sema warning that MS ignores bitfield alignment in unions. } else { // Allocate a new block of memory and place the bitfield in it. CharUnits FieldOffset = Size.alignTo(Info.Alignment); placeFieldAtOffset(FieldOffset); - Size = FieldOffset + Info.Size; + DataSize = Size = FieldOffset + Info.Size; Alignment = std::max(Alignment, Info.Alignment); RemainingBitsInField = Context.toBits(Info.Size) - Width; } @@ -3029,13 +3169,13 @@ MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) { ElementInfo Info = getAdjustedElementInfo(FD); if (IsUnion) { placeFieldAtOffset(CharUnits::Zero()); - Size = std::max(Size, Info.Size); + DataSize = Size = std::max(Size, Info.Size); // TODO: Add a Sema warning that MS ignores bitfield alignment in unions. } else { // Round up the current record size to the field's alignment boundary. CharUnits FieldOffset = Size.alignTo(Info.Alignment); placeFieldAtOffset(FieldOffset); - Size = FieldOffset; + DataSize = Size = FieldOffset; Alignment = std::max(Alignment, Info.Alignment); } } @@ -3055,7 +3195,7 @@ void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) { // It is possible that there were no fields or bases located after vbptr, // so the size was not adjusted before. if (Size < FieldStart) - Size = FieldStart; + DataSize = Size = FieldStart; return; } // Make sure that the amount we push the fields back by is a multiple of the @@ -3103,6 +3243,7 @@ void MicrosoftRecordLayoutBuilder::injectVFPtr(const CXXRecordDecl *RD) { void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { if (!HasVBPtr) return; + // Vtordisps are always 4 bytes (even in 64-bit mode) CharUnits VtorDispSize = CharUnits::fromQuantity(4); CharUnits VtorDispAlignment = VtorDispSize; @@ -3136,7 +3277,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { if ((PreviousBaseLayout && PreviousBaseLayout->endsWithZeroSizedObject() && BaseLayout.leadsWithZeroSizedBase() && !recordUsesEBO(RD)) || HasVtordisp) { - Size = Size.alignTo(VtorDispAlignment) + VtorDispSize; + DataSize = Size = Size.alignTo(VtorDispAlignment) + VtorDispSize; Alignment = std::max(VtorDispAlignment, Alignment); } // Insert the virtual base. @@ -3154,7 +3295,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { VBases.insert(std::make_pair(BaseDecl, ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp))); - Size = BaseOffset + BaseLayout.getNonVirtualSize(); + DataSize = Size = BaseOffset + BaseLayout.getNonVirtualSize(); PreviousBaseLayout = &BaseLayout; } } @@ -3304,8 +3445,9 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { const ASTRecordLayout *NewEntry = nullptr; if (isMsLayout(*this)) { - MicrosoftRecordLayoutBuilder Builder(*this); if (const auto *RD = dyn_cast<CXXRecordDecl>(D)) { + EmptySubobjectMap EmptySubobjects(*this, RD); + MicrosoftRecordLayoutBuilder Builder(*this, &EmptySubobjects); Builder.cxxLayout(RD); NewEntry = new (*this) ASTRecordLayout( *this, Builder.Size, Builder.Alignment, Builder.Alignment, @@ -3317,6 +3459,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { Builder.EndsWithZeroSizedObject, Builder.LeadsWithZeroSizedBase, Builder.Bases, Builder.VBases); } else { + MicrosoftRecordLayoutBuilder Builder(*this, /*EmptySubobjects*/ nullptr); Builder.layout(D); NewEntry = new (*this) ASTRecordLayout( *this, Builder.Size, Builder.Alignment, Builder.Alignment, diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp index 270ff11559417d9..792ff3ce45cf3a3 100644 --- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp +++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp @@ -743,6 +743,7 @@ void CGRecordLowering::calculateZeroInit() { void CGRecordLowering::clipTailPadding() { std::vector<MemberInfo>::iterator Prior = Members.begin(); CharUnits Tail = getSize(Prior->Data); + for (std::vector<MemberInfo>::iterator Member = Prior + 1, MemberEnd = Members.end(); Member != MemberEnd; ++Member) { diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 59e0f3e83cfdd80..370dfdeb800b38f 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -8368,6 +8368,20 @@ static void handleNoMergeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { D->addAttr(NoMergeAttr::Create(S.Context, AL)); } +static void handleNoUniqueAddressAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + NoUniqueAddressAttr TmpAttr(S.Context, AL); + if (S.getLangOpts().MSVCCompat) { + if (TmpAttr.isDefault()) { + S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL; + return; + } + } else if (TmpAttr.isMSVC()) { + S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL; + return; + } + D->addAttr(NoUniqueAddressAttr::Create(S.Context, AL)); +} + static void handleSYCLKernelAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // The 'sycl_kernel' attribute applies only to function templates. const auto *FD = cast<FunctionDecl>(D); @@ -9273,6 +9287,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_NoMerge: handleNoMergeAttr(S, D, AL); break; + case ParsedAttr::AT_NoUniqueAddress: + handleNoUniqueAddressAttr(S, D, AL); + break; case ParsedAttr::AT_AvailableOnlyInDefaultEvalMethod: handleAvailableOnlyInDefaultEvalMethod(S, D, AL); diff --git a/clang/test/Layout/ms-no-unique-address.cpp b/clang/test/Layout/ms-no-unique-address.cpp new file mode 100644 index 000000000000000..b03c63cf9bc67f0 --- /dev/null +++ b/clang/test/Layout/ms-no-unique-address.cpp @@ -0,0 +1,338 @@ +// RUN: %clang_cc1 -std=c++2a -fsyntax-only -triple x86_64-windows-msvc -fms-compatibility -fdump-record-layouts %s | FileCheck %s + +namespace Empty { + struct A {}; + struct A2 {}; + struct A3 { [[msvc::no_unique_address]] A a; }; + struct alignas(8) A4 {}; + + struct B { + [[msvc::no_unique_address]] A a; + char b; + }; + static_assert(sizeof(B) == 1); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::B + // CHECK-NEXT: 0 | struct Empty::A a (empty) + // CHECK-NEXT: 0 | char b + // CHECK-NEXT: | [sizeof=1, align=1, + // CHECK-NEXT: | nvsize=1, nvalign=1] + + struct C { + [[msvc::no_unique_address]] A a; + [[msvc::no_unique_address]] A2 a2; + char c; + }; + static_assert(sizeof(C) == 1); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::C + // CHECK-NEXT: 0 | struct Empty::A a (empty) + // CHECK-NEXT: 0 | struct Empty::A2 a2 (empty) + // CHECK-NEXT: 0 | char c + // CHECK-NEXT: | [sizeof=1, align=1, + // CHECK-NEXT: | nvsize=1, nvalign=1] + + struct D { + [[msvc::no_unique_address]] A3 a; + int i; + }; + static_assert(sizeof(D) == 8); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::D + // CHECK-NEXT: 0 | struct Empty::A3 a (empty) + // CHECK-NEXT: 0 | struct Empty::A a (empty) + // CHECK-NEXT: 4 | int i + // CHECK-NEXT: | [sizeof=8, align=4, + // CHECK-NEXT: | nvsize=8, nvalign=4] + + struct E { + [[msvc::no_unique_address]] A a1; + [[msvc::no_unique_address]] A a2; + char e; + }; + static_assert(sizeof(E) == 2); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::E + // CHECK-NEXT: 0 | struct Empty::A a1 (empty) + // CHECK-NEXT: 1 | struct Empty::A a2 (empty) + // CHECK-NEXT: 0 | char e + // CHECK-NEXT: | [sizeof=2, align=1, + // CHECK-NEXT: | nvsize=2, nvalign=1] + + struct F { + ~F(); + [[msvc::no_unique_address]] A a1; + [[msvc::no_unique_address]] A a2; + char f; + }; + static_assert(sizeof(F) == 2); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::F + // CHECK-NEXT: 0 | struct Empty::A a1 (empty) + // CHECK-NEXT: 1 | struct Empty::A a2 (empty) + // CHECK-NEXT: 0 | char f + // CHECK-NEXT: | [sizeof=2, align=1, + // CHECK-NEXT: | nvsize=2, nvalign=1] + + struct G { [[msvc::no_unique_address]] A a; ~G(); }; + static_assert(sizeof(G) == 1); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::G + // CHECK-NEXT: 0 | struct Empty::A a (empty) + // CHECK-NEXT: | [sizeof=1, align=1, + // CHECK-NEXT: | nvsize=1, nvalign=1] + + struct H { + [[msvc::no_unique_address]] A a; + [[msvc::no_unique_address]] A b; + ~H(); + }; + static_assert(sizeof(H) == 2); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::H + // CHECK-NEXT: 0 | struct Empty::A a (empty) + // CHECK-NEXT: 1 | struct Empty::A b (empty) + // CHECK-NEXT: | [sizeof=2, align=1, + // CHECK-NEXT: | nvsize=2, nvalign=1] + + struct I { + [[msvc::no_unique_address]] A4 a; + [[msvc::no_unique_address]] A4 b; + }; + static_assert(sizeof(I) == 16); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::I + // CHECK-NEXT: 0 | struct Empty::A4 a (empty) + // CHECK-NEXT: 8 | struct Empty::A4 b (empty) + // CHECK-NEXT: | [sizeof=16, align=8, + // CHECK-NEXT: | nvsize=16, nvalign=8] + + struct J { + [[msvc::no_unique_address]] A4 a; + A4 b; + }; + static_assert(sizeof(J) == 16); + + // MSVC puts a and b at the same offset. + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::J + // CHECK-NEXT: 0 | struct Empty::A4 a (empty) + // CHECK-NEXT: 8 | struct Empty::A4 b (empty) + // CHECK-NEXT: | [sizeof=16, align=8, + // CHECK-NEXT: | nvsize=16, nvalign=8] + + struct K { + [[msvc::no_unique_address]] A4 a; + [[msvc::no_unique_address]] char c; + [[msvc::no_unique_address]] A4 b; + }; + static_assert(sizeof(K) == 16); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::K + // CHECK-NEXT: 0 | struct Empty::A4 a (empty) + // CHECK-NEXT: 0 | char c + // CHECK-NEXT: 8 | struct Empty::A4 b (empty) + // CHECK-NEXT: | [sizeof=16, align=8, + // CHECK-NEXT: | nvsize=16, nvalign=8] + + struct OversizedEmpty : A { + ~OversizedEmpty(); + [[msvc::no_unique_address]] A a; + }; + static_assert(sizeof(OversizedEmpty) == 2); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::OversizedEmpty + // CHECK-NEXT: 0 | struct Empty::A (base) (empty) + // CHECK-NEXT: 1 | struct Empty::A a (empty) + // CHECK-NEXT: | [sizeof=2, align=1, + // CHECK-NEXT: | nvsize=2, nvalign=1] + + struct HasOversizedEmpty { + [[msvc::no_unique_address]] OversizedEmpty m; + }; + static_assert(sizeof(HasOversizedEmpty) == 2); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::HasOversizedEmpty + // CHECK-NEXT: 0 | struct Empty::OversizedEmpty m (empty) + // CHECK-NEXT: 0 | struct Empty::A (base) (empty) + // CHECK-NEXT: 1 | struct Empty::A a (empty) + // CHECK-NEXT: | [sizeof=2, align=1, + // CHECK-NEXT: | nvsize=2, nvalign=1] + + struct EmptyWithNonzeroDSize { + [[msvc::no_unique_address]] A a; + int x; + [[msvc::no_unique_address]] A b; + int y; + [[msvc::no_unique_address]] A c; + }; + static_assert(sizeof(EmptyWithNonzeroDSize) == 8); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::EmptyWithNonzeroDSize + // CHECK-NEXT: 0 | struct Empty::A a (empty) + // CHECK-NEXT: 0 | int x + // CHECK-NEXT: 1 | struct Empty::A b (empty) + // CHECK-NEXT: 4 | int y + // CHECK-NEXT: 2 | struct Empty::A c (empty) + // CHECK-NEXT: | [sizeof=8, align=4, + // CHECK-NEXT: | nvsize=8, nvalign=4] + + struct EmptyWithNonzeroDSizeNonPOD { + ~EmptyWithNonzeroDSizeNonPOD(); + [[msvc::no_unique_address]] A a; + int x; + [[msvc::no_unique_address]] A b; + int y; + [[msvc::no_unique_address]] A c; + }; + static_assert(sizeof(EmptyWithNonzeroDSizeNonPOD) == 8); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct Empty::EmptyWithNonzeroDSizeNonPOD + // CHECK-NEXT: 0 | struct Empty::A a (empty) + // CHECK-NEXT: 0 | int x + // CHECK-NEXT: 1 | struct Empty::A b (empty) + // CHECK-NEXT: 4 | int y + // CHECK-NEXT: 2 | struct Empty::A c (empty) + // CHECK-NEXT: | [sizeof=8, align=4, + // CHECK-NEXT: | nvsize=8, nvalign=4] +} + +namespace POD { + struct A { int n; char c[3]; }; + struct B { [[msvc::no_unique_address]] A a; char d; }; + static_assert(sizeof(B) == 12); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct POD::B + // CHECK-NEXT: 0 | struct POD::A a + // CHECK-NEXT: 0 | int n + // CHECK-NEXT: 4 | char[3] c + // CHECK-NEXT: 8 | char d + // CHECK-NEXT: | [sizeof=12, align=4, + // CHECK-NEXT: | nvsize=12, nvalign=4] +} + +namespace NonPOD { + struct A { int n; char c[3]; ~A(); }; + struct B { [[msvc::no_unique_address]] A a; char d; }; + static_assert(sizeof(B) == 12); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct NonPOD::B + // CHECK-NEXT: 0 | struct NonPOD::A a + // CHECK-NEXT: 0 | int n + // CHECK-NEXT: 4 | char[3] c + // CHECK-NEXT: 8 | char d + // CHECK-NEXT: | [sizeof=12, align=4, + // CHECK-NEXT: | nvsize=12, nvalign=4] +} + +namespace VBases { + // The nvsize of an object includes the complete size of its empty subobjects + // (although it's unclear why). Ensure this corner case is handled properly. + struct Empty {}; + struct alignas(8) A {}; // dsize 0, nvsize 0, size 8 + struct B : A { char c; }; // dsize 1, nvsize 8, size 8 + static_assert(sizeof(B) == 8); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct VBases::B + // CHECK-NEXT: 0 | struct VBases::A (base) (empty) + // CHECK-NEXT: 0 | char c + // CHECK-NEXT: | [sizeof=8, align=8, + // CHECK-NEXT: | nvsize=8, nvalign=8] + + struct V { int n; }; + + struct C : B, virtual V {}; + static_assert(sizeof(C) == 24); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct VBases::C + // CHECK-NEXT: 0 | struct VBases::B (base) + // CHECK-NEXT: 0 | struct VBases::A (base) (empty) + // CHECK-NEXT: 0 | char c + // CHECK-NEXT: 8 | (C vbtable pointer) + // CHECK-NEXT: 16 | struct VBases::V (virtual base) + // CHECK-NEXT: 16 | int n + // CHECK-NEXT: | [sizeof=24, align=8, + // CHECK-NEXT: | nvsize=16, nvalign=8] + + struct D : virtual Empty { + [[msvc::no_unique_address]] Empty a; + }; + static_assert(sizeof(D) == 16); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct VBases::D + // CHECK-NEXT: 0 | (D vbtable pointer) + // CHECK-NEXT: 9 | struct VBases::Empty a + // CHECK-NEXT: 16 | struct VBases::Empty (virtual base) (empty) + // CHECK-NEXT: | [sizeof=16, align=8, + // CHECK-NEXT: | nvsize=16, nvalign=8] + + struct E : virtual V { + [[msvc::no_unique_address]] B b; + }; + static_assert(sizeof(E) == 24); + + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct VBases::E + // CHECK-NEXT: 0 | (E vbtable pointer) + // CHECK-NEXT: 8 | struct VBases::B b + // CHECK-NEXT: 8 | struct VBases::A (base) (empty) + // CHECK-NEXT: 8 | char c + // CHECK-NEXT: 16 | struct VBases::V (virtual base) + // CHECK-NEXT: 16 | int n + // CHECK-NEXT: | [sizeof=24, align=8, + // CHECK-NEXT: | nvsize=16, nvalign=8] + + struct X : virtual A { [[msvc::no_unique_address]] A a; }; + struct F : virtual A { + [[msvc::no_unique_address]] A a; + [[msvc::no_unique_address]] X x; + }; + static_assert(sizeof(F) == 32); + + // MSVC places x after a and the total size is 48. + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct VBases::F + // CHECK-NEXT: 0 | (F vbtable pointer) + // CHECK-NEXT: 16 | struct VBases::A a (empty) + // CHECK-NEXT: 8 | struct VBases::X x + // CHECK-NEXT: 8 | (X vbtable pointer) + // CHECK-NEXT: 24 | struct VBases::A a (empty) + // CHECK-NEXT: 32 | struct VBases::A (virtual base) (empty) + // CHECK-NEXT: 32 | struct VBases::A (virtual base) (empty) + // CHECK-NEXT: | [sizeof=32, align=8, + // CHECK-NEXT: | nvsize=32, nvalign=8] + + struct G : virtual Empty { + int i; + [[msvc::no_unique_address]] A a; + }; + static_assert(sizeof(G) == 16); + + // MSVC places a at offset 12. + // CHECK:*** Dumping AST Record Layout + // CHECK: 0 | struct VBases::G + // CHECK-NEXT: 0 | (G vbtable pointer) + // CHECK-NEXT: 8 | int i + // CHECK-NEXT: 8 | struct VBases::A a (empty) + // CHECK-NEXT: 16 | struct VBases::Empty (virtual base) (empty) + // CHECK-NEXT: | [sizeof=16, align=8, + // CHECK-NEXT: | nvsize=16, nvalign=8] +} >From b336a0b149bc3b1a7742b301a702b71c7afd16da Mon Sep 17 00:00:00 2001 From: Amy Huang <akhu...@google.com> Date: Tue, 12 Sep 2023 12:50:35 -0700 Subject: [PATCH 2/2] Address comments and update some test cases --- clang/lib/AST/Decl.cpp | 18 +-- clang/lib/AST/RecordLayoutBuilder.cpp | 137 +++--------------- clang/test/Layout/ms-no-unique-address.cpp | 32 ++-- clang/test/Preprocessor/has_attribute.cpp | 2 +- .../test/SemaCXX/cxx2a-no-unique-address.cpp | 2 +- 5 files changed, 45 insertions(+), 146 deletions(-) diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index d1770330070a519..c99d5f6c19a15d6 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -4505,19 +4505,15 @@ bool FieldDecl::isZeroSize(const ASTContext &Ctx) const { if (!CXXRD->isEmpty()) return false; - // MS ABI: nonzero if class type with class type fields - if (Ctx.getTargetInfo().getCXXABI().isMicrosoft() && - llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) { - return Field->getType()->getAs<RecordType>(); - })) { - return false; - } - // Otherwise, [...] the circumstances under which the object has zero size // are implementation-defined. - // FIXME: This might be Itanium ABI specific; we don't yet know what the MS - // ABI will do. - return true; + if (!Ctx.getTargetInfo().getCXXABI().isMicrosoft()) + return true; + + // MS ABI: nonzero if class type with class type fields + return !llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) { + return Field->getType()->getAs<RecordType>(); + }); } bool FieldDecl::isPotentiallyOverlapping() const { diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 2152e69732d65c9..2f5b3be413a7b9e 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2565,11 +2565,7 @@ struct MicrosoftRecordLayoutBuilder { void layoutNonVirtualBase(const CXXRecordDecl *RD, const CXXRecordDecl *BaseDecl, const ASTRecordLayout &BaseLayout, - const ASTRecordLayout *&PreviousBaseLayout, - BaseSubobjectInfo *Base); - BaseSubobjectInfo *computeBaseSubobjectInfo(const CXXRecordDecl *RD, - bool IsVirtual, - BaseSubobjectInfo *Derived); + const ASTRecordLayout *&PreviousBaseLayout); void injectVFPtr(const CXXRecordDecl *RD); void injectVBPtr(const CXXRecordDecl *RD); /// Lays out the fields of the record. Also rounds size up to @@ -2831,8 +2827,6 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { // Iterate through the bases and lay out the non-virtual ones. for (const CXXBaseSpecifier &Base : RD->bases()) { const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl(); - BaseSubobjectInfo *BaseInfo = - computeBaseSubobjectInfo(BaseDecl, Base.isVirtual(), nullptr); HasPolymorphicBaseClass |= BaseDecl->isPolymorphic(); const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl); // Mark and skip virtual bases. @@ -2854,8 +2848,7 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { LeadsWithZeroSizedBase = BaseLayout.leadsWithZeroSizedBase(); } // Lay out the base. - layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout, - BaseInfo); + layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout); } // Figure out if we need a fresh VFPtr for this class. if (RD->isPolymorphic()) { @@ -2882,18 +2875,9 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { for (const CXXBaseSpecifier &Base : RD->bases()) { const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl(); const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl); - BaseSubobjectInfo *BaseInfo = - computeBaseSubobjectInfo(BaseDecl, Base.isVirtual(), nullptr); - if (Base.isVirtual()) { - // Mark offset for virtual base. - CharUnits Offset = CharUnits::Zero(); - while (!EmptySubobjects->CanPlaceBaseAtOffset(BaseInfo, Offset)) { - ElementInfo Info = getAdjustedElementInfo(BaseLayout); - Offset += Info.Alignment; - } + if (Base.isVirtual()) continue; - } // Only lay out bases without extendable VFPtrs on the second pass. if (BaseLayout.hasExtendableVFPtr()) { @@ -2907,8 +2891,7 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { LeadsWithZeroSizedBase = BaseLayout.leadsWithZeroSizedBase(); } // Lay out the base. - layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout, - BaseInfo); + layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout); VBPtrOffset = Bases[BaseDecl] + BaseLayout.getNonVirtualSize(); } // Set our VBPtroffset if we know it at this point. @@ -2938,7 +2921,7 @@ static bool recordUsesEBO(const RecordDecl *RD) { void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase( const CXXRecordDecl *RD, const CXXRecordDecl *BaseDecl, const ASTRecordLayout &BaseLayout, - const ASTRecordLayout *&PreviousBaseLayout, BaseSubobjectInfo *Base) { + const ASTRecordLayout *&PreviousBaseLayout) { // Insert padding between two bases if the left first one is zero sized or // contains a zero sized subobject and the right is zero sized or one leads // with a zero sized base. @@ -2954,7 +2937,7 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase( if (UseExternalLayout) { FoundBase = External.getExternalNVBaseOffset(BaseDecl, BaseOffset); if (BaseOffset > Size) { - DataSize = Size = BaseOffset; + Size = BaseOffset; } } @@ -2964,13 +2947,8 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase( BaseOffset = CharUnits::Zero(); } else { // Otherwise, lay the base out at the end of the MDC. - BaseOffset = DataSize = Size = Size.alignTo(Info.Alignment); + BaseOffset = Size = Size.alignTo(Info.Alignment); } - - // Place in EmptySubobjects map but don't check the position? MSVC seems to - // not allow fields to overlap at the end of a virtual base, but they can - // overlap with other bass. - EmptySubobjects->CanPlaceBaseAtOffset(Base, BaseOffset); } Bases.insert(std::make_pair(BaseDecl, BaseOffset)); @@ -2979,82 +2957,6 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase( PreviousBaseLayout = &BaseLayout; } -BaseSubobjectInfo *MicrosoftRecordLayoutBuilder::computeBaseSubobjectInfo( - const CXXRecordDecl *RD, bool IsVirtual, BaseSubobjectInfo *Derived) { - // This is copied directly from ItaniumRecordLayoutBuilder::ComputeBaseSubobjectInfo. - BaseSubobjectInfo *Info; - - if (IsVirtual) { - // Check if we already have info about this virtual base. - BaseSubobjectInfo *&InfoSlot = VirtualBaseInfo[RD]; - if (InfoSlot) { - assert(InfoSlot->Class == RD && "Wrong class for virtual base info!"); - return InfoSlot; - } - - // We don't, create it. - InfoSlot = new (BaseSubobjectInfoAllocator.Allocate()) BaseSubobjectInfo; - Info = InfoSlot; - } else { - Info = new (BaseSubobjectInfoAllocator.Allocate()) BaseSubobjectInfo; - } - - Info->Class = RD; - Info->IsVirtual = IsVirtual; - Info->Derived = nullptr; - Info->PrimaryVirtualBaseInfo = nullptr; - - const CXXRecordDecl *PrimaryVirtualBase = nullptr; - BaseSubobjectInfo *PrimaryVirtualBaseInfo = nullptr; - - // Check if this base has a primary virtual base. - if (RD->getNumVBases()) { - const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD); - if (Layout.isPrimaryBaseVirtual()) { - // This base does have a primary virtual base. - PrimaryVirtualBase = Layout.getPrimaryBase(); - assert(PrimaryVirtualBase && "Didn't have a primary virtual base!"); - - // Now check if we have base subobject info about this primary base. - PrimaryVirtualBaseInfo = VirtualBaseInfo.lookup(PrimaryVirtualBase); - - if (PrimaryVirtualBaseInfo) { - if (PrimaryVirtualBaseInfo->Derived) { - // We did have info about this primary base, and it turns out that it - // has already been claimed as a primary virtual base for another - // base. - PrimaryVirtualBase = nullptr; - } else { - // We can claim this base as our primary base. - Info->PrimaryVirtualBaseInfo = PrimaryVirtualBaseInfo; - PrimaryVirtualBaseInfo->Derived = Info; - } - } - } - } - - // Now go through all direct bases. - for (const auto &I : RD->bases()) { - bool IsVirtual = I.isVirtual(); - - const CXXRecordDecl *BaseDecl = I.getType()->getAsCXXRecordDecl(); - - Info->Bases.push_back(computeBaseSubobjectInfo(BaseDecl, IsVirtual, Info)); - } - - if (PrimaryVirtualBase && !PrimaryVirtualBaseInfo) { - // Traversing the bases must have created the base info for our primary - // virtual base. - PrimaryVirtualBaseInfo = VirtualBaseInfo.lookup(PrimaryVirtualBase); - assert(PrimaryVirtualBaseInfo && "Did not create a primary virtual base!"); - - // Claim the primary virtual base as our primary virtual base. - Info->PrimaryVirtualBaseInfo = PrimaryVirtualBaseInfo; - PrimaryVirtualBaseInfo->Derived = Info; - } - return Info; -} - void MicrosoftRecordLayoutBuilder::layoutFields(const RecordDecl *RD) { LastFieldIsNonZeroWidthBitfield = false; for (const FieldDecl *Field : RD->fields()) @@ -3075,10 +2977,6 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) { bool IsOverlappingEmptyField = FD->isPotentiallyOverlapping() && FieldClass->isEmpty() && FieldClass->fields().empty(); - const CXXRecordDecl *ParentClass = cast<CXXRecordDecl>(FD->getParent()); - bool HasBases = - !ParentClass->bases().empty() || !ParentClass->vbases().empty(); - CharUnits FieldOffset = CharUnits::Zero(); if (UseExternalLayout) { @@ -3091,6 +2989,9 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) { FieldOffset = DataSize.alignTo(Info.Alignment); while (!EmptySubobjects->CanPlaceFieldAtOffset(FD, FieldOffset)) { + const CXXRecordDecl *ParentClass = cast<CXXRecordDecl>(FD->getParent()); + bool HasBases = ParentClass && (!ParentClass->bases().empty() || + !ParentClass->vbases().empty()); if (FieldOffset == CharUnits::Zero() && DataSize != CharUnits::Zero() && HasBases) { // MSVC appears to only do this when there are base classes; @@ -3139,20 +3040,21 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { auto NewSize = Context.toCharUnitsFromBits( llvm::alignDown(FieldBitOffset, Context.toBits(Info.Alignment)) + Context.toBits(Info.Size)); - DataSize = Size = std::max(Size, NewSize); + Size = std::max(Size, NewSize); Alignment = std::max(Alignment, Info.Alignment); } else if (IsUnion) { placeFieldAtOffset(CharUnits::Zero()); - DataSize = Size = std::max(Size, Info.Size); + Size = std::max(Size, Info.Size); // TODO: Add a Sema warning that MS ignores bitfield alignment in unions. } else { // Allocate a new block of memory and place the bitfield in it. CharUnits FieldOffset = Size.alignTo(Info.Alignment); placeFieldAtOffset(FieldOffset); - DataSize = Size = FieldOffset + Info.Size; + Size = FieldOffset + Info.Size; Alignment = std::max(Alignment, Info.Alignment); RemainingBitsInField = Context.toBits(Info.Size) - Width; } + DataSize = Size; } void @@ -3169,15 +3071,16 @@ MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) { ElementInfo Info = getAdjustedElementInfo(FD); if (IsUnion) { placeFieldAtOffset(CharUnits::Zero()); - DataSize = Size = std::max(Size, Info.Size); + Size = std::max(Size, Info.Size); // TODO: Add a Sema warning that MS ignores bitfield alignment in unions. } else { // Round up the current record size to the field's alignment boundary. CharUnits FieldOffset = Size.alignTo(Info.Alignment); placeFieldAtOffset(FieldOffset); - DataSize = Size = FieldOffset; + Size = FieldOffset; Alignment = std::max(Alignment, Info.Alignment); } + DataSize = Size; } void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) { @@ -3195,7 +3098,7 @@ void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) { // It is possible that there were no fields or bases located after vbptr, // so the size was not adjusted before. if (Size < FieldStart) - DataSize = Size = FieldStart; + Size = FieldStart; return; } // Make sure that the amount we push the fields back by is a multiple of the @@ -3277,7 +3180,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { if ((PreviousBaseLayout && PreviousBaseLayout->endsWithZeroSizedObject() && BaseLayout.leadsWithZeroSizedBase() && !recordUsesEBO(RD)) || HasVtordisp) { - DataSize = Size = Size.alignTo(VtorDispAlignment) + VtorDispSize; + Size = Size.alignTo(VtorDispAlignment) + VtorDispSize; Alignment = std::max(VtorDispAlignment, Alignment); } // Insert the virtual base. @@ -3295,7 +3198,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { VBases.insert(std::make_pair(BaseDecl, ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp))); - DataSize = Size = BaseOffset + BaseLayout.getNonVirtualSize(); + Size = BaseOffset + BaseLayout.getNonVirtualSize(); PreviousBaseLayout = &BaseLayout; } } diff --git a/clang/test/Layout/ms-no-unique-address.cpp b/clang/test/Layout/ms-no-unique-address.cpp index b03c63cf9bc67f0..c5dd80aa5fce4d6 100644 --- a/clang/test/Layout/ms-no-unique-address.cpp +++ b/clang/test/Layout/ms-no-unique-address.cpp @@ -148,27 +148,27 @@ namespace Empty { ~OversizedEmpty(); [[msvc::no_unique_address]] A a; }; - static_assert(sizeof(OversizedEmpty) == 2); + static_assert(sizeof(OversizedEmpty) == 1); // CHECK:*** Dumping AST Record Layout // CHECK: 0 | struct Empty::OversizedEmpty // CHECK-NEXT: 0 | struct Empty::A (base) (empty) - // CHECK-NEXT: 1 | struct Empty::A a (empty) - // CHECK-NEXT: | [sizeof=2, align=1, - // CHECK-NEXT: | nvsize=2, nvalign=1] + // CHECK-NEXT: 0 | struct Empty::A a (empty) + // CHECK-NEXT: | [sizeof=1, align=1, + // CHECK-NEXT: | nvsize=1, nvalign=1] struct HasOversizedEmpty { [[msvc::no_unique_address]] OversizedEmpty m; }; - static_assert(sizeof(HasOversizedEmpty) == 2); + static_assert(sizeof(HasOversizedEmpty) == 1); // CHECK:*** Dumping AST Record Layout // CHECK: 0 | struct Empty::HasOversizedEmpty // CHECK-NEXT: 0 | struct Empty::OversizedEmpty m (empty) // CHECK-NEXT: 0 | struct Empty::A (base) (empty) - // CHECK-NEXT: 1 | struct Empty::A a (empty) - // CHECK-NEXT: | [sizeof=2, align=1, - // CHECK-NEXT: | nvsize=2, nvalign=1] + // CHECK-NEXT: 0 | struct Empty::A a (empty) + // CHECK-NEXT: | [sizeof=1, align=1, + // CHECK-NEXT: | nvsize=1, nvalign=1] struct EmptyWithNonzeroDSize { [[msvc::no_unique_address]] A a; @@ -279,7 +279,7 @@ namespace VBases { // CHECK:*** Dumping AST Record Layout // CHECK: 0 | struct VBases::D // CHECK-NEXT: 0 | (D vbtable pointer) - // CHECK-NEXT: 9 | struct VBases::Empty a + // CHECK-NEXT: 8 | struct VBases::Empty a // CHECK-NEXT: 16 | struct VBases::Empty (virtual base) (empty) // CHECK-NEXT: | [sizeof=16, align=8, // CHECK-NEXT: | nvsize=16, nvalign=8] @@ -305,20 +305,20 @@ namespace VBases { [[msvc::no_unique_address]] A a; [[msvc::no_unique_address]] X x; }; - static_assert(sizeof(F) == 32); + static_assert(sizeof(F) == 24); // MSVC places x after a and the total size is 48. // CHECK:*** Dumping AST Record Layout // CHECK: 0 | struct VBases::F // CHECK-NEXT: 0 | (F vbtable pointer) - // CHECK-NEXT: 16 | struct VBases::A a (empty) + // CHECK-NEXT: 8 | struct VBases::A a (empty) // CHECK-NEXT: 8 | struct VBases::X x // CHECK-NEXT: 8 | (X vbtable pointer) - // CHECK-NEXT: 24 | struct VBases::A a (empty) - // CHECK-NEXT: 32 | struct VBases::A (virtual base) (empty) - // CHECK-NEXT: 32 | struct VBases::A (virtual base) (empty) - // CHECK-NEXT: | [sizeof=32, align=8, - // CHECK-NEXT: | nvsize=32, nvalign=8] + // CHECK-NEXT: 16 | struct VBases::A a (empty) + // CHECK-NEXT: 24 | struct VBases::A (virtual base) (empty) + // CHECK-NEXT: 24 | struct VBases::A (virtual base) (empty) + // CHECK-NEXT: | [sizeof=24, align=8, + // CHECK-NEXT: | nvsize=24, nvalign=8] struct G : virtual Empty { int i; diff --git a/clang/test/Preprocessor/has_attribute.cpp b/clang/test/Preprocessor/has_attribute.cpp index bf0f9b3bc4a8f08..53b3792d5664797 100644 --- a/clang/test/Preprocessor/has_attribute.cpp +++ b/clang/test/Preprocessor/has_attribute.cpp @@ -55,7 +55,7 @@ CXX11(unlikely) // CHECK: likely: 201803L // CHECK: maybe_unused: 201603L // ITANIUM: no_unique_address: 201803L -// WINDOWS: no_unique_address: 0 +// WINDOWS: no_unique_address: 201803L // CHECK: nodiscard: 201907L // CHECK: noreturn: 200809L // CHECK: unlikely: 201803L diff --git a/clang/test/SemaCXX/cxx2a-no-unique-address.cpp b/clang/test/SemaCXX/cxx2a-no-unique-address.cpp index 44d6d3acdddef70..49f987dc3865fd1 100644 --- a/clang/test/SemaCXX/cxx2a-no-unique-address.cpp +++ b/clang/test/SemaCXX/cxx2a-no-unique-address.cpp @@ -1,5 +1,5 @@ // RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-linux-gnu -// RUN: %clang_cc1 -std=c++2a %s -verify=unsupported -triple x86_64-windows +// RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-windows [[no_unique_address]] int a; // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}} [[no_unique_address]] void f(); // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
