=?utf-8?q?Théo?= De Magalhaes <theo.de-magalh...@epita.fr>, =?utf-8?q?Théo?= De Magalhaes <theo.de-magalh...@epita.fr>, =?utf-8?q?Théo?= De Magalhaes <theo.de-magalh...@epita.fr>, =?utf-8?q?Théo?= De Magalhaes <theo.de-magalh...@epita.fr>, =?utf-8?q?Théo?= De Magalhaes <theo.de-magalh...@epita.fr>, =?utf-8?q?Théo?= De Magalhaes <theo.de-magalh...@epita.fr>, =?utf-8?q?Théo?= De Magalhaes <theo.de-magalh...@epita.fr>, =?utf-8?q?Théo?= De Magalhaes <theo.de-magalh...@epita.fr> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/130...@github.com>
https://github.com/therealcoochieman updated https://github.com/llvm/llvm-project/pull/130182 >From 09572af03c07966925a3456a7d51ad9b3abb750b Mon Sep 17 00:00:00 2001 From: Theo de Magalhaes <theodemagalh...@icloud.com> Date: Thu, 6 Mar 2025 22:26:37 +0100 Subject: [PATCH 01/11] chore(clang-format): formatted clang/lib/AST/RecordLayoutBuilder.cpp --- clang/lib/AST/RecordLayoutBuilder.cpp | 199 ++++++++++++-------------- 1 file changed, 94 insertions(+), 105 deletions(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 3e756ab9b9bfe..9740c8e4cdd65 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -43,7 +43,7 @@ struct BaseSubobjectInfo { bool IsVirtual; /// Bases - Information about the base subobjects. - SmallVector<BaseSubobjectInfo*, 4> Bases; + SmallVector<BaseSubobjectInfo *, 4> Bases; /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base /// of this base info (if one exists). @@ -77,8 +77,7 @@ struct ExternalLayout { /// 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"); + assert(FieldOffsets.count(FD) && "Field does not have an external offset"); return FieldOffsets[FD]; } @@ -167,16 +166,15 @@ class EmptySubobjectMap { CharUnits SizeOfLargestEmptySubobject; EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class) - : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) { - ComputeEmptySubobjectSizes(); + : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) { + ComputeEmptySubobjectSizes(); } /// CanPlaceBaseAtOffset - Return whether the given base class can be placed /// at the given offset. /// Returns false if placing the record will result in two components /// (direct or indirect) of the same type having the same offset. - bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, - CharUnits Offset); + bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset); /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given /// offset. @@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() { } } -bool -EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD, - CharUnits Offset) const { +bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD, + CharUnits Offset) const { // We only need to check empty bases. if (!RD->isEmpty()) return true; @@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const CXXRecordDecl *RD, MaxEmptyClassOffset = Offset; } -bool -EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info, - CharUnits Offset) { +bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset( + const BaseSubobjectInfo *Info, CharUnits Offset) { // We don't have to keep looking past the maximum offset that's known to // contain an empty class. if (!AnyEmptySubobjectsBeyondOffset(Offset)) @@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, return true; } -bool -EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD, - const CXXRecordDecl *Class, - CharUnits Offset) const { +bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset( + const CXXRecordDecl *RD, const CXXRecordDecl *Class, + CharUnits Offset) const { // We don't have to keep looking past the maximum offset that's known to // contain an empty class. if (!AnyEmptySubobjectsBeyondOffset(Offset)) @@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD, return true; } -bool -EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD, - CharUnits Offset) const { +bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD, + CharUnits Offset) const { // We don't have to keep looking past the maximum offset that's known to // contain an empty class. if (!AnyEmptySubobjectsBeyondOffset(Offset)) @@ -560,7 +554,7 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects( } } -typedef llvm::SmallPtrSet<const CXXRecordDecl*, 4> ClassSetTy; +typedef llvm::SmallPtrSet<const CXXRecordDecl *, 4> ClassSetTy; class ItaniumRecordLayoutBuilder { protected: @@ -712,15 +706,13 @@ class ItaniumRecordLayoutBuilder { bool FieldPacked, const FieldDecl *D); void LayoutBitField(const FieldDecl *D); - TargetCXXABI getCXXABI() const { - return Context.getTargetInfo().getCXXABI(); - } + TargetCXXABI getCXXABI() const { return Context.getTargetInfo().getCXXABI(); } /// BaseSubobjectInfoAllocator - Allocator for BaseSubobjectInfo objects. llvm::SpecificBumpPtrAllocator<BaseSubobjectInfo> BaseSubobjectInfoAllocator; typedef llvm::DenseMap<const CXXRecordDecl *, BaseSubobjectInfo *> - BaseSubobjectInfoMapTy; + BaseSubobjectInfoMapTy; /// VirtualBaseInfo - Map from all the (direct or indirect) virtual bases /// of the class we're laying out to their base subobject info. @@ -793,8 +785,8 @@ class ItaniumRecordLayoutBuilder { uint64_t ComputedOffset); void CheckFieldPadding(uint64_t Offset, uint64_t UnpaddedOffset, - uint64_t UnpackedOffset, unsigned UnpackedAlign, - bool isPacked, const FieldDecl *D); + uint64_t UnpackedOffset, unsigned UnpackedAlign, + bool isPacked, const FieldDecl *D); DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID); @@ -965,8 +957,7 @@ BaseSubobjectInfo *ItaniumRecordLayoutBuilder::ComputeBaseSubobjectInfo( // 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!"); + assert(PrimaryVirtualBaseInfo && "Did not create a primary virtual base!"); // Claim the primary virtual base as our primary virtual base. Info->PrimaryVirtualBaseInfo = PrimaryVirtualBaseInfo; @@ -984,13 +975,12 @@ void ItaniumRecordLayoutBuilder::ComputeBaseSubobjectInfo( const CXXRecordDecl *BaseDecl = I.getType()->getAsCXXRecordDecl(); // Compute the base subobject info for this base. - BaseSubobjectInfo *Info = ComputeBaseSubobjectInfo(BaseDecl, IsVirtual, - nullptr); + BaseSubobjectInfo *Info = + ComputeBaseSubobjectInfo(BaseDecl, IsVirtual, nullptr); if (IsVirtual) { // ComputeBaseInfo has already added this base for us. - assert(VirtualBaseInfo.count(BaseDecl) && - "Did not add virtual base!"); + assert(VirtualBaseInfo.count(BaseDecl) && "Did not add virtual base!"); } else { // Add the base info to the map of non-virtual bases. assert(!NonVirtualBaseInfo.count(BaseDecl) && @@ -1043,15 +1033,15 @@ void ItaniumRecordLayoutBuilder::LayoutNonVirtualBases( LayoutVirtualBase(PrimaryBaseInfo); } else { BaseSubobjectInfo *PrimaryBaseInfo = - NonVirtualBaseInfo.lookup(PrimaryBase); + NonVirtualBaseInfo.lookup(PrimaryBase); assert(PrimaryBaseInfo && "Did not find base info for non-virtual primary base!"); LayoutNonVirtualBase(PrimaryBaseInfo); } - // If this class needs a vtable/vf-table and didn't get one from a - // primary base, add it in now. + // If this class needs a vtable/vf-table and didn't get one from a + // primary base, add it in now. } else if (RD->isDynamicClass()) { assert(DataSize == 0 && "Vtable pointer must be at offset zero!"); CharUnits PtrWidth = Context.toCharUnitsFromBits( @@ -1191,8 +1181,8 @@ void ItaniumRecordLayoutBuilder::LayoutVirtualBase( // Add its base class offset. assert(!VBases.count(Base->Class) && "vbase offset already exists!"); - VBases.insert(std::make_pair(Base->Class, - ASTRecordLayout::VBaseInfo(Offset, false))); + VBases.insert( + std::make_pair(Base->Class, ASTRecordLayout::VBaseInfo(Offset, false))); AddPrimaryVirtualBaseOffsets(Base, Offset); } @@ -1450,9 +1440,8 @@ void ItaniumRecordLayoutBuilder::LayoutFields(const RecordDecl *D) { } // Rounds the specified size to have it a multiple of the char size. -static uint64_t -roundUpSizeToCharAlignment(uint64_t Size, - const ASTContext &Context) { +static uint64_t roundUpSizeToCharAlignment(uint64_t Size, + const ASTContext &Context) { uint64_t CharAlignment = Context.getTargetInfo().getCharAlign(); return llvm::alignTo(Size, CharAlignment); } @@ -1497,8 +1486,7 @@ void ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize, uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit; if (IsUnion) { - uint64_t RoundedFieldSize = roundUpSizeToCharAlignment(FieldSize, - Context); + uint64_t RoundedFieldSize = roundUpSizeToCharAlignment(FieldSize, Context); setDataSize(std::max(getDataSizeInBits(), RoundedFieldSize)); FieldOffset = 0; } else { @@ -1648,7 +1636,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { // Compute the next available bit offset. uint64_t FieldOffset = - IsUnion ? 0 : (getDataSizeInBits() - UnfilledBitsInLastUnit); + IsUnion ? 0 : (getDataSizeInBits() - UnfilledBitsInLastUnit); // Handle targets that don't honor bitfield type alignment. if (!IsMsStruct && !Context.getTargetInfo().useBitFieldTypeAlignment()) { @@ -1666,7 +1654,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { Context.getTargetInfo().getZeroLengthBitfieldBoundary(); FieldAlign = std::max(FieldAlign, ZeroLengthBitfieldBoundary); } - // If that doesn't apply, just ignore the field alignment. + // If that doesn't apply, just ignore the field alignment. } else { FieldAlign = 1; } @@ -1810,8 +1798,8 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { } setDataSize(std::max(getDataSizeInBits(), RoundedFieldSize)); - // For non-zero-width bitfields in ms_struct structs, allocate a new - // storage unit if necessary. + // For non-zero-width bitfields in ms_struct structs, allocate a new + // storage unit if necessary. } else if (IsMsStruct && FieldSize) { // We should have cleared UnfilledBitsInLastUnit in every case // where we changed storage units. @@ -1960,13 +1948,13 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D, } } - bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() || - FieldClass->hasAttr<PackedAttr>() || - Context.getLangOpts().getClangABICompat() <= - LangOptions::ClangABI::Ver15 || - Target.isPS() || Target.isOSDarwin() || - Target.isOSAIX())) || - D->hasAttr<PackedAttr>(); + bool FieldPacked = + (Packed && (!FieldClass || FieldClass->isPOD() || + FieldClass->hasAttr<PackedAttr>() || + Context.getLangOpts().getClangABICompat() <= + LangOptions::ClangABI::Ver15 || + Target.isPS() || Target.isOSDarwin() || Target.isOSAIX())) || + D->hasAttr<PackedAttr>(); // When used as part of a typedef, or together with a 'packed' attribute, the // 'aligned' attribute can be used to decrease alignment. In that case, it @@ -2036,7 +2024,6 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D, UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignment); } - if (!FieldPacked) FieldAlign = UnpackedFieldAlign; if (DefaultsToAIXPowerAlignment) @@ -2142,8 +2129,7 @@ void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) { // array of zero-length, remains of Size 0 if (RD->isEmpty()) setSize(CharUnits::One()); - } - else + } else setSize(CharUnits::One()); } @@ -2190,8 +2176,7 @@ void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) { InBits = false; } Diag(RD->getLocation(), diag::warn_padded_struct_size) - << Context.getTypeDeclType(RD) - << PadSize + << Context.getTypeDeclType(RD) << PadSize << (InBits ? 1 : 0); // (byte|bit) } @@ -2270,7 +2255,8 @@ static unsigned getPaddingDiagFromTagKind(TagTypeKind Tag) { return 1; case TagTypeKind::Class: return 2; - default: llvm_unreachable("Invalid tag kind for field padding diagnostic!"); + default: + llvm_unreachable("Invalid tag kind for field padding diagnostic!"); } } @@ -2313,10 +2299,10 @@ void ItaniumRecordLayoutBuilder::CheckFieldPadding( << Context.getTypeDeclType(D->getParent()) << PadSize << (InBits ? 1 : 0); // (byte|bit) } - } - if (isPacked && Offset != UnpackedOffset) { - HasPackedField = true; - } + } + if (isPacked && Offset != UnpackedOffset) { + HasPackedField = true; + } } static const CXXMethodDecl *computeKeyFunction(ASTContext &Context, @@ -2340,7 +2326,7 @@ static const CXXMethodDecl *computeKeyFunction(ASTContext &Context, return nullptr; bool allowInlineFunctions = - Context.getTargetInfo().getCXXABI().canKeyFunctionBeInline(); + Context.getTargetInfo().getCXXABI().canKeyFunctionBeInline(); for (const CXXMethodDecl *MD : RD->methods()) { if (!MD->isVirtual()) @@ -2560,6 +2546,7 @@ struct MicrosoftRecordLayoutBuilder { private: MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete; void operator=(const MicrosoftRecordLayoutBuilder &) = delete; + public: void layout(const RecordDecl *RD); void cxxLayout(const CXXRecordDecl *RD); @@ -2684,15 +2671,15 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( // the alignment in the case of pragma pack. Note that the required alignment // doesn't actually apply to the struct alignment at this point. Alignment = std::max(Alignment, Info.Alignment); - RequiredAlignment = std::max(RequiredAlignment, Layout.getRequiredAlignment()); + RequiredAlignment = + std::max(RequiredAlignment, Layout.getRequiredAlignment()); Info.Alignment = std::max(Info.Alignment, Layout.getRequiredAlignment()); Info.Size = Layout.getNonVirtualSize(); return Info; } MicrosoftRecordLayoutBuilder::ElementInfo -MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( - const FieldDecl *FD) { +MicrosoftRecordLayoutBuilder::getAdjustedElementInfo(const FieldDecl *FD) { // Get the alignment of the field type's natural alignment, ignore any // alignment attributes. auto TInfo = @@ -2715,8 +2702,8 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( FD->getType()->getBaseElementTypeUnsafe()->getAs<RecordType>()) { auto const &Layout = Context.getASTRecordLayout(RT->getDecl()); EndsWithZeroSizedObject = Layout.endsWithZeroSizedObject(); - FieldRequiredAlignment = std::max(FieldRequiredAlignment, - Layout.getRequiredAlignment()); + FieldRequiredAlignment = + std::max(FieldRequiredAlignment, Layout.getRequiredAlignment()); } // Capture required alignment as a side-effect. RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment); @@ -2778,10 +2765,11 @@ void MicrosoftRecordLayoutBuilder::initializeLayout(const RecordDecl *RD) { MaxFieldAlignment = CharUnits::Zero(); // Honor the default struct packing maximum alignment flag. if (unsigned DefaultMaxFieldAlignment = Context.getLangOpts().PackStruct) - MaxFieldAlignment = CharUnits::fromQuantity(DefaultMaxFieldAlignment); + MaxFieldAlignment = CharUnits::fromQuantity(DefaultMaxFieldAlignment); // Honor the packing attribute. The MS-ABI ignores pragma pack if its larger // than the pointer size. - if (const MaxFieldAlignmentAttr *MFAA = RD->getAttr<MaxFieldAlignmentAttr>()){ + if (const MaxFieldAlignmentAttr *MFAA = + RD->getAttr<MaxFieldAlignmentAttr>()) { unsigned PackedAlignment = MFAA->getAlignment(); if (PackedAlignment <= Context.getTargetInfo().getPointerWidth(LangAS::Default)) @@ -2799,8 +2787,8 @@ void MicrosoftRecordLayoutBuilder::initializeLayout(const RecordDecl *RD) { External.BaseOffsets, External.VirtualBaseOffsets); } -void -MicrosoftRecordLayoutBuilder::initializeCXXLayout(const CXXRecordDecl *RD) { +void MicrosoftRecordLayoutBuilder::initializeCXXLayout( + const CXXRecordDecl *RD) { EndsWithZeroSizedObject = false; LeadsWithZeroSizedBase = false; HasOwnVFPtr = false; @@ -2818,8 +2806,8 @@ MicrosoftRecordLayoutBuilder::initializeCXXLayout(const CXXRecordDecl *RD) { PointerInfo.Alignment = std::min(PointerInfo.Alignment, MaxFieldAlignment); } -void -MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { +void MicrosoftRecordLayoutBuilder::layoutNonVirtualBases( + const CXXRecordDecl *RD) { // The MS-ABI lays out all bases that contain leading vfptrs before it lays // out any bases that do not contain vfptrs. We implement this as two passes // over the bases. This approach guarantees that the primary base is laid out @@ -3057,8 +3045,8 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { DataSize = Size; } -void -MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) { +void MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField( + const FieldDecl *FD) { // Zero-width bitfields are ignored unless they follow a non-zero-width // bitfield. if (!LastFieldIsNonZeroWidthBitfield) { @@ -3195,8 +3183,8 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { assert(BaseOffset >= Size && "base offset already allocated"); - VBases.insert(std::make_pair(BaseDecl, - ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp))); + VBases.insert(std::make_pair( + BaseDecl, ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp))); Size = BaseOffset + BaseLayout.getNonVirtualSize(); PreviousBaseLayout = &BaseLayout; } @@ -3236,10 +3224,9 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { // Recursively walks the non-virtual bases of a class and determines if any of // them are in the bases with overridden methods set. -static bool -RequiresVtordisp(const llvm::SmallPtrSetImpl<const CXXRecordDecl *> & - BasesWithOverriddenMethods, - const CXXRecordDecl *RD) { +static bool RequiresVtordisp(const llvm::SmallPtrSetImpl<const CXXRecordDecl *> + &BasesWithOverriddenMethods, + const CXXRecordDecl *RD) { if (BasesWithOverriddenMethods.count(RD)) return true; // If any of a virtual bases non-virtual bases (recursively) requires a @@ -3329,7 +3316,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { // until we *finish* parsing the definition. if (D->hasExternalLexicalStorage() && !D->getDefinition()) - getExternalSource()->CompleteType(const_cast<RecordDecl*>(D)); + getExternalSource()->CompleteType(const_cast<RecordDecl *>(D)); // Complete the redecl chain (if necessary). (void)D->getMostRecentDecl(); @@ -3342,7 +3329,8 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { // Note that we can't save a reference to the entry because this function // is recursive. const ASTRecordLayout *Entry = ASTRecordLayouts[D]; - if (Entry) return *Entry; + if (Entry) + return *Entry; const ASTRecordLayout *NewEntry = nullptr; @@ -3418,7 +3406,8 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { return *NewEntry; } -const CXXMethodDecl *ASTContext::getCurrentKeyFunction(const CXXRecordDecl *RD) { +const CXXMethodDecl * +ASTContext::getCurrentKeyFunction(const CXXRecordDecl *RD) { if (!getTargetInfo().getCXXABI().hasKeyFunctions()) return nullptr; @@ -3436,7 +3425,7 @@ const CXXMethodDecl *ASTContext::getCurrentKeyFunction(const CXXRecordDecl *RD) // Store it back if it changed. if (Entry.isOffset() || Entry.isValid() != bool(Result)) - KeyFunctions[RD] = const_cast<Decl*>(Result); + KeyFunctions[RD] = const_cast<Decl *>(Result); return cast_or_null<CXXMethodDecl>(Result); } @@ -3452,7 +3441,8 @@ void ASTContext::setNonKeyFunction(const CXXMethodDecl *Method) { auto I = Map.find(Method->getParent()); // If it's not cached, there's nothing to do. - if (I == Map.end()) return; + if (I == Map.end()) + return; // If it is cached, check whether it's the target method, and if so, // remove it from the cache. Note, the call to 'get' might invalidate @@ -3497,8 +3487,8 @@ uint64_t ASTContext::lookupFieldBitOffset(const ObjCInterfaceDecl *OID, // directly. unsigned Index = 0; - for (const ObjCIvarDecl *IVD = Container->all_declared_ivar_begin(); - IVD; IVD = IVD->getNextIvar()) { + for (const ObjCIvarDecl *IVD = Container->all_declared_ivar_begin(); IVD; + IVD = IVD->getNextIvar()) { if (Ivar == IVD) break; ++Index; @@ -3517,7 +3507,7 @@ const ASTRecordLayout & ASTContext::getObjCLayout(const ObjCInterfaceDecl *D) const { // Retrieve the definition if (D->hasExternalLexicalStorage() && !D->getDefinition()) - getExternalSource()->CompleteType(const_cast<ObjCInterfaceDecl*>(D)); + getExternalSource()->CompleteType(const_cast<ObjCInterfaceDecl *>(D)); D = D->getDefinition(); assert(D && !D->isInvalidDecl() && D->isThisDeclarationADefinition() && "Invalid interface decl!"); @@ -3540,8 +3530,8 @@ ASTContext::getObjCLayout(const ObjCInterfaceDecl *D) const { return *NewEntry; } -static void PrintOffset(raw_ostream &OS, - CharUnits Offset, unsigned IndentLevel) { +static void PrintOffset(raw_ostream &OS, CharUnits Offset, + unsigned IndentLevel) { OS << llvm::format("%10" PRId64 " | ", (int64_t)Offset.getQuantity()); OS.indent(IndentLevel * 2); } @@ -3570,12 +3560,9 @@ static void PrintIndentNoOffset(raw_ostream &OS, unsigned IndentLevel) { } static void DumpRecordLayout(raw_ostream &OS, const RecordDecl *RD, - const ASTContext &C, - CharUnits Offset, - unsigned IndentLevel, - const char* Description, - bool PrintSizeInfo, - bool IncludeVirtualBases) { + const ASTContext &C, CharUnits Offset, + unsigned IndentLevel, const char *Description, + bool PrintSizeInfo, bool IncludeVirtualBases) { const ASTRecordLayout &Layout = C.getASTRecordLayout(RD); auto CXXRD = dyn_cast<CXXRecordDecl>(RD); @@ -3641,7 +3628,7 @@ static void DumpRecordLayout(raw_ostream &OS, const RecordDecl *RD, uint64_t LocalFieldOffsetInBits = Layout.getFieldOffset(Field->getFieldIndex()); CharUnits FieldOffset = - Offset + C.toCharUnitsFromBits(LocalFieldOffsetInBits); + Offset + C.toCharUnitsFromBits(LocalFieldOffsetInBits); // Recursively dump fields of record type. if (auto RT = Field->getType()->getAs<RecordType>()) { @@ -3669,7 +3656,7 @@ static void DumpRecordLayout(raw_ostream &OS, const RecordDecl *RD, // Dump virtual bases. if (CXXRD && IncludeVirtualBases) { const ASTRecordLayout::VBaseOffsetsMapTy &VtorDisps = - Layout.getVBaseOffsetsMap(); + Layout.getVBaseOffsetsMap(); for (const CXXBaseSpecifier &Base : CXXRD->vbases()) { assert(Base.isVirtual() && "Found non-virtual class!"); @@ -3683,14 +3670,16 @@ static void DumpRecordLayout(raw_ostream &OS, const RecordDecl *RD, } DumpRecordLayout(OS, VBase, C, VBaseOffset, IndentLevel, - VBase == Layout.getPrimaryBase() ? - "(primary virtual base)" : "(virtual base)", + VBase == Layout.getPrimaryBase() + ? "(primary virtual base)" + : "(virtual base)", /*PrintSizeInfo=*/false, /*IncludeVirtualBases=*/false); } } - if (!PrintSizeInfo) return; + if (!PrintSizeInfo) + return; PrintIndentNoOffset(OS, IndentLevel - 1); OS << "[sizeof=" << Layout.getSize().getQuantity(); >From 60ddbafd6c55c35fee92a0681f60305d3efa8cde Mon Sep 17 00:00:00 2001 From: Theo de Magalhaes <theodemagalh...@icloud.com> Date: Thu, 6 Mar 2025 22:48:49 +0100 Subject: [PATCH 02/11] fix(clang-cl): added -Wpadded and -Wpadded-bitfield warning --- clang/lib/AST/RecordLayoutBuilder.cpp | 86 ++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 9740c8e4cdd65..d4164179c67af 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2592,6 +2592,8 @@ struct MicrosoftRecordLayoutBuilder { void computeVtorDispSet( llvm::SmallPtrSetImpl<const CXXRecordDecl *> &HasVtorDispSet, const CXXRecordDecl *RD) const; + void CheckFieldPadding(uint64_t Offset, uint64_t UnpaddedOffset, + const FieldDecl *D); const ASTContext &Context; EmptySubobjectMap *EmptySubobjects; @@ -2629,8 +2631,6 @@ struct MicrosoftRecordLayoutBuilder { /// virtual base classes and their offsets in the record. ASTRecordLayout::VBaseOffsetsMapTy VBases; /// The number of remaining bits in our last bitfield allocation. - /// This value isn't meaningful unless LastFieldIsNonZeroWidthBitfield is - /// true. unsigned RemainingBitsInField; bool IsUnion : 1; /// True if the last field laid out was a bitfield and was not 0 @@ -2992,6 +2992,15 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) { } else { FieldOffset = Size.alignTo(Info.Alignment); } + + uint64_t UnpaddedFielddOffsetInBits = + Context.toBits(DataSize) - RemainingBitsInField; + + CheckFieldPadding(Context.toBits(FieldOffset), UnpaddedFielddOffsetInBits, + FD); + + RemainingBitsInField = 0; + placeFieldAtOffset(FieldOffset); if (!IsOverlappingEmptyField) @@ -3037,10 +3046,14 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { } else { // Allocate a new block of memory and place the bitfield in it. CharUnits FieldOffset = Size.alignTo(Info.Alignment); + uint64_t UnpaddedFieldOffsetInBits = + Context.toBits(DataSize) - RemainingBitsInField; placeFieldAtOffset(FieldOffset); Size = FieldOffset + Info.Size; Alignment = std::max(Alignment, Info.Alignment); RemainingBitsInField = Context.toBits(Info.Size) - Width; + CheckFieldPadding(Context.toBits(FieldOffset), UnpaddedFieldOffsetInBits, + FD); } DataSize = Size; } @@ -3064,9 +3077,14 @@ void MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField( } else { // Round up the current record size to the field's alignment boundary. CharUnits FieldOffset = Size.alignTo(Info.Alignment); + uint64_t UnpaddedFieldOffsetInBits = + Context.toBits(DataSize) - RemainingBitsInField; placeFieldAtOffset(FieldOffset); + RemainingBitsInField = 0; Size = FieldOffset; Alignment = std::max(Alignment, Info.Alignment); + CheckFieldPadding(Context.toBits(FieldOffset), UnpaddedFieldOffsetInBits, + FD); } DataSize = Size; } @@ -3189,8 +3207,56 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { PreviousBaseLayout = &BaseLayout; } } +// Function based on ItaniumRecordLayoutBuilder::CheckFieldPadding. +void MicrosoftRecordLayoutBuilder::CheckFieldPadding(uint64_t Offset, + uint64_t UnpaddedOffset, + const FieldDecl *D) { + + // We let objc ivars without warning, objc interfaces generally are not used + // for padding tricks. + if (isa<ObjCIvarDecl>(D)) + return; + + // Don't warn about structs created without a SourceLocation. This can + // be done by clients of the AST, such as codegen. + if (D->getLocation().isInvalid()) + return; + + unsigned CharBitNum = Context.getTargetInfo().getCharWidth(); + + // Warn if padding was introduced to the struct/class. + if (!IsUnion && Offset > UnpaddedOffset) { + unsigned PadSize = Offset - UnpaddedOffset; + bool InBits = true; + if (PadSize % CharBitNum == 0) { + PadSize = PadSize / CharBitNum; + InBits = false; + } + if (D->getIdentifier()) { + auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_bitfield + : diag::warn_padded_struct_field; + Context.getDiagnostics().Report(D->getLocation(), + Diagnostic) + << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) + << Context.getTypeDeclType(D->getParent()) << PadSize + << (InBits ? 1 : 0) // (byte|bit) + << D->getIdentifier(); + } else { + auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_anon_bitfield + : diag::warn_padded_struct_anon_field; + Context.getDiagnostics().Report(D->getLocation(), + Diagnostic) + << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) + << Context.getTypeDeclType(D->getParent()) << PadSize + << (InBits ? 1 : 0); // (byte|bit) + } + } +} void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { + uint64_t UnpaddedSizeInBits = Context.toBits(DataSize); + UnpaddedSizeInBits -= RemainingBitsInField; + // Respect required alignment. Note that in 32-bit mode Required alignment // may be 0 and cause size not to be updated. DataSize = Size; @@ -3219,6 +3285,22 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { Size = Context.toCharUnitsFromBits(External.Size); if (External.Align) Alignment = Context.toCharUnitsFromBits(External.Align); + return; + } + unsigned CharBitNum = Context.getTargetInfo().getCharWidth(); + uint64_t SizeInBits = Context.toBits(Size); + if (SizeInBits > UnpaddedSizeInBits) { + unsigned int PadSize = SizeInBits - UnpaddedSizeInBits; + bool InBits = true; + if (PadSize % CharBitNum == 0) { + PadSize = PadSize / CharBitNum; + InBits = false; + } + + Context.getDiagnostics().Report(RD->getLocation(), + diag::warn_padded_struct_size) + << Context.getTypeDeclType(RD) << PadSize + << (InBits ? 1 : 0); // (byte|bit) } } >From 05048948ef58aaca06f72359899984a313bca5bc Mon Sep 17 00:00:00 2001 From: Theo de Magalhaes <theodemagalh...@icloud.com> Date: Fri, 7 Mar 2025 10:17:48 +0100 Subject: [PATCH 03/11] Revert "chore(clang-format): formatted clang/lib/AST/RecordLayoutBuilder.cpp" This reverts commit 3a9bc908a791669c82f288ff745664411bf285ac. --- clang/lib/AST/RecordLayoutBuilder.cpp | 199 ++++++++++++++------------ 1 file changed, 105 insertions(+), 94 deletions(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index d4164179c67af..68d70ebadddcb 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -43,7 +43,7 @@ struct BaseSubobjectInfo { bool IsVirtual; /// Bases - Information about the base subobjects. - SmallVector<BaseSubobjectInfo *, 4> Bases; + SmallVector<BaseSubobjectInfo*, 4> Bases; /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base /// of this base info (if one exists). @@ -77,7 +77,8 @@ struct ExternalLayout { /// 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"); + assert(FieldOffsets.count(FD) && + "Field does not have an external offset"); return FieldOffsets[FD]; } @@ -166,15 +167,16 @@ class EmptySubobjectMap { CharUnits SizeOfLargestEmptySubobject; EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class) - : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) { - ComputeEmptySubobjectSizes(); + : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) { + ComputeEmptySubobjectSizes(); } /// CanPlaceBaseAtOffset - Return whether the given base class can be placed /// at the given offset. /// Returns false if placing the record will result in two components /// (direct or indirect) of the same type having the same offset. - bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset); + bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, + CharUnits Offset); /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given /// offset. @@ -225,8 +227,9 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() { } } -bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD, - CharUnits Offset) const { +bool +EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD, + CharUnits Offset) const { // We only need to check empty bases. if (!RD->isEmpty()) return true; @@ -262,8 +265,9 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const CXXRecordDecl *RD, MaxEmptyClassOffset = Offset; } -bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset( - const BaseSubobjectInfo *Info, CharUnits Offset) { +bool +EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info, + CharUnits Offset) { // We don't have to keep looking past the maximum offset that's known to // contain an empty class. if (!AnyEmptySubobjectsBeyondOffset(Offset)) @@ -364,9 +368,10 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, return true; } -bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset( - const CXXRecordDecl *RD, const CXXRecordDecl *Class, - CharUnits Offset) const { +bool +EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD, + const CXXRecordDecl *Class, + CharUnits Offset) const { // We don't have to keep looking past the maximum offset that's known to // contain an empty class. if (!AnyEmptySubobjectsBeyondOffset(Offset)) @@ -413,8 +418,9 @@ bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset( return true; } -bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD, - CharUnits Offset) const { +bool +EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD, + CharUnits Offset) const { // We don't have to keep looking past the maximum offset that's known to // contain an empty class. if (!AnyEmptySubobjectsBeyondOffset(Offset)) @@ -554,7 +560,7 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects( } } -typedef llvm::SmallPtrSet<const CXXRecordDecl *, 4> ClassSetTy; +typedef llvm::SmallPtrSet<const CXXRecordDecl*, 4> ClassSetTy; class ItaniumRecordLayoutBuilder { protected: @@ -706,13 +712,15 @@ class ItaniumRecordLayoutBuilder { bool FieldPacked, const FieldDecl *D); void LayoutBitField(const FieldDecl *D); - TargetCXXABI getCXXABI() const { return Context.getTargetInfo().getCXXABI(); } + TargetCXXABI getCXXABI() const { + return Context.getTargetInfo().getCXXABI(); + } /// BaseSubobjectInfoAllocator - Allocator for BaseSubobjectInfo objects. llvm::SpecificBumpPtrAllocator<BaseSubobjectInfo> BaseSubobjectInfoAllocator; typedef llvm::DenseMap<const CXXRecordDecl *, BaseSubobjectInfo *> - BaseSubobjectInfoMapTy; + BaseSubobjectInfoMapTy; /// VirtualBaseInfo - Map from all the (direct or indirect) virtual bases /// of the class we're laying out to their base subobject info. @@ -785,8 +793,8 @@ class ItaniumRecordLayoutBuilder { uint64_t ComputedOffset); void CheckFieldPadding(uint64_t Offset, uint64_t UnpaddedOffset, - uint64_t UnpackedOffset, unsigned UnpackedAlign, - bool isPacked, const FieldDecl *D); + uint64_t UnpackedOffset, unsigned UnpackedAlign, + bool isPacked, const FieldDecl *D); DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID); @@ -957,7 +965,8 @@ BaseSubobjectInfo *ItaniumRecordLayoutBuilder::ComputeBaseSubobjectInfo( // 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!"); + assert(PrimaryVirtualBaseInfo && + "Did not create a primary virtual base!"); // Claim the primary virtual base as our primary virtual base. Info->PrimaryVirtualBaseInfo = PrimaryVirtualBaseInfo; @@ -975,12 +984,13 @@ void ItaniumRecordLayoutBuilder::ComputeBaseSubobjectInfo( const CXXRecordDecl *BaseDecl = I.getType()->getAsCXXRecordDecl(); // Compute the base subobject info for this base. - BaseSubobjectInfo *Info = - ComputeBaseSubobjectInfo(BaseDecl, IsVirtual, nullptr); + BaseSubobjectInfo *Info = ComputeBaseSubobjectInfo(BaseDecl, IsVirtual, + nullptr); if (IsVirtual) { // ComputeBaseInfo has already added this base for us. - assert(VirtualBaseInfo.count(BaseDecl) && "Did not add virtual base!"); + assert(VirtualBaseInfo.count(BaseDecl) && + "Did not add virtual base!"); } else { // Add the base info to the map of non-virtual bases. assert(!NonVirtualBaseInfo.count(BaseDecl) && @@ -1033,15 +1043,15 @@ void ItaniumRecordLayoutBuilder::LayoutNonVirtualBases( LayoutVirtualBase(PrimaryBaseInfo); } else { BaseSubobjectInfo *PrimaryBaseInfo = - NonVirtualBaseInfo.lookup(PrimaryBase); + NonVirtualBaseInfo.lookup(PrimaryBase); assert(PrimaryBaseInfo && "Did not find base info for non-virtual primary base!"); LayoutNonVirtualBase(PrimaryBaseInfo); } - // If this class needs a vtable/vf-table and didn't get one from a - // primary base, add it in now. + // If this class needs a vtable/vf-table and didn't get one from a + // primary base, add it in now. } else if (RD->isDynamicClass()) { assert(DataSize == 0 && "Vtable pointer must be at offset zero!"); CharUnits PtrWidth = Context.toCharUnitsFromBits( @@ -1181,8 +1191,8 @@ void ItaniumRecordLayoutBuilder::LayoutVirtualBase( // Add its base class offset. assert(!VBases.count(Base->Class) && "vbase offset already exists!"); - VBases.insert( - std::make_pair(Base->Class, ASTRecordLayout::VBaseInfo(Offset, false))); + VBases.insert(std::make_pair(Base->Class, + ASTRecordLayout::VBaseInfo(Offset, false))); AddPrimaryVirtualBaseOffsets(Base, Offset); } @@ -1440,8 +1450,9 @@ void ItaniumRecordLayoutBuilder::LayoutFields(const RecordDecl *D) { } // Rounds the specified size to have it a multiple of the char size. -static uint64_t roundUpSizeToCharAlignment(uint64_t Size, - const ASTContext &Context) { +static uint64_t +roundUpSizeToCharAlignment(uint64_t Size, + const ASTContext &Context) { uint64_t CharAlignment = Context.getTargetInfo().getCharAlign(); return llvm::alignTo(Size, CharAlignment); } @@ -1486,7 +1497,8 @@ void ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize, uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit; if (IsUnion) { - uint64_t RoundedFieldSize = roundUpSizeToCharAlignment(FieldSize, Context); + uint64_t RoundedFieldSize = roundUpSizeToCharAlignment(FieldSize, + Context); setDataSize(std::max(getDataSizeInBits(), RoundedFieldSize)); FieldOffset = 0; } else { @@ -1636,7 +1648,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { // Compute the next available bit offset. uint64_t FieldOffset = - IsUnion ? 0 : (getDataSizeInBits() - UnfilledBitsInLastUnit); + IsUnion ? 0 : (getDataSizeInBits() - UnfilledBitsInLastUnit); // Handle targets that don't honor bitfield type alignment. if (!IsMsStruct && !Context.getTargetInfo().useBitFieldTypeAlignment()) { @@ -1654,7 +1666,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { Context.getTargetInfo().getZeroLengthBitfieldBoundary(); FieldAlign = std::max(FieldAlign, ZeroLengthBitfieldBoundary); } - // If that doesn't apply, just ignore the field alignment. + // If that doesn't apply, just ignore the field alignment. } else { FieldAlign = 1; } @@ -1798,8 +1810,8 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { } setDataSize(std::max(getDataSizeInBits(), RoundedFieldSize)); - // For non-zero-width bitfields in ms_struct structs, allocate a new - // storage unit if necessary. + // For non-zero-width bitfields in ms_struct structs, allocate a new + // storage unit if necessary. } else if (IsMsStruct && FieldSize) { // We should have cleared UnfilledBitsInLastUnit in every case // where we changed storage units. @@ -1948,13 +1960,13 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D, } } - bool FieldPacked = - (Packed && (!FieldClass || FieldClass->isPOD() || - FieldClass->hasAttr<PackedAttr>() || - Context.getLangOpts().getClangABICompat() <= - LangOptions::ClangABI::Ver15 || - Target.isPS() || Target.isOSDarwin() || Target.isOSAIX())) || - D->hasAttr<PackedAttr>(); + bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() || + FieldClass->hasAttr<PackedAttr>() || + Context.getLangOpts().getClangABICompat() <= + LangOptions::ClangABI::Ver15 || + Target.isPS() || Target.isOSDarwin() || + Target.isOSAIX())) || + D->hasAttr<PackedAttr>(); // When used as part of a typedef, or together with a 'packed' attribute, the // 'aligned' attribute can be used to decrease alignment. In that case, it @@ -2024,6 +2036,7 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D, UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignment); } + if (!FieldPacked) FieldAlign = UnpackedFieldAlign; if (DefaultsToAIXPowerAlignment) @@ -2129,7 +2142,8 @@ void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) { // array of zero-length, remains of Size 0 if (RD->isEmpty()) setSize(CharUnits::One()); - } else + } + else setSize(CharUnits::One()); } @@ -2176,7 +2190,8 @@ void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) { InBits = false; } Diag(RD->getLocation(), diag::warn_padded_struct_size) - << Context.getTypeDeclType(RD) << PadSize + << Context.getTypeDeclType(RD) + << PadSize << (InBits ? 1 : 0); // (byte|bit) } @@ -2255,8 +2270,7 @@ static unsigned getPaddingDiagFromTagKind(TagTypeKind Tag) { return 1; case TagTypeKind::Class: return 2; - default: - llvm_unreachable("Invalid tag kind for field padding diagnostic!"); + default: llvm_unreachable("Invalid tag kind for field padding diagnostic!"); } } @@ -2299,10 +2313,10 @@ void ItaniumRecordLayoutBuilder::CheckFieldPadding( << Context.getTypeDeclType(D->getParent()) << PadSize << (InBits ? 1 : 0); // (byte|bit) } - } - if (isPacked && Offset != UnpackedOffset) { - HasPackedField = true; - } + } + if (isPacked && Offset != UnpackedOffset) { + HasPackedField = true; + } } static const CXXMethodDecl *computeKeyFunction(ASTContext &Context, @@ -2326,7 +2340,7 @@ static const CXXMethodDecl *computeKeyFunction(ASTContext &Context, return nullptr; bool allowInlineFunctions = - Context.getTargetInfo().getCXXABI().canKeyFunctionBeInline(); + Context.getTargetInfo().getCXXABI().canKeyFunctionBeInline(); for (const CXXMethodDecl *MD : RD->methods()) { if (!MD->isVirtual()) @@ -2546,7 +2560,6 @@ struct MicrosoftRecordLayoutBuilder { private: MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete; void operator=(const MicrosoftRecordLayoutBuilder &) = delete; - public: void layout(const RecordDecl *RD); void cxxLayout(const CXXRecordDecl *RD); @@ -2671,15 +2684,15 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( // the alignment in the case of pragma pack. Note that the required alignment // doesn't actually apply to the struct alignment at this point. Alignment = std::max(Alignment, Info.Alignment); - RequiredAlignment = - std::max(RequiredAlignment, Layout.getRequiredAlignment()); + RequiredAlignment = std::max(RequiredAlignment, Layout.getRequiredAlignment()); Info.Alignment = std::max(Info.Alignment, Layout.getRequiredAlignment()); Info.Size = Layout.getNonVirtualSize(); return Info; } MicrosoftRecordLayoutBuilder::ElementInfo -MicrosoftRecordLayoutBuilder::getAdjustedElementInfo(const FieldDecl *FD) { +MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( + const FieldDecl *FD) { // Get the alignment of the field type's natural alignment, ignore any // alignment attributes. auto TInfo = @@ -2702,8 +2715,8 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo(const FieldDecl *FD) { FD->getType()->getBaseElementTypeUnsafe()->getAs<RecordType>()) { auto const &Layout = Context.getASTRecordLayout(RT->getDecl()); EndsWithZeroSizedObject = Layout.endsWithZeroSizedObject(); - FieldRequiredAlignment = - std::max(FieldRequiredAlignment, Layout.getRequiredAlignment()); + FieldRequiredAlignment = std::max(FieldRequiredAlignment, + Layout.getRequiredAlignment()); } // Capture required alignment as a side-effect. RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment); @@ -2765,11 +2778,10 @@ void MicrosoftRecordLayoutBuilder::initializeLayout(const RecordDecl *RD) { MaxFieldAlignment = CharUnits::Zero(); // Honor the default struct packing maximum alignment flag. if (unsigned DefaultMaxFieldAlignment = Context.getLangOpts().PackStruct) - MaxFieldAlignment = CharUnits::fromQuantity(DefaultMaxFieldAlignment); + MaxFieldAlignment = CharUnits::fromQuantity(DefaultMaxFieldAlignment); // Honor the packing attribute. The MS-ABI ignores pragma pack if its larger // than the pointer size. - if (const MaxFieldAlignmentAttr *MFAA = - RD->getAttr<MaxFieldAlignmentAttr>()) { + if (const MaxFieldAlignmentAttr *MFAA = RD->getAttr<MaxFieldAlignmentAttr>()){ unsigned PackedAlignment = MFAA->getAlignment(); if (PackedAlignment <= Context.getTargetInfo().getPointerWidth(LangAS::Default)) @@ -2787,8 +2799,8 @@ void MicrosoftRecordLayoutBuilder::initializeLayout(const RecordDecl *RD) { External.BaseOffsets, External.VirtualBaseOffsets); } -void MicrosoftRecordLayoutBuilder::initializeCXXLayout( - const CXXRecordDecl *RD) { +void +MicrosoftRecordLayoutBuilder::initializeCXXLayout(const CXXRecordDecl *RD) { EndsWithZeroSizedObject = false; LeadsWithZeroSizedBase = false; HasOwnVFPtr = false; @@ -2806,8 +2818,8 @@ void MicrosoftRecordLayoutBuilder::initializeCXXLayout( PointerInfo.Alignment = std::min(PointerInfo.Alignment, MaxFieldAlignment); } -void MicrosoftRecordLayoutBuilder::layoutNonVirtualBases( - const CXXRecordDecl *RD) { +void +MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { // The MS-ABI lays out all bases that contain leading vfptrs before it lays // out any bases that do not contain vfptrs. We implement this as two passes // over the bases. This approach guarantees that the primary base is laid out @@ -3058,8 +3070,8 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { DataSize = Size; } -void MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField( - const FieldDecl *FD) { +void +MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) { // Zero-width bitfields are ignored unless they follow a non-zero-width // bitfield. if (!LastFieldIsNonZeroWidthBitfield) { @@ -3201,8 +3213,8 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { assert(BaseOffset >= Size && "base offset already allocated"); - VBases.insert(std::make_pair( - BaseDecl, ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp))); + VBases.insert(std::make_pair(BaseDecl, + ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp))); Size = BaseOffset + BaseLayout.getNonVirtualSize(); PreviousBaseLayout = &BaseLayout; } @@ -3306,9 +3318,10 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { // Recursively walks the non-virtual bases of a class and determines if any of // them are in the bases with overridden methods set. -static bool RequiresVtordisp(const llvm::SmallPtrSetImpl<const CXXRecordDecl *> - &BasesWithOverriddenMethods, - const CXXRecordDecl *RD) { +static bool +RequiresVtordisp(const llvm::SmallPtrSetImpl<const CXXRecordDecl *> & + BasesWithOverriddenMethods, + const CXXRecordDecl *RD) { if (BasesWithOverriddenMethods.count(RD)) return true; // If any of a virtual bases non-virtual bases (recursively) requires a @@ -3398,7 +3411,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { // until we *finish* parsing the definition. if (D->hasExternalLexicalStorage() && !D->getDefinition()) - getExternalSource()->CompleteType(const_cast<RecordDecl *>(D)); + getExternalSource()->CompleteType(const_cast<RecordDecl*>(D)); // Complete the redecl chain (if necessary). (void)D->getMostRecentDecl(); @@ -3411,8 +3424,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { // Note that we can't save a reference to the entry because this function // is recursive. const ASTRecordLayout *Entry = ASTRecordLayouts[D]; - if (Entry) - return *Entry; + if (Entry) return *Entry; const ASTRecordLayout *NewEntry = nullptr; @@ -3488,8 +3500,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { return *NewEntry; } -const CXXMethodDecl * -ASTContext::getCurrentKeyFunction(const CXXRecordDecl *RD) { +const CXXMethodDecl *ASTContext::getCurrentKeyFunction(const CXXRecordDecl *RD) { if (!getTargetInfo().getCXXABI().hasKeyFunctions()) return nullptr; @@ -3507,7 +3518,7 @@ ASTContext::getCurrentKeyFunction(const CXXRecordDecl *RD) { // Store it back if it changed. if (Entry.isOffset() || Entry.isValid() != bool(Result)) - KeyFunctions[RD] = const_cast<Decl *>(Result); + KeyFunctions[RD] = const_cast<Decl*>(Result); return cast_or_null<CXXMethodDecl>(Result); } @@ -3523,8 +3534,7 @@ void ASTContext::setNonKeyFunction(const CXXMethodDecl *Method) { auto I = Map.find(Method->getParent()); // If it's not cached, there's nothing to do. - if (I == Map.end()) - return; + if (I == Map.end()) return; // If it is cached, check whether it's the target method, and if so, // remove it from the cache. Note, the call to 'get' might invalidate @@ -3569,8 +3579,8 @@ uint64_t ASTContext::lookupFieldBitOffset(const ObjCInterfaceDecl *OID, // directly. unsigned Index = 0; - for (const ObjCIvarDecl *IVD = Container->all_declared_ivar_begin(); IVD; - IVD = IVD->getNextIvar()) { + for (const ObjCIvarDecl *IVD = Container->all_declared_ivar_begin(); + IVD; IVD = IVD->getNextIvar()) { if (Ivar == IVD) break; ++Index; @@ -3589,7 +3599,7 @@ const ASTRecordLayout & ASTContext::getObjCLayout(const ObjCInterfaceDecl *D) const { // Retrieve the definition if (D->hasExternalLexicalStorage() && !D->getDefinition()) - getExternalSource()->CompleteType(const_cast<ObjCInterfaceDecl *>(D)); + getExternalSource()->CompleteType(const_cast<ObjCInterfaceDecl*>(D)); D = D->getDefinition(); assert(D && !D->isInvalidDecl() && D->isThisDeclarationADefinition() && "Invalid interface decl!"); @@ -3612,8 +3622,8 @@ ASTContext::getObjCLayout(const ObjCInterfaceDecl *D) const { return *NewEntry; } -static void PrintOffset(raw_ostream &OS, CharUnits Offset, - unsigned IndentLevel) { +static void PrintOffset(raw_ostream &OS, + CharUnits Offset, unsigned IndentLevel) { OS << llvm::format("%10" PRId64 " | ", (int64_t)Offset.getQuantity()); OS.indent(IndentLevel * 2); } @@ -3642,9 +3652,12 @@ static void PrintIndentNoOffset(raw_ostream &OS, unsigned IndentLevel) { } static void DumpRecordLayout(raw_ostream &OS, const RecordDecl *RD, - const ASTContext &C, CharUnits Offset, - unsigned IndentLevel, const char *Description, - bool PrintSizeInfo, bool IncludeVirtualBases) { + const ASTContext &C, + CharUnits Offset, + unsigned IndentLevel, + const char* Description, + bool PrintSizeInfo, + bool IncludeVirtualBases) { const ASTRecordLayout &Layout = C.getASTRecordLayout(RD); auto CXXRD = dyn_cast<CXXRecordDecl>(RD); @@ -3710,7 +3723,7 @@ static void DumpRecordLayout(raw_ostream &OS, const RecordDecl *RD, uint64_t LocalFieldOffsetInBits = Layout.getFieldOffset(Field->getFieldIndex()); CharUnits FieldOffset = - Offset + C.toCharUnitsFromBits(LocalFieldOffsetInBits); + Offset + C.toCharUnitsFromBits(LocalFieldOffsetInBits); // Recursively dump fields of record type. if (auto RT = Field->getType()->getAs<RecordType>()) { @@ -3738,7 +3751,7 @@ static void DumpRecordLayout(raw_ostream &OS, const RecordDecl *RD, // Dump virtual bases. if (CXXRD && IncludeVirtualBases) { const ASTRecordLayout::VBaseOffsetsMapTy &VtorDisps = - Layout.getVBaseOffsetsMap(); + Layout.getVBaseOffsetsMap(); for (const CXXBaseSpecifier &Base : CXXRD->vbases()) { assert(Base.isVirtual() && "Found non-virtual class!"); @@ -3752,16 +3765,14 @@ static void DumpRecordLayout(raw_ostream &OS, const RecordDecl *RD, } DumpRecordLayout(OS, VBase, C, VBaseOffset, IndentLevel, - VBase == Layout.getPrimaryBase() - ? "(primary virtual base)" - : "(virtual base)", + VBase == Layout.getPrimaryBase() ? + "(primary virtual base)" : "(virtual base)", /*PrintSizeInfo=*/false, /*IncludeVirtualBases=*/false); } } - if (!PrintSizeInfo) - return; + if (!PrintSizeInfo) return; PrintIndentNoOffset(OS, IndentLevel - 1); OS << "[sizeof=" << Layout.getSize().getQuantity(); >From 93ee1ddbb3aafa431648b3179262afa479804dcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20De=20Magalhaes?= <theo.de-magalh...@epita.fr> Date: Thu, 13 Mar 2025 20:27:20 +0100 Subject: [PATCH 04/11] test(clang-cl): bitfield & struct padding --- clang/test/Driver/windows-Wpadded.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 clang/test/Driver/windows-Wpadded.cpp diff --git a/clang/test/Driver/windows-Wpadded.cpp b/clang/test/Driver/windows-Wpadded.cpp new file mode 100644 index 0000000000000..3babf6db39270 --- /dev/null +++ b/clang/test/Driver/windows-Wpadded.cpp @@ -0,0 +1,11 @@ +// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s + +struct Foo { + int b : 1; + char a; +}; + +int main () {Foo foo;} + +// WARN: warning: padding struct 'Foo' with 31 bits to align 'a' +// WARN: warning: padding size of 'Foo' with 3 bytes to alignment boundary >From a259c2bb1b5a48d27a91f79e720dbec9d77e7585 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20De=20Magalhaes?= <theo.de-magalh...@epita.fr> Date: Tue, 18 Mar 2025 00:24:50 +0100 Subject: [PATCH 05/11] refacto: made CheckFieldPadding a static helper --- clang/lib/AST/RecordLayoutBuilder.cpp | 86 ++++++++------------------- 1 file changed, 24 insertions(+), 62 deletions(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 68d70ebadddcb..db4401650e2ef 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2274,9 +2274,10 @@ static unsigned getPaddingDiagFromTagKind(TagTypeKind Tag) { } } -void ItaniumRecordLayoutBuilder::CheckFieldPadding( - uint64_t Offset, uint64_t UnpaddedOffset, uint64_t UnpackedOffset, - unsigned UnpackedAlign, bool isPacked, const FieldDecl *D) { +// Function based on ItaniumRecordLayoutBuilder::CheckFieldPadding. +static void CheckFieldPadding(const ASTContext &Context, bool IsUnion, + uint64_t Offset, uint64_t UnpaddedOffset, + const FieldDecl *D) { // We let objc ivars without warning, objc interfaces generally are not used // for padding tricks. if (isa<ObjCIvarDecl>(D)) @@ -2300,7 +2301,8 @@ void ItaniumRecordLayoutBuilder::CheckFieldPadding( if (D->getIdentifier()) { auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_bitfield : diag::warn_padded_struct_field; - Diag(D->getLocation(), Diagnostic) + Context.getDiagnostics().Report(D->getLocation(), + Diagnostic) << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) << Context.getTypeDeclType(D->getParent()) << PadSize << (InBits ? 1 : 0) // (byte|bit) @@ -2308,15 +2310,22 @@ void ItaniumRecordLayoutBuilder::CheckFieldPadding( } else { auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_anon_bitfield : diag::warn_padded_struct_anon_field; - Diag(D->getLocation(), Diagnostic) + Context.getDiagnostics().Report(D->getLocation(), + Diagnostic) << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) << Context.getTypeDeclType(D->getParent()) << PadSize << (InBits ? 1 : 0); // (byte|bit) } - } - if (isPacked && Offset != UnpackedOffset) { - HasPackedField = true; - } + } +} + +void ItaniumRecordLayoutBuilder::CheckFieldPadding( + uint64_t Offset, uint64_t UnpaddedOffset, uint64_t UnpackedOffset, + unsigned UnpackedAlign, bool isPacked, const FieldDecl *D) { + ::CheckFieldPadding(Context, IsUnion, Offset, UnpaddedOffset, D); + if (isPacked && Offset != UnpackedOffset) { + HasPackedField = true; + } } static const CXXMethodDecl *computeKeyFunction(ASTContext &Context, @@ -2605,8 +2614,6 @@ struct MicrosoftRecordLayoutBuilder { void computeVtorDispSet( llvm::SmallPtrSetImpl<const CXXRecordDecl *> &HasVtorDispSet, const CXXRecordDecl *RD) const; - void CheckFieldPadding(uint64_t Offset, uint64_t UnpaddedOffset, - const FieldDecl *D); const ASTContext &Context; EmptySubobjectMap *EmptySubobjects; @@ -3008,8 +3015,8 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) { uint64_t UnpaddedFielddOffsetInBits = Context.toBits(DataSize) - RemainingBitsInField; - CheckFieldPadding(Context.toBits(FieldOffset), UnpaddedFielddOffsetInBits, - FD); + ::CheckFieldPadding(Context, IsUnion, Context.toBits(FieldOffset), + UnpaddedFielddOffsetInBits, FD); RemainingBitsInField = 0; @@ -3064,8 +3071,8 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { Size = FieldOffset + Info.Size; Alignment = std::max(Alignment, Info.Alignment); RemainingBitsInField = Context.toBits(Info.Size) - Width; - CheckFieldPadding(Context.toBits(FieldOffset), UnpaddedFieldOffsetInBits, - FD); + ::CheckFieldPadding(Context, IsUnion, Context.toBits(FieldOffset), + UnpaddedFieldOffsetInBits, FD); } DataSize = Size; } @@ -3095,8 +3102,8 @@ MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) { RemainingBitsInField = 0; Size = FieldOffset; Alignment = std::max(Alignment, Info.Alignment); - CheckFieldPadding(Context.toBits(FieldOffset), UnpaddedFieldOffsetInBits, - FD); + ::CheckFieldPadding(Context, IsUnion, Context.toBits(FieldOffset), + UnpaddedFieldOffsetInBits, FD); } DataSize = Size; } @@ -3219,51 +3226,6 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { PreviousBaseLayout = &BaseLayout; } } -// Function based on ItaniumRecordLayoutBuilder::CheckFieldPadding. -void MicrosoftRecordLayoutBuilder::CheckFieldPadding(uint64_t Offset, - uint64_t UnpaddedOffset, - const FieldDecl *D) { - - // We let objc ivars without warning, objc interfaces generally are not used - // for padding tricks. - if (isa<ObjCIvarDecl>(D)) - return; - - // Don't warn about structs created without a SourceLocation. This can - // be done by clients of the AST, such as codegen. - if (D->getLocation().isInvalid()) - return; - - unsigned CharBitNum = Context.getTargetInfo().getCharWidth(); - - // Warn if padding was introduced to the struct/class. - if (!IsUnion && Offset > UnpaddedOffset) { - unsigned PadSize = Offset - UnpaddedOffset; - bool InBits = true; - if (PadSize % CharBitNum == 0) { - PadSize = PadSize / CharBitNum; - InBits = false; - } - if (D->getIdentifier()) { - auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_bitfield - : diag::warn_padded_struct_field; - Context.getDiagnostics().Report(D->getLocation(), - Diagnostic) - << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) - << Context.getTypeDeclType(D->getParent()) << PadSize - << (InBits ? 1 : 0) // (byte|bit) - << D->getIdentifier(); - } else { - auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_anon_bitfield - : diag::warn_padded_struct_anon_field; - Context.getDiagnostics().Report(D->getLocation(), - Diagnostic) - << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) - << Context.getTypeDeclType(D->getParent()) << PadSize - << (InBits ? 1 : 0); // (byte|bit) - } - } -} void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { uint64_t UnpaddedSizeInBits = Context.toBits(DataSize); >From ec85ec2348ada4b561a47e8ffc832eeae1a544e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20De=20Magalhaes?= <theo.de-magalh...@epita.fr> Date: Tue, 18 Mar 2025 00:25:48 +0100 Subject: [PATCH 06/11] test(clang-cl): anonymous bitfield, union, alignas, derived padding --- clang/test/Driver/windows-Wpadded-alignas.cpp | 12 ++++++++++++ .../windows-Wpadded-anonymous-bitfield.cpp | 12 ++++++++++++ .../Driver/windows-Wpadded-derived-struct.cpp | 13 +++++++++++++ clang/test/Driver/windows-Wpadded-union.cpp | 19 +++++++++++++++++++ clang/test/Driver/windows-Wpadded.cpp | 3 ++- 5 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 clang/test/Driver/windows-Wpadded-alignas.cpp create mode 100644 clang/test/Driver/windows-Wpadded-anonymous-bitfield.cpp create mode 100644 clang/test/Driver/windows-Wpadded-derived-struct.cpp create mode 100644 clang/test/Driver/windows-Wpadded-union.cpp diff --git a/clang/test/Driver/windows-Wpadded-alignas.cpp b/clang/test/Driver/windows-Wpadded-alignas.cpp new file mode 100644 index 0000000000000..6160cd9b4a47b --- /dev/null +++ b/clang/test/Driver/windows-Wpadded-alignas.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s +// RUN: %clang -Wpadded -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s + +struct __attribute__((ms_struct)) AlignedStruct { + char c; + alignas(8) int i; +}; + +int main() {AlignedStruct s;} + +// WARN: warning: padding struct 'AlignedStruct' with 7 bytes to align 'i' +// WARN: warning: padding size of 'AlignedStruct' with 4 bytes to alignment boundary diff --git a/clang/test/Driver/windows-Wpadded-anonymous-bitfield.cpp b/clang/test/Driver/windows-Wpadded-anonymous-bitfield.cpp new file mode 100644 index 0000000000000..a16a84c4f7487 --- /dev/null +++ b/clang/test/Driver/windows-Wpadded-anonymous-bitfield.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s + +struct __attribute__((ms_struct)) BitfieldStruct { + char c : 1; + int : 0; + char i; +}; + +int main() {BitfieldStruct s;} + +// WARN: warning: padding struct 'BitfieldStruct' with 31 bits to align anonymous bit-field +// WARN: warning: padding size of 'BitfieldStruct' with 3 bytes to alignment boundary diff --git a/clang/test/Driver/windows-Wpadded-derived-struct.cpp b/clang/test/Driver/windows-Wpadded-derived-struct.cpp new file mode 100644 index 0000000000000..ae2325258770f --- /dev/null +++ b/clang/test/Driver/windows-Wpadded-derived-struct.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s +// RUN: %clang -Wpadded -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s + +struct Base { + int b; +}; + +struct Derived : public Base { + char c; +}; + +int main() {Derived d;} +// WARN: warning: padding size of 'Derived' with 3 bytes to alignment boundary diff --git a/clang/test/Driver/windows-Wpadded-union.cpp b/clang/test/Driver/windows-Wpadded-union.cpp new file mode 100644 index 0000000000000..f5b591ea62ac8 --- /dev/null +++ b/clang/test/Driver/windows-Wpadded-union.cpp @@ -0,0 +1,19 @@ +// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s +// RUN: %clang -Wpadded -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s + +union __attribute__((ms_struct)) Union { + char c; + long long u; +}; + +struct __attribute__((ms_struct)) StructWithUnion { + char c; + int : 0; + Union t; + short i; +}; + +int main() { StructWithUnion s; } + +// WARN: warning: padding struct 'StructWithUnion' with 7 bytes to align 't' +// WARN: warning: padding size of 'StructWithUnion' with 6 bytes to alignment boundary diff --git a/clang/test/Driver/windows-Wpadded.cpp b/clang/test/Driver/windows-Wpadded.cpp index 3babf6db39270..67bd5dc076637 100644 --- a/clang/test/Driver/windows-Wpadded.cpp +++ b/clang/test/Driver/windows-Wpadded.cpp @@ -1,6 +1,7 @@ // RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s +// RUN: %clang -Wpadded -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s -struct Foo { +struct __attribute__((ms_struct)) Foo { int b : 1; char a; }; >From b969498abe3ff7449bec17118d3f56f77b532c17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20De=20Magalhaes?= <theo.de-magalh...@epita.fr> Date: Tue, 18 Mar 2025 20:09:03 +0100 Subject: [PATCH 07/11] refacto: removed inaccurate comment --- clang/lib/AST/RecordLayoutBuilder.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index db4401650e2ef..65d7307135300 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2274,7 +2274,6 @@ static unsigned getPaddingDiagFromTagKind(TagTypeKind Tag) { } } -// Function based on ItaniumRecordLayoutBuilder::CheckFieldPadding. static void CheckFieldPadding(const ASTContext &Context, bool IsUnion, uint64_t Offset, uint64_t UnpaddedOffset, const FieldDecl *D) { >From f722c6eb16f09919d1421e713c0280813d2002c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20De=20Magalhaes?= <theo.de-magalh...@epita.fr> Date: Wed, 19 Mar 2025 18:24:41 +0100 Subject: [PATCH 08/11] refacto(tests): moved folder and combined test files when possible --- clang/test/Driver/windows-Wpadded-alignas.cpp | 12 ------ .../windows-Wpadded-anonymous-bitfield.cpp | 12 ------ .../Driver/windows-Wpadded-derived-struct.cpp | 13 ------ clang/test/Driver/windows-Wpadded-union.cpp | 19 --------- clang/test/Driver/windows-Wpadded.cpp | 12 ------ .../test/SemaCXX/windows-Wpadded-bitfield.cpp | 11 +++++ clang/test/SemaCXX/windows-Wpadded.cpp | 40 +++++++++++++++++++ 7 files changed, 51 insertions(+), 68 deletions(-) delete mode 100644 clang/test/Driver/windows-Wpadded-alignas.cpp delete mode 100644 clang/test/Driver/windows-Wpadded-anonymous-bitfield.cpp delete mode 100644 clang/test/Driver/windows-Wpadded-derived-struct.cpp delete mode 100644 clang/test/Driver/windows-Wpadded-union.cpp delete mode 100644 clang/test/Driver/windows-Wpadded.cpp create mode 100644 clang/test/SemaCXX/windows-Wpadded-bitfield.cpp create mode 100644 clang/test/SemaCXX/windows-Wpadded.cpp diff --git a/clang/test/Driver/windows-Wpadded-alignas.cpp b/clang/test/Driver/windows-Wpadded-alignas.cpp deleted file mode 100644 index 6160cd9b4a47b..0000000000000 --- a/clang/test/Driver/windows-Wpadded-alignas.cpp +++ /dev/null @@ -1,12 +0,0 @@ -// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s -// RUN: %clang -Wpadded -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s - -struct __attribute__((ms_struct)) AlignedStruct { - char c; - alignas(8) int i; -}; - -int main() {AlignedStruct s;} - -// WARN: warning: padding struct 'AlignedStruct' with 7 bytes to align 'i' -// WARN: warning: padding size of 'AlignedStruct' with 4 bytes to alignment boundary diff --git a/clang/test/Driver/windows-Wpadded-anonymous-bitfield.cpp b/clang/test/Driver/windows-Wpadded-anonymous-bitfield.cpp deleted file mode 100644 index a16a84c4f7487..0000000000000 --- a/clang/test/Driver/windows-Wpadded-anonymous-bitfield.cpp +++ /dev/null @@ -1,12 +0,0 @@ -// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s - -struct __attribute__((ms_struct)) BitfieldStruct { - char c : 1; - int : 0; - char i; -}; - -int main() {BitfieldStruct s;} - -// WARN: warning: padding struct 'BitfieldStruct' with 31 bits to align anonymous bit-field -// WARN: warning: padding size of 'BitfieldStruct' with 3 bytes to alignment boundary diff --git a/clang/test/Driver/windows-Wpadded-derived-struct.cpp b/clang/test/Driver/windows-Wpadded-derived-struct.cpp deleted file mode 100644 index ae2325258770f..0000000000000 --- a/clang/test/Driver/windows-Wpadded-derived-struct.cpp +++ /dev/null @@ -1,13 +0,0 @@ -// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s -// RUN: %clang -Wpadded -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s - -struct Base { - int b; -}; - -struct Derived : public Base { - char c; -}; - -int main() {Derived d;} -// WARN: warning: padding size of 'Derived' with 3 bytes to alignment boundary diff --git a/clang/test/Driver/windows-Wpadded-union.cpp b/clang/test/Driver/windows-Wpadded-union.cpp deleted file mode 100644 index f5b591ea62ac8..0000000000000 --- a/clang/test/Driver/windows-Wpadded-union.cpp +++ /dev/null @@ -1,19 +0,0 @@ -// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s -// RUN: %clang -Wpadded -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s - -union __attribute__((ms_struct)) Union { - char c; - long long u; -}; - -struct __attribute__((ms_struct)) StructWithUnion { - char c; - int : 0; - Union t; - short i; -}; - -int main() { StructWithUnion s; } - -// WARN: warning: padding struct 'StructWithUnion' with 7 bytes to align 't' -// WARN: warning: padding size of 'StructWithUnion' with 6 bytes to alignment boundary diff --git a/clang/test/Driver/windows-Wpadded.cpp b/clang/test/Driver/windows-Wpadded.cpp deleted file mode 100644 index 67bd5dc076637..0000000000000 --- a/clang/test/Driver/windows-Wpadded.cpp +++ /dev/null @@ -1,12 +0,0 @@ -// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s -// RUN: %clang -Wpadded -fsyntax-only -- %s 2>&1 | FileCheck -check-prefix=WARN %s - -struct __attribute__((ms_struct)) Foo { - int b : 1; - char a; -}; - -int main () {Foo foo;} - -// WARN: warning: padding struct 'Foo' with 31 bits to align 'a' -// WARN: warning: padding size of 'Foo' with 3 bytes to alignment boundary diff --git a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp new file mode 100644 index 0000000000000..2907d0201d98d --- /dev/null +++ b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -triple x86_64-windows-msvc -fsyntax-only -verify -Wpadded %s + +struct __attribute__((ms_struct)) BitfieldStruct { // expected-warning {{padding size of 'BitfieldStruct' with 3 bytes to alignment boundary}} + char c : 1; + int : 0; // expected-warning {{padding struct 'BitfieldStruct' with 31 bits to align anonymous bit-field}} + char i; +}; + +int main() { + BitfieldStruct b; +} diff --git a/clang/test/SemaCXX/windows-Wpadded.cpp b/clang/test/SemaCXX/windows-Wpadded.cpp new file mode 100644 index 0000000000000..da3f2bf08c6b8 --- /dev/null +++ b/clang/test/SemaCXX/windows-Wpadded.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -triple x86_64-windows-msvc -fsyntax-only -verify -Wpadded %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -Wpadded %s + +struct __attribute__((ms_struct)) Foo { // expected-warning {{padding size of 'Foo' with 3 bytes to alignment boundary}} + int b : 1; + char a; // expected-warning {{padding struct 'Foo' with 31 bits to align 'a'}} +}; + +struct __attribute__((ms_struct)) AlignedStruct { // expected-warning {{padding size of 'AlignedStruct' with 4 bytes to alignment boundary}} + char c; + alignas(8) int i; // expected-warning {{padding struct 'AlignedStruct' with 7 bytes to align 'i'}} +}; + + +struct Base { + int b; +}; + +struct Derived : public Base { // expected-warning {{padding size of 'Derived' with 3 bytes to alignment boundary}} + char c; +}; + +union __attribute__((ms_struct)) Union { + char c; + long long u; +}; + +struct __attribute__((ms_struct)) StructWithUnion { // expected-warning {{padding size of 'StructWithUnion' with 6 bytes to alignment boundary}} + char c; + int : 0; + Union t; // expected-warning {{padding struct 'StructWithUnion' with 7 bytes to align 't'}} + short i; +}; + +int main() { + Foo f; + AlignedStruct a; + Derived d; + StructWithUnion swu; +} >From 6c79da3341ee0cc6892d575568a29450cab8847d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20De=20Magalhaes?= <theo.de-magalh...@epita.fr> Date: Thu, 27 Mar 2025 18:28:06 +0100 Subject: [PATCH 09/11] test(clang-cl): more bitfield padding tests --- .../test/SemaCXX/windows-Wpadded-bitfield.cpp | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp index 2907d0201d98d..ee5a57124eca5 100644 --- a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp +++ b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp @@ -6,6 +6,27 @@ struct __attribute__((ms_struct)) BitfieldStruct { // expected-warning {{padding char i; }; +struct __attribute__((ms_struct)) SevenBitfieldStruct { // expected-warning {{padding size of 'SevenBitfieldStruct' with 3 bytes to alignment boundary}} + char c : 7; + int : 0; // expected-warning {{padding struct 'SevenBitfieldStruct' with 25 bits to align anonymous bit-field}} + char i; +}; + +struct __attribute__((ms_struct)) SameUnitSizeBitfield { + char c : 7; + char : 1; // Same unit size attributes fall in the same unit + they fill the unit -> no padding + char i; +}; + +struct __attribute__((ms_struct)) DifferentUnitSizeBitfield { // expected-warning {{padding size of 'DifferentUnitSizeBitfield' with 3 bytes to alignment boundary}} + char c : 7; + int : 1; // expected-warning {{padding struct 'DifferentUnitSizeBitfield' with 25 bits to align anonymous bit-field}} + char i; // expected-warning {{padding struct 'DifferentUnitSizeBitfield' with 31 bits to align 'i'}} +}; + int main() { BitfieldStruct b; + SevenBitfieldStruct s; + SameUnitSizeBitfield su; + DifferentUnitSizeBitfield du; } >From ca615889b4ad7e400773f3e5a11f70befe42f7e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20De=20Magalhaes?= <theo.de-magalh...@epita.fr> Date: Fri, 28 Mar 2025 01:05:46 +0100 Subject: [PATCH 10/11] chore: clang-format --- clang/lib/AST/RecordLayoutBuilder.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 65d7307135300..41e7198cb7581 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -3071,7 +3071,7 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { Alignment = std::max(Alignment, Info.Alignment); RemainingBitsInField = Context.toBits(Info.Size) - Width; ::CheckFieldPadding(Context, IsUnion, Context.toBits(FieldOffset), - UnpaddedFieldOffsetInBits, FD); + UnpaddedFieldOffsetInBits, FD); } DataSize = Size; } @@ -3102,7 +3102,7 @@ MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) { Size = FieldOffset; Alignment = std::max(Alignment, Info.Alignment); ::CheckFieldPadding(Context, IsUnion, Context.toBits(FieldOffset), - UnpaddedFieldOffsetInBits, FD); + UnpaddedFieldOffsetInBits, FD); } DataSize = Size; } >From 721da29ecb1529d81b2c839c25151cf0c9f9ab82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20De=20Magalhaes?= <theo.de-magalh...@epita.fr> Date: Fri, 28 Mar 2025 18:47:37 +0100 Subject: [PATCH 11/11] chore: updated clang release notes --- clang/docs/ReleaseNotes.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index eaecead0e6b9d..d2f23f05e7631 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -187,6 +187,8 @@ Modified Compiler Flags - The compiler flag `-fbracket-depth` default value is increased from 256 to 2048. (#GH94728) +- `-Wpadded` option implemented for the `x86_64-windows-msvc` target. Fixes #61702 + Removed Compiler Flags ------------------------- _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
