From d2c1350f36685bbf6e4a445103320d83258cc0de Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Wed, 25 Feb 2015 10:32:13 +0000 Subject: [PATCH] Revert r230446, "MS ABI: Try to respect external AST source record layouts" It fails on Clang::PCH/headersearch.cpp for targeting msvc. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@230474 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/AST/RecordLayoutBuilder.cpp | 190 +++++++++------------------- test/CodeGenCXX/override-layout.cpp | 6 +- 2 files changed, 63 insertions(+), 133 deletions(-) diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp index 3753d27aec..f4733cc6b7 100644 --- a/lib/AST/RecordLayoutBuilder.cpp +++ b/lib/AST/RecordLayoutBuilder.cpp @@ -55,52 +55,6 @@ struct BaseSubobjectInfo { const BaseSubobjectInfo *Derived; }; -/// \brief Externally provided layout. Typically used when the AST source, such -/// as DWARF, lacks all the information that was available at compile time, such -/// as alignment attributes on fields and pragmas in effect. -struct ExternalLayout { - ExternalLayout() : Size(0), Align(0) {} - - /// \brief Overall record size in bits. - uint64_t Size; - - /// \brief Overall record alignment in bits. - uint64_t Align; - - /// \brief Record field offsets in bits. - llvm::DenseMap FieldOffsets; - - /// \brief Direct, non-virtual base offsets. - llvm::DenseMap BaseOffsets; - - /// \brief Virtual base offsets. - llvm::DenseMap VirtualBaseOffsets; - - /// Get the offset of the given field. The external source must provide - /// entries for all fields in the record. - uint64_t getExternalFieldOffset(const FieldDecl *FD) { - assert(FieldOffsets.count(FD) && - "Field does not have an external offset"); - return FieldOffsets[FD]; - } - - bool getExternalNVBaseOffset(const CXXRecordDecl *RD, CharUnits &BaseOffset) { - auto Known = BaseOffsets.find(RD); - if (Known == BaseOffsets.end()) - return false; - BaseOffset = Known->second; - return true; - } - - bool getExternalVBaseOffset(const CXXRecordDecl *RD, CharUnits &BaseOffset) { - auto Known = VirtualBaseOffsets.find(RD); - if (Known == VirtualBaseOffsets.end()) - return false; - BaseOffset = Known->second; - return true; - } -}; - /// EmptySubobjectMap - Keeps track of which empty subobjects exist at different /// offsets while laying out a C++ class. class EmptySubobjectMap { @@ -587,7 +541,7 @@ protected: /// \brief Whether the external AST source has provided a layout for this /// record. - unsigned UseExternalLayout : 1; + unsigned ExternalLayout : 1; /// \brief Whether we need to infer alignment, even when we have an /// externally-provided layout. @@ -653,14 +607,26 @@ protected: /// avoid visiting virtual bases more than once. llvm::SmallPtrSet VisitedVirtualBases; - /// Valid if UseExternalLayout is true. - ExternalLayout External; + /// \brief Externally-provided size. + uint64_t ExternalSize; + + /// \brief Externally-provided alignment. + uint64_t ExternalAlign; + + /// \brief Externally-provided field offsets. + llvm::DenseMap ExternalFieldOffsets; + + /// \brief Externally-provided direct, non-virtual base offsets. + llvm::DenseMap ExternalBaseOffsets; + + /// \brief Externally-provided virtual base offsets. + llvm::DenseMap ExternalVirtualBaseOffsets; RecordLayoutBuilder(const ASTContext &Context, EmptySubobjectMap *EmptySubobjects) : Context(Context), EmptySubobjects(EmptySubobjects), Size(0), Alignment(CharUnits::One()), UnpackedAlignment(CharUnits::One()), - UseExternalLayout(false), InferAlignment(false), + ExternalLayout(false), InferAlignment(false), Packed(false), IsUnion(false), IsMac68kAlign(false), IsMsStruct(false), UnfilledBitsInLastUnit(0), LastBitfieldTypeSize(0), MaxFieldAlignment(CharUnits::Zero()), @@ -1168,12 +1134,21 @@ CharUnits RecordLayoutBuilder::LayoutBase(const BaseSubobjectInfo *Base) { // Query the external layout to see if it provides an offset. bool HasExternalLayout = false; - if (UseExternalLayout) { + if (ExternalLayout) { llvm::DenseMap::iterator Known; - if (Base->IsVirtual) - HasExternalLayout = External.getExternalNVBaseOffset(Base->Class, Offset); - else - HasExternalLayout = External.getExternalVBaseOffset(Base->Class, Offset); + if (Base->IsVirtual) { + Known = ExternalVirtualBaseOffsets.find(Base->Class); + if (Known != ExternalVirtualBaseOffsets.end()) { + Offset = Known->second; + HasExternalLayout = true; + } + } else { + Known = ExternalBaseOffsets.find(Base->Class); + if (Known != ExternalBaseOffsets.end()) { + Offset = Known->second; + HasExternalLayout = true; + } + } } CharUnits UnpackedBaseAlign = Layout.getNonVirtualAlignment(); @@ -1260,15 +1235,18 @@ void RecordLayoutBuilder::InitializeLayout(const Decl *D) { // If there is an external AST source, ask it for the various offsets. if (const RecordDecl *RD = dyn_cast(D)) - if (ExternalASTSource *Source = Context.getExternalSource()) { - UseExternalLayout = Source->layoutRecordType( - RD, External.Size, External.Align, External.FieldOffsets, - External.BaseOffsets, External.VirtualBaseOffsets); - + if (ExternalASTSource *External = Context.getExternalSource()) { + ExternalLayout = External->layoutRecordType(RD, + ExternalSize, + ExternalAlign, + ExternalFieldOffsets, + ExternalBaseOffsets, + ExternalVirtualBaseOffsets); + // Update based on external alignment. - if (UseExternalLayout) { - if (External.Align > 0) { - Alignment = Context.toCharUnitsFromBits(External.Align); + if (ExternalLayout) { + if (ExternalAlign > 0) { + Alignment = Context.toCharUnitsFromBits(ExternalAlign); } else { // The external source didn't have alignment information; infer it. InferAlignment = true; @@ -1610,7 +1588,7 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { // If we're using external layout, give the external layout a chance // to override this information. - if (UseExternalLayout) + if (ExternalLayout) FieldOffset = updateExternalFieldOffset(D, FieldOffset); // Okay, place the bitfield at the calculated offset. @@ -1626,7 +1604,7 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { FieldAlign = UnpackedFieldAlign = 1; // Diagnose differences in layout due to padding or packing. - if (!UseExternalLayout) + if (!ExternalLayout) CheckFieldPadding(FieldOffset, UnpaddedFieldOffset, UnpackedFieldOffset, UnpackedFieldAlign, FieldPacked, D); @@ -1749,7 +1727,7 @@ void RecordLayoutBuilder::LayoutField(const FieldDecl *D, UnpackedFieldOffset = UnpackedFieldOffset.RoundUpToAlignment(UnpackedFieldAlign); - if (UseExternalLayout) { + if (ExternalLayout) { FieldOffset = Context.toCharUnitsFromBits( updateExternalFieldOffset(D, Context.toBits(FieldOffset))); @@ -1772,7 +1750,7 @@ void RecordLayoutBuilder::LayoutField(const FieldDecl *D, // Place this field at the current location. FieldOffsets.push_back(Context.toBits(FieldOffset)); - if (!UseExternalLayout) + if (!ExternalLayout) CheckFieldPadding(Context.toBits(FieldOffset), UnpaddedFieldOffset, Context.toBits(UnpackedFieldOffset), Context.toBits(UnpackedFieldAlign), FieldPacked, D); @@ -1824,15 +1802,15 @@ void RecordLayoutBuilder::FinishLayout(const NamedDecl *D) { uint64_t RoundedSize = llvm::RoundUpToAlignment(getSizeInBits(), Context.toBits(Alignment)); - if (UseExternalLayout) { + if (ExternalLayout) { // If we're inferring alignment, and the external size is smaller than // our size after we've rounded up to alignment, conservatively set the // alignment to 1. - if (InferAlignment && External.Size < RoundedSize) { + if (InferAlignment && ExternalSize < RoundedSize) { Alignment = CharUnits::One(); InferAlignment = false; } - setSize(External.Size); + setSize(ExternalSize); return; } @@ -1868,7 +1846,7 @@ void RecordLayoutBuilder::UpdateAlignment(CharUnits NewAlignment, CharUnits UnpackedNewAlignment) { // The alignment is not modified when using 'mac68k' alignment or when // we have an externally-supplied layout that also provides overall alignment. - if (IsMac68kAlign || (UseExternalLayout && !InferAlignment)) + if (IsMac68kAlign || (ExternalLayout && !InferAlignment)) return; if (NewAlignment > Alignment) { @@ -1887,8 +1865,11 @@ void RecordLayoutBuilder::UpdateAlignment(CharUnits NewAlignment, uint64_t RecordLayoutBuilder::updateExternalFieldOffset(const FieldDecl *Field, uint64_t ComputedOffset) { - uint64_t ExternalFieldOffset = External.getExternalFieldOffset(Field); - + assert(ExternalFieldOffsets.find(Field) != ExternalFieldOffsets.end() && + "Field does not have an external offset"); + + uint64_t ExternalFieldOffset = ExternalFieldOffsets[Field]; + if (InferAlignment && ExternalFieldOffset < ComputedOffset) { // The externally-supplied field offset is before the field offset we // computed. Assume that the structure is packed. @@ -2270,13 +2251,6 @@ public: /// \brief True if this class is zero sized or first base is zero sized or /// has this property. Only used for MS-ABI. bool LeadsWithZeroSizedBase : 1; - - /// \brief True if the external AST source provided a layout for this record. - bool UseExternalLayout : 1; - - /// \brief The layout provided by the external AST source. Only active if - /// UseExternalLayout is true. - ExternalLayout External; }; } // namespace @@ -2396,13 +2370,6 @@ void MicrosoftRecordLayoutBuilder::initializeLayout(const RecordDecl *RD) { // Packed attribute forces max field alignment to be 1. if (RD->hasAttr()) MaxFieldAlignment = CharUnits::One(); - - // Try to respect the external layout if present. - UseExternalLayout = false; - if (ExternalASTSource *Source = Context.getExternalSource()) - UseExternalLayout = Source->layoutRecordType( - RD, External.Size, External.Align, External.FieldOffsets, - External.BaseOffsets, External.VirtualBaseOffsets); } void @@ -2507,18 +2474,7 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase( BaseLayout.leadsWithZeroSizedBase()) Size++; ElementInfo Info = getAdjustedElementInfo(BaseLayout); - CharUnits BaseOffset; - - // Respect the external AST source base offset, if present. - bool FoundBase = false; - if (UseExternalLayout) { - FoundBase = External.getExternalNVBaseOffset(BaseDecl, BaseOffset); - if (FoundBase) - assert(BaseOffset >= Size && "base offset already allocated"); - } - - if (!FoundBase) - BaseOffset = Size.RoundUpToAlignment(Info.Alignment); + CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment); Bases.insert(std::make_pair(BaseDecl, BaseOffset)); Size = BaseOffset + BaseLayout.getNonVirtualSize(); PreviousBaseLayout = &BaseLayout; @@ -2542,14 +2498,7 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) { placeFieldAtOffset(CharUnits::Zero()); Size = std::max(Size, Info.Size); } else { - CharUnits FieldOffset; - if (UseExternalLayout) { - FieldOffset = - Context.toCharUnitsFromBits(External.getExternalFieldOffset(FD)); - assert(FieldOffset >= Size && "field offset already allocated"); - } else { - FieldOffset = Size.RoundUpToAlignment(Info.Alignment); - } + CharUnits FieldOffset = Size.RoundUpToAlignment(Info.Alignment); placeFieldAtOffset(FieldOffset); Size = FieldOffset + Info.Size; } @@ -2623,16 +2572,14 @@ void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) { CharUnits InjectionSite = VBPtrOffset; // But before we do, make sure it's properly aligned. VBPtrOffset = VBPtrOffset.RoundUpToAlignment(PointerInfo.Alignment); - // Shift everything after the vbptr down, unless we're using an external - // layout. - if (UseExternalLayout) - return; // Determine where the first field should be laid out after the vbptr. CharUnits FieldStart = VBPtrOffset + PointerInfo.Size; // Make sure that the amount we push the fields back by is a multiple of the // alignment. CharUnits Offset = (FieldStart - InjectionSite).RoundUpToAlignment( std::max(RequiredAlignment, Alignment)); + // Increase the size of the object and push back all fields by the offset + // amount. Size += Offset; for (uint64_t &FieldOffset : FieldOffsets) FieldOffset += Context.toBits(Offset); @@ -2699,18 +2646,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { } // Insert the virtual base. ElementInfo Info = getAdjustedElementInfo(BaseLayout); - CharUnits BaseOffset; - - // Respect the external AST source base offset, if present. - bool FoundBase = false; - if (UseExternalLayout) { - FoundBase = External.getExternalVBaseOffset(BaseDecl, BaseOffset); - if (FoundBase) - assert(BaseOffset >= Size && "base offset already allocated"); - } - if (!FoundBase) - BaseOffset = Size.RoundUpToAlignment(Info.Alignment); - + CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment); VBases.insert(std::make_pair(BaseDecl, ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp))); Size = BaseOffset + BaseLayout.getNonVirtualSize(); @@ -2740,12 +2676,6 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { else Size = MinEmptyStructSize; } - - if (UseExternalLayout) { - Size = Context.toCharUnitsFromBits(External.Size); - if (External.Align) - Alignment = Context.toCharUnitsFromBits(External.Align); - } } // Recursively walks the non-virtual bases of a class and determines if any of @@ -2884,7 +2814,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { const ASTRecordLayout *NewEntry = nullptr; - if (isMsLayout(D)) { + if (isMsLayout(D) && !D->getASTContext().getExternalSource()) { NewEntry = BuildMicrosoftASTRecordLayout(D); } else if (const CXXRecordDecl *RD = dyn_cast(D)) { EmptySubobjectMap EmptySubobjects(*this, RD); diff --git a/test/CodeGenCXX/override-layout.cpp b/test/CodeGenCXX/override-layout.cpp index a3c4bb446c..418c4ffab6 100644 --- a/test/CodeGenCXX/override-layout.cpp +++ b/test/CodeGenCXX/override-layout.cpp @@ -1,6 +1,6 @@ -// RUN: %clang_cc1 -w -fdump-record-layouts-simple %s > %t.layouts -// RUN: %clang_cc1 -w -fdump-record-layouts-simple %s > %t.before -// RUN: %clang_cc1 -w -DPACKED= -DALIGNED16= -fdump-record-layouts-simple -foverride-record-layout=%t.layouts %s > %t.after +// RUN: %clang_cc1 -fdump-record-layouts-simple %s > %t.layouts +// RUN: %clang_cc1 -fdump-record-layouts-simple %s > %t.before +// RUN: %clang_cc1 -DPACKED= -DALIGNED16= -fdump-record-layouts-simple -foverride-record-layout=%t.layouts %s > %t.after // RUN: diff -u %t.before %t.after // RUN: FileCheck %s < %t.after -- 2.40.0