Author: Zahira Ammarguellat Date: 2026-10-12T11:00:22Z New Revision: ea7431065550e96ddaf7d1c6fae5ad109d5b6645
URL: https://github.com/llvm/llvm-project/commit/ea7431065550e96ddaf7d1c6fae5ad109d5b6645 DIFF: https://github.com/llvm/llvm-project/commit/ea7431065550e96ddaf7d1c6fae5ad109d5b6645.diff LOG: [Clang][Windows] Preserve natural alignment for vector and x86_fp80 arrays under #pragma pack. (#208256) This is to fix a misalignment issue when using `-mlong-double-80` on Windows with types containing `long double` inside STL containers or structures with `#pragma pack(8)`. On Windows, MSVC STL headers use `#pragma pack(8)` which limits struct alignment to 8 bytes. When `-mlong-double-80` is used, `long double` becomes `x86_fp80` which requires 16-byte alignment for correctness (movaps instructions). This creates a conflict: - Array allocated with 8-byte alignment (due to pragma pack) - Element type requires 16-byte alignment - memcpy assumes 16-byte alignment - Result: Crash due to misaligned access Example: ```cpp #include <array> struct Klass { long double a; }; void test() { std::array<Klass, 16> matrix; // align 8 (wrong!) matrix.fill({}); // memcpy with align 16 (crash!) } Added: clang/test/CodeGen/pragma-pack-array-alignment-msvc.cpp Modified: clang/docs/ReleaseNotes.md clang/include/clang/AST/ASTContext.h clang/include/clang/AST/RecordLayout.h clang/lib/AST/ASTContext.cpp clang/lib/AST/RecordLayout.cpp clang/lib/AST/RecordLayoutBuilder.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 11f9e6e7a13b6..216274fc5c670 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -119,6 +119,13 @@ features cannot lower the translation-unit ABI level; for homogeneous aggregate classification. `-fclang-abi-compat=23` restores the previous behavior. (#GH218799) +- On Windows targets using the Microsoft record layout, `#pragma pack` no longer + reduces the alignment of fields of vector type (e.g. `__m128`) or of x87 + 80-bit `long double` (with `-mlong-double-80`), including when such fields + are nested in structs or arrays. This matches MSVC and fixes misaligned + accesses to these fields. `__attribute__((packed))` on the field or the + enclosing record still reduces the alignment. (#GH208256) + ### AST Dumping Potentially Breaking Changes ### Clang Frontend Potentially Breaking Changes diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index b04b633561661..1349637273d1e 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -183,6 +183,10 @@ enum class AlignRequirementKind { /// The alignment comes from an alignment attribute on a enum type. RequiredByEnum, + + /// The type's natural alignment is preserved under #pragma pack in MSVC + /// record layout (e.g., vectors, x87 fp80). + ResistPragmaPack, }; struct TypeInfo { @@ -195,7 +199,8 @@ struct TypeInfo { AlignRequirementKind AlignRequirement) : Width(Width), Align(Align), AlignRequirement(AlignRequirement) {} bool isAlignRequired() { - return AlignRequirement != AlignRequirementKind::None; + return AlignRequirement != AlignRequirementKind::None && + AlignRequirement != AlignRequirementKind::ResistPragmaPack; } }; @@ -209,7 +214,8 @@ struct TypeInfoChars { AlignRequirementKind AlignRequirement) : Width(Width), Align(Align), AlignRequirement(AlignRequirement) {} bool isAlignRequired() { - return AlignRequirement != AlignRequirementKind::None; + return AlignRequirement != AlignRequirementKind::None && + AlignRequirement != AlignRequirementKind::ResistPragmaPack; } }; diff --git a/clang/include/clang/AST/RecordLayout.h b/clang/include/clang/AST/RecordLayout.h index 9af17df229b37..5ae2f94f062cf 100644 --- a/clang/include/clang/AST/RecordLayout.h +++ b/clang/include/clang/AST/RecordLayout.h @@ -84,6 +84,10 @@ class ASTRecordLayout { /// the __declspec(align()) trumps #pramga pack and must always be obeyed. CharUnits RequiredAlignment; + /// Natural alignment of pragma-pack-resistant fields (vectors, x87 fp80), + /// or zero if none. + CharUnits PragmaPackResistantAlignment; + /// FieldOffsets - Array of field offsets in bits. ASTVector<uint64_t> FieldOffsets; @@ -157,7 +161,8 @@ class ASTRecordLayout { ASTRecordLayout(const ASTContext &Ctx, CharUnits size, CharUnits alignment, CharUnits preferredAlignment, CharUnits unadjustedAlignment, - CharUnits requiredAlignment, CharUnits datasize, + CharUnits requiredAlignment, + CharUnits pragmaPackResistantAlignment, CharUnits datasize, ArrayRef<uint64_t> fieldoffsets); using BaseOffsetsMapTy = CXXRecordLayoutInfo::BaseOffsetsMapTy; @@ -165,7 +170,8 @@ class ASTRecordLayout { // Constructor for C++ records. ASTRecordLayout(const ASTContext &Ctx, CharUnits size, CharUnits alignment, CharUnits preferredAlignment, CharUnits unadjustedAlignment, - CharUnits requiredAlignment, bool hasOwnVFPtr, + CharUnits requiredAlignment, + CharUnits pragmaPackResistantAlignment, bool hasOwnVFPtr, bool hasExtendableVFPtr, CharUnits vbptroffset, CharUnits datasize, ArrayRef<uint64_t> fieldoffsets, CharUnits nonvirtualsize, CharUnits nonvirtualalignment, @@ -327,6 +333,11 @@ class ASTRecordLayout { CharUnits getRequiredAlignment() const { return RequiredAlignment; } + /// Get the natural alignment of pragma-pack-resistant fields. + CharUnits getPragmaPackResistantAlignment() const { + return PragmaPackResistantAlignment; + } + bool endsWithZeroSizedObject() const { return CXXInfo && CXXInfo->EndsWithZeroSizedObject; } diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index d2774b03151b2..02d656feb0cee 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -2071,7 +2071,7 @@ bool ASTContext::isPromotableIntegerType(QualType T) const { } bool ASTContext::isAlignmentRequired(const Type *T) const { - return getTypeInfo(T).AlignRequirement != AlignRequirementKind::None; + return getTypeInfo(T).isAlignRequired(); } bool ASTContext::isAlignmentRequired(QualType T) const { @@ -2201,6 +2201,9 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { VT->getVectorKind() == VectorKind::RVVFixedLengthMask_4) // Adjust the alignment for fixed-length RVV vectors. Align = std::min<unsigned>(64, Width); + // Vector types have their natural alignment resist reduction by + // #pragma pack in MSVC record layout. + AlignRequirement = AlignRequirementKind::ResistPragmaPack; break; } @@ -2362,6 +2365,10 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { Width = Target->getLongDoubleWidth(); Align = Target->getLongDoubleAlign(); } + // x87 extended precision (fp80) has its natural alignment resist + // reduction by #pragma pack in MSVC record layout. + if (&Target->getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended()) + AlignRequirement = AlignRequirementKind::ResistPragmaPack; break; case BuiltinType::Float128: if (Target->hasFloat128Type() || !getLangOpts().OpenMP || diff --git a/clang/lib/AST/RecordLayout.cpp b/clang/lib/AST/RecordLayout.cpp index 314b24a1f319e..b5fefb2b97724 100644 --- a/clang/lib/AST/RecordLayout.cpp +++ b/clang/lib/AST/RecordLayout.cpp @@ -27,17 +27,16 @@ void ASTRecordLayout::Destroy(ASTContext &Ctx) { Ctx.Deallocate(this); } -ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx, CharUnits size, - CharUnits alignment, - CharUnits preferredAlignment, - CharUnits unadjustedAlignment, - CharUnits requiredAlignment, - CharUnits datasize, - ArrayRef<uint64_t> fieldoffsets) +ASTRecordLayout::ASTRecordLayout( + const ASTContext &Ctx, CharUnits size, CharUnits alignment, + CharUnits preferredAlignment, CharUnits unadjustedAlignment, + CharUnits requiredAlignment, CharUnits pragmaPackResistantAlignment, + CharUnits datasize, ArrayRef<uint64_t> fieldoffsets) : Size(size), DataSize(datasize), Alignment(alignment), PreferredAlignment(preferredAlignment), UnadjustedAlignment(unadjustedAlignment), - RequiredAlignment(requiredAlignment) { + RequiredAlignment(requiredAlignment), + PragmaPackResistantAlignment(pragmaPackResistantAlignment) { FieldOffsets.append(Ctx, fieldoffsets.begin(), fieldoffsets.end()); } @@ -45,8 +44,9 @@ ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx, CharUnits size, ASTRecordLayout::ASTRecordLayout( const ASTContext &Ctx, CharUnits size, CharUnits alignment, CharUnits preferredAlignment, CharUnits unadjustedAlignment, - CharUnits requiredAlignment, bool hasOwnVFPtr, bool hasExtendableVFPtr, - CharUnits vbptroffset, CharUnits datasize, ArrayRef<uint64_t> fieldoffsets, + CharUnits requiredAlignment, CharUnits pragmaPackResistantAlignment, + bool hasOwnVFPtr, bool hasExtendableVFPtr, CharUnits vbptroffset, + CharUnits datasize, ArrayRef<uint64_t> fieldoffsets, CharUnits nonvirtualsize, CharUnits nonvirtualalignment, CharUnits preferrednvalignment, CharUnits nonrequirednvalignment, CharUnits SizeOfLargestEmptySubobject, const CXXRecordDecl *PrimaryBase, @@ -57,6 +57,7 @@ ASTRecordLayout::ASTRecordLayout( PreferredAlignment(preferredAlignment), UnadjustedAlignment(unadjustedAlignment), RequiredAlignment(requiredAlignment), + PragmaPackResistantAlignment(pragmaPackResistantAlignment), CXXInfo(new (Ctx) CXXRecordLayoutInfo) { FieldOffsets.append(Ctx, fieldoffsets.begin(), fieldoffsets.end()); diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 45636457b4e8b..40b747da5f9bc 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2633,6 +2633,9 @@ struct MicrosoftRecordLayoutBuilder { /// The alignment of the record ignoring any over-alignment imposed via /// __declspec(align()) / alignas on the record or its bases. CharUnits NonRequiredAlignment; + /// Natural alignment of pragma-pack-resistant fields (vectors, x87 fp80), + /// or zero if none. + CharUnits PragmaPackResistantAlignment; /// The size of the allocation of the currently active bitfield. /// This value isn't meaningful unless LastFieldIsNonZeroWidthBitfield /// is true. @@ -2724,8 +2727,17 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( // The portion of the field's required alignment that comes from its type // rather than from over-alignment applied directly to the field. CharUnits FieldTypeRequiredAlignment = CharUnits::Zero(); - // Respect align attributes on the type. - if (Context.isAlignmentRequired(FD->getType())) + // Types that resist #pragma pack (vectors, x87 fp80) contribute their + // natural alignment for the purpose of resisting pragma pack, but do not + // affect the record's required alignment. + CharUnits FieldTypePragmaPackResistantAlignment = CharUnits::Zero(); + AlignRequirementKind FieldAlignReq = + Context.getTypeInfo(FD->getType()).AlignRequirement; + if (FieldAlignReq == AlignRequirementKind::ResistPragmaPack) + FieldTypePragmaPackResistantAlignment = + Context.getTypeAlignInChars(FD->getType()); + else if (FieldAlignReq != AlignRequirementKind::None) + // Respect align attributes on the type. FieldTypeRequiredAlignment = Context.getTypeAlignInChars(FD->getType()); // Respect attributes applied to subobjects of the field. if (FD->isBitField()) @@ -2742,17 +2754,30 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( EndsWithZeroSizedObject = Layout.endsWithZeroSizedObject(); FieldTypeRequiredAlignment = std::max(FieldTypeRequiredAlignment, Layout.getRequiredAlignment()); + // Propagate pragma-pack-resistant alignment from nested records. + FieldTypePragmaPackResistantAlignment = + std::max(FieldTypePragmaPackResistantAlignment, + Layout.getPragmaPackResistantAlignment()); } // Capture required alignment as a side-effect. RequiredAlignment = std::max(RequiredAlignment, std::max(DirectFieldAlignment, FieldTypeRequiredAlignment)); } + // Capture pragma-pack-resistant alignment as a side-effect. + PragmaPackResistantAlignment = std::max( + PragmaPackResistantAlignment, FieldTypePragmaPackResistantAlignment); // Respect pragma pack, attribute pack and declspec align if (!MaxFieldAlignment.isZero()) Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment); if (FD->hasAttr<PackedAttr>()) Info.Alignment = CharUnits::One(); + // Types that resist #pragma pack restore their natural alignment after + // pragma pack clamping, but only when neither the field nor the enclosing + // record is explicitly packed via __attribute__((packed)). + if (!FD->hasAttr<PackedAttr>() && !FD->getParent()->hasAttr<PackedAttr>()) + Info.Alignment = + std::max(Info.Alignment, FieldTypePragmaPackResistantAlignment); // The alignment used to update the record's alignment excludes over-alignment // applied directly to the field; the alignment used for placement includes // it. On targets that don't reuse over-aligned tail padding the two are the @@ -3454,7 +3479,8 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { Builder.cxxLayout(RD); NewEntry = new (*this) ASTRecordLayout( *this, Builder.Size, Builder.Alignment, Builder.Alignment, - Builder.Alignment, Builder.RequiredAlignment, Builder.HasOwnVFPtr, + Builder.Alignment, Builder.RequiredAlignment, + Builder.PragmaPackResistantAlignment, Builder.HasOwnVFPtr, Builder.HasOwnVFPtr || Builder.PrimaryBase, Builder.VBPtrOffset, Builder.DataSize, Builder.FieldOffsets, Builder.NonVirtualSize, Builder.Alignment, Builder.Alignment, Builder.NonRequiredAlignment, @@ -3466,7 +3492,8 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { Builder.layout(D); NewEntry = new (*this) ASTRecordLayout( *this, Builder.Size, Builder.Alignment, Builder.Alignment, - Builder.Alignment, Builder.RequiredAlignment, Builder.Size, + Builder.Alignment, Builder.RequiredAlignment, + Builder.PragmaPackResistantAlignment, Builder.Size, Builder.FieldOffsets); } } else { @@ -3490,7 +3517,8 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { *this, Builder.getSize(), Builder.Alignment, Builder.PreferredAlignment, Builder.UnadjustedAlignment, /*RequiredAlignment : used by MS-ABI)*/ - Builder.Alignment, Builder.HasOwnVFPtr, RD->isDynamicClass(), + Builder.Alignment, /*PragmaPackResistantAlignment=*/CharUnits::Zero(), + Builder.HasOwnVFPtr, RD->isDynamicClass(), CharUnits::fromQuantity(-1), DataSize, Builder.FieldOffsets, NonVirtualSize, Builder.NonVirtualAlignment, Builder.PreferredNVAlignment, Builder.NonVirtualAlignment, @@ -3505,7 +3533,8 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { *this, Builder.getSize(), Builder.Alignment, Builder.PreferredAlignment, Builder.UnadjustedAlignment, /*RequiredAlignment : used by MS-ABI)*/ - Builder.Alignment, Builder.getSize(), Builder.FieldOffsets); + Builder.Alignment, /*PragmaPackResistantAlignment=*/CharUnits::Zero(), + Builder.getSize(), Builder.FieldOffsets); } } @@ -3644,7 +3673,8 @@ ASTContext::getObjCLayout(const ObjCInterfaceDecl *D) const { *this, Builder.getSize(), Builder.Alignment, Builder.PreferredAlignment, Builder.UnadjustedAlignment, /*RequiredAlignment : used by MS-ABI)*/ - Builder.Alignment, Builder.getDataSize(), Builder.FieldOffsets); + Builder.Alignment, /*PragmaPackResistantAlignment=*/CharUnits::Zero(), + Builder.getDataSize(), Builder.FieldOffsets); ObjCLayouts[D] = NewEntry; diff --git a/clang/test/CodeGen/pragma-pack-array-alignment-msvc.cpp b/clang/test/CodeGen/pragma-pack-array-alignment-msvc.cpp new file mode 100644 index 0000000000000..f22f76a315e00 --- /dev/null +++ b/clang/test/CodeGen/pragma-pack-array-alignment-msvc.cpp @@ -0,0 +1,168 @@ +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fms-extensions \ +// RUN: -ffreestanding -emit-llvm -o - %s | FileCheck %s + +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fms-extensions \ +// RUN: -ffreestanding -mlong-double-80 -emit-llvm -o - %s | FileCheck %s \ +// RUN: --check-prefix=CHECK-FP80 + +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fms-extensions \ +// RUN: -ffreestanding -mlong-double-80 -fdump-record-layouts -fsyntax-only %s \ +// RUN: | FileCheck %s --check-prefix=CHECK-LAYOUT + +// Test that #pragma pack does not reduce natural type alignment for vector +// and x86_fp80 types when used as array elements (matching MSVC behavior). + +#include <xmmintrin.h> + +// Simple array-like struct with vector type under pragma pack. +#pragma pack(push, 8) +template<typename T, unsigned N> +struct array { + T _Elems[N]; +}; +#pragma pack(pop) + +// CHECK-LABEL: define {{.*}} @"?test_vector_array +void test_vector_array() { + // CHECK: %matrix = alloca %struct.array, align 16 + array<__m128, 16> matrix; + matrix._Elems[0] = _mm_setzero_ps(); +} + +// Struct containing vector under pragma pack. +#pragma pack(push, 8) +struct VectorStruct { + __m128 vec; +}; + +struct ArrayOfVectorStruct { + VectorStruct elems[4]; +}; +#pragma pack(pop) + +// CHECK-LABEL: define {{.*}} @"?test_vector_struct +void test_vector_struct() { + // CHECK: %s = alloca %struct.ArrayOfVectorStruct, align 16 + ArrayOfVectorStruct s; + s.elems[0].vec = _mm_setzero_ps(); +} + +// Test x86_fp80 (long double with -mlong-double-80) arrays under pragma pack. +struct Klass { long double a; }; + +#pragma pack(push, 8) +template<typename T, unsigned N> +struct fp80_array { + T _Elems[N]; + + void fill(const T& val) { + for (unsigned i = 0; i < N; i++) + _Elems[i] = val; + } +}; +#pragma pack(pop) + +// CHECK-FP80-LABEL: define {{.*}} @"?test_fp80_array +void test_fp80_array() { + // CHECK-FP80: %matrix = alloca %struct.fp80_array, align 16 + fp80_array<Klass, 16> matrix; + matrix.fill({}); +} + +// Struct containing x86_fp80 under pragma pack. +#pragma pack(push, 8) +struct Fp80Struct { + long double val; +}; + +struct ArrayOfFp80Struct { + Fp80Struct elems[4]; +}; +#pragma pack(pop) + +// CHECK-FP80-LABEL: define {{.*}} @"?test_fp80_struct +void test_fp80_struct() { + // CHECK-FP80: %s = alloca %struct.ArrayOfFp80Struct, align 16 + ArrayOfFp80Struct s; + s.elems[0].val = 1.0L; +} + +// An alignment attribute on a typedef of x86_fp80 makes the alignment +// requirement come from the typedef rather than from the pragma-pack-resistant +// natural alignment of the underlying type. + +// The typedef raises the alignment above the natural alignment. +struct TypedefOverAligned { + typedef long double foo __attribute__((aligned(32))); + foo bar; +}; + +// CHECK-LAYOUT-LABEL: 0 | struct TypedefOverAligned{{$}} +// CHECK-LAYOUT-NEXT: 0 | foo bar +// CHECK-LAYOUT-NEXT: | [sizeof=32, align=32, +// CHECK-LAYOUT-NEXT: | nvsize=32, nvalign=32] + +// The typedef's alignment attribute is not reduced by #pragma pack. +#pragma pack(push, 8) +struct TypedefOverAlignedPacked { + char c; + typedef long double foo __attribute__((aligned(32))); + foo bar; +}; +#pragma pack(pop) + +// CHECK-LAYOUT-LABEL: 0 | struct TypedefOverAlignedPacked{{$}} +// CHECK-LAYOUT-NEXT: 0 | char c +// CHECK-LAYOUT-NEXT: 32 | foo bar +// CHECK-LAYOUT-NEXT: | [sizeof=64, align=32, +// CHECK-LAYOUT-NEXT: | nvsize=48, nvalign=32] + +// The typedef lowers the alignment below the natural alignment, so the field +// no longer resists #pragma pack. +#pragma pack(push, 8) +typedef long double fp80_align4 __attribute__((aligned(4))); +struct TypedefUnderAlignedPacked { + char c; + fp80_align4 x; +}; +#pragma pack(pop) + +// CHECK-LAYOUT-LABEL: 0 | struct TypedefUnderAlignedPacked{{$}} +// CHECK-LAYOUT-NEXT: 0 | char c +// CHECK-LAYOUT-NEXT: 8 | fp80_align4 x +// CHECK-LAYOUT-NEXT: | [sizeof=24, align=8, +// CHECK-LAYOUT-NEXT: | nvsize=24, nvalign=8] + +// A plain x86_fp80 field resists #pragma pack. +#pragma pack(push, 8) +struct PlainFp80Packed { + char c; + long double x; +}; +#pragma pack(pop) + +// CHECK-LAYOUT-LABEL: 0 | struct PlainFp80Packed{{$}} +// CHECK-LAYOUT-NEXT: 0 | char c +// CHECK-LAYOUT-NEXT: 16 | long double x +// CHECK-LAYOUT-NEXT: | [sizeof=32, align=16, +// CHECK-LAYOUT-NEXT: | nvsize=32, nvalign=16] + +// A typedef without an alignment attribute keeps the pragma-pack resistance. +#pragma pack(push, 8) +typedef long double fp80_t; +struct TypedefNoAttrPacked { + char c; + fp80_t x; +}; +#pragma pack(pop) + +// CHECK-LAYOUT-LABEL: 0 | struct TypedefNoAttrPacked{{$}} +// CHECK-LAYOUT-NEXT: 0 | char c +// CHECK-LAYOUT-NEXT: 16 | fp80_t x +// CHECK-LAYOUT-NEXT: | [sizeof=32, align=16, +// CHECK-LAYOUT-NEXT: | nvsize=32, nvalign=16] + +int typedef_sizes[] = { + sizeof(TypedefOverAligned), sizeof(TypedefOverAlignedPacked), + sizeof(TypedefUnderAlignedPacked), sizeof(PlainFp80Packed), + sizeof(TypedefNoAttrPacked)}; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
