akhuang updated this revision to Diff 556090. akhuang added a comment. more code updates and added some test cases
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157762/new/ https://reviews.llvm.org/D157762 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/lib/AST/Decl.cpp clang/lib/AST/RecordLayoutBuilder.cpp clang/lib/CodeGen/CGRecordLayoutBuilder.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/test/Layout/ms-no-unique-address.cpp
Index: clang/test/Layout/ms-no-unique-address.cpp =================================================================== --- /dev/null +++ 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] +} Index: clang/lib/Sema/SemaDeclAttr.cpp =================================================================== --- clang/lib/Sema/SemaDeclAttr.cpp +++ clang/lib/Sema/SemaDeclAttr.cpp @@ -8416,6 +8416,20 @@ 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); @@ -9321,6 +9335,9 @@ 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); Index: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp =================================================================== --- clang/lib/CodeGen/CGRecordLayoutBuilder.cpp +++ clang/lib/CodeGen/CGRecordLayoutBuilder.cpp @@ -743,6 +743,7 @@ 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) { Index: clang/lib/AST/RecordLayoutBuilder.cpp =================================================================== --- clang/lib/AST/RecordLayoutBuilder.cpp +++ clang/lib/AST/RecordLayoutBuilder.cpp @@ -2545,7 +2545,10 @@ 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 @@ 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 @@ 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 @@ // 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 @@ 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 @@ 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 @@ 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 @@ } 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 @@ if (UseExternalLayout) { FoundBase = External.getExternalNVBaseOffset(BaseDecl, BaseOffset); if (BaseOffset > Size) { - Size = BaseOffset; + DataSize = Size = BaseOffset; } } @@ -2937,14 +2964,97 @@ 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 @@ 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 @@ 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 @@ 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 @@ // 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::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 @@ 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 @@ 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 @@ 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 @@ 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, Index: clang/lib/AST/Decl.cpp =================================================================== --- clang/lib/AST/Decl.cpp +++ clang/lib/AST/Decl.cpp @@ -4505,6 +4505,14 @@ 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 Index: clang/include/clang/Basic/AttrDocs.td =================================================================== --- clang/include/clang/Basic/AttrDocs.td +++ clang/include/clang/Basic/AttrDocs.td @@ -1405,6 +1405,9 @@ ``[[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. }]; } Index: clang/include/clang/Basic/Attr.td =================================================================== --- clang/include/clang/Basic/Attr.td +++ clang/include/clang/Basic/Attr.td @@ -1787,11 +1787,13 @@ 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 {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits