llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Zahira Ammarguellat (zahiraam)
<details>
<summary>Changes</summary>
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!)
}
---
Full diff: https://github.com/llvm/llvm-project/pull/208256.diff
4 Files Affected:
- (modified) clang/include/clang/AST/ASTContext.h (+8)
- (modified) clang/lib/AST/ASTContext.cpp (+43-3)
- (modified) clang/lib/AST/RecordLayoutBuilder.cpp (+60-5)
- (added) clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp (+130)
``````````diff
diff --git a/clang/include/clang/AST/ASTContext.h
b/clang/include/clang/AST/ASTContext.h
index 763039e690dec..4f1fd163f6f38 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -183,6 +183,9 @@ enum class AlignRequirementKind {
/// The alignment comes from an alignment attribute on a enum type.
RequiredByEnum,
+
+ /// The alignment is required by the ABI for correctness.
+ RequiredByABI,
};
struct TypeInfo {
@@ -2758,6 +2761,11 @@ class ASTContext : public RefCountedBase<ASTContext> {
TypeInfo getTypeInfo(const Type *T) const;
TypeInfo getTypeInfo(QualType T) const { return getTypeInfo(T.getTypePtr());
}
+ /// Check if a type requires natural alignment preservation under #pragma
pack
+ /// (but not explicit __attribute__((packed))). This includes x86_fp80 on
+ /// Windows MSVC and standard SIMD vectors (__m128, __m256).
+ bool typeRequiresPreserveAlignUnderPragmaPack(QualType T) const;
+
/// Get default simd alignment of the specified complete type in bits.
unsigned getOpenMPDefaultSimdAlign(QualType T) const;
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 02a3f88431f58..b536fcec9110c 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -2100,6 +2100,32 @@ TypeInfo ASTContext::getTypeInfo(const Type *T) const {
return TI;
}
+bool ASTContext::typeRequiresPreserveAlignUnderPragmaPack(QualType T) const {
+ T = T.getCanonicalType();
+ const llvm::Triple &Triple = Target->getTriple();
+ if (Triple.isOSWindows() && Triple.isWindowsMSVCEnvironment()) {
+ if (const auto *BT = T->getAs<BuiltinType>()) {
+ if (BT->getKind() == BuiltinType::LongDouble &&
+ &Target->getLongDoubleFormat() ==
&llvm::APFloat::x87DoubleExtended())
+ return true;
+ }
+ if (const auto *VT = T->getAs<VectorType>()) {
+ uint64_t VecWidth = getTypeSize(VT);
+ return VT->getVectorKind() == VectorKind::Generic &&
+ (VecWidth == 128 || VecWidth == 256) &&
+ VecWidth == getTypeAlign(VT);
+ }
+ }
+ if (const auto *AT = T->getAsArrayTypeUnsafe())
+ return typeRequiresPreserveAlignUnderPragmaPack(AT->getElementType());
+ if (const auto *RT = T->getAs<RecordType>()) {
+ for (const auto *Field : RT->getDecl()->fields())
+ if (typeRequiresPreserveAlignUnderPragmaPack(Field->getType()))
+ return true;
+ }
+ return false;
+}
+
/// getTypeInfoImpl - Return the size of the specified type, in bits. This
/// method does not work on incomplete types.
///
@@ -2539,9 +2565,23 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T)
const {
const ASTRecordLayout &Layout = getASTRecordLayout(RD);
Width = toBits(Layout.getSize());
Align = toBits(Layout.getAlignment());
- AlignRequirement = RD->hasAttr<AlignedAttr>()
- ? AlignRequirementKind::RequiredByRecord
- : AlignRequirementKind::None;
+ // Check if the record has an aligned attribute, or if it contains
+ // fields with ABI-required alignment (e.g., vectors on MSVC).
+ if (RD->hasAttr<AlignedAttr>()) {
+ AlignRequirement = AlignRequirementKind::RequiredByRecord;
+ } else {
+ // Check if any field has RequiredByABI alignment requirement.
+ // If so, propagate it to the record.
+ AlignRequirement = AlignRequirementKind::None;
+ for (const auto *Field : RD->fields()) {
+ TypeInfo FI = getTypeInfo(Field->getType().getTypePtr());
+ if (FI.AlignRequirement == AlignRequirementKind::RequiredByABI ||
+ FI.AlignRequirement == AlignRequirementKind::RequiredByRecord) {
+ AlignRequirement = AlignRequirementKind::RequiredByABI;
+ break;
+ }
+ }
+ }
break;
}
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index c27572bc7f50d..23754c87faa3d 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -559,6 +559,41 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects(
typedef llvm::SmallPtrSet<const CXXRecordDecl*, 4> ClassSetTy;
+/// Check if we should prevent MaxFieldAlignment from reducing this field's
+/// alignment. Returns true if the field has ABI-required alignment
+/// (e.g., x86_fp80, SIMD vectors on Windows), unless the struct has explicit
+/// __attribute__((packed)).
+static bool ShouldPreserveFieldAlignment(const ASTContext &Context,
+ const FieldDecl *FD,
+ AlignRequirementKind AlignReq,
+ bool StructHasPackedAttr) {
+ if (AlignReq == AlignRequirementKind::RequiredByABI ||
+ AlignReq == AlignRequirementKind::RequiredByRecord)
+ return true;
+
+ // Check if type requires alignment preservation under #pragma pack
+ // (but respect explicit __attribute__((packed))).
+ if (!StructHasPackedAttr &&
+ Context.typeRequiresPreserveAlignUnderPragmaPack(FD->getType()))
+ return true;
+ return false;
+}
+
+/// Check if a record contains any fields requiring alignment preservation.
+/// Returns false if the record has explicit __attribute__((packed)).
+static bool RecordContainsAlignPreservingFields(const ASTContext &Context,
+ const RecordDecl *RD) {
+ if (RD->hasAttr<PackedAttr>())
+ return false;
+ for (const auto *Field : RD->fields()) {
+ TypeInfo TI = Context.getTypeInfo(Field->getType());
+ if (TI.isAlignRequired() ||
+ Context.typeRequiresPreserveAlignUnderPragmaPack(Field->getType()))
+ return true;
+ }
+ return false;
+}
+
class ItaniumRecordLayoutBuilder {
protected:
// FIXME: Remove this and make the appropriate fields public.
@@ -2029,13 +2064,20 @@ void ItaniumRecordLayoutBuilder::LayoutField(const
FieldDecl *D,
UnpackedFieldAlign = std::max(UnpackedFieldAlign, MaxAlignmentInChars);
// The maximum field alignment overrides the aligned attribute.
- if (!MaxFieldAlignment.isZero()) {
+ // However, do not reduce alignment for ABI-required alignments (e.g.,
+ // x86_fp80, vector types) which must be preserved for correctness.
+ // On Windows, check if the field type is a vector with standard SIMD
+ // alignment (16 or 32 bytes with size == alignment) - these need their
+ // alignment preserved under #pragma pack. However, honor explicit
+ // __attribute__((packed)) on the struct (Packed=true means the struct
+ // has the packed attribute, not the field).
+ if (!MaxFieldAlignment.isZero() &&
+ !ShouldPreserveFieldAlignment(Context, D, AlignRequirement, Packed)) {
PackedFieldAlign = std::min(PackedFieldAlign, MaxFieldAlignment);
PreferredAlign = std::min(PreferredAlign, MaxFieldAlignment);
UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignment);
}
-
if (!FieldPacked)
FieldAlign = UnpackedFieldAlign;
if (DefaultsToAIXPowerAlignment)
@@ -2748,8 +2790,15 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo(
std::max(RequiredAlignment,
std::max(DirectFieldAlignment, FieldTypeRequiredAlignment));
}
- // Respect pragma pack, attribute pack and declspec align
- if (!MaxFieldAlignment.isZero())
+
+ // Respect pragma pack, attribute pack and declspec align.
+ // However, do not reduce alignment for ABI-required alignments (e.g.,
+ // x86_fp80, vector types) which must be preserved for correctness.
+ bool StructHasPackedAttr =
+ FD->getParent() && FD->getParent()->hasAttr<PackedAttr>();
+ if (!MaxFieldAlignment.isZero() &&
+ !ShouldPreserveFieldAlignment(Context, FD, TInfo.AlignRequirement,
+ StructHasPackedAttr))
Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment);
if (FD->hasAttr<PackedAttr>())
Info.Alignment = CharUnits::One();
@@ -3287,7 +3336,13 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const
RecordDecl *RD) {
if (!RequiredAlignment.isZero()) {
Alignment = std::max(Alignment, RequiredAlignment);
auto RoundingAlignment = Alignment;
- if (!MaxFieldAlignment.isZero())
+
+ // Check if this struct contains fields requiring alignment preservation
+ // (x86_fp80, vectors) before allowing MaxFieldAlignment (from #pragma
pack)
+ // to reduce the overall struct alignment. However, if the struct has
+ // __attribute__((packed)), honor that explicit request.
+ if (!MaxFieldAlignment.isZero() &&
+ !RecordContainsAlignPreservingFields(Context, RD))
RoundingAlignment = std::min(RoundingAlignment, MaxFieldAlignment);
RoundingAlignment = std::max(RoundingAlignment, RequiredAlignment);
Size = Size.alignTo(RoundingAlignment);
diff --git a/clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp
b/clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp
new file mode 100644
index 0000000000000..d185236f2a500
--- /dev/null
+++ b/clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp
@@ -0,0 +1,130 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+
+// Test that vector types maintain their required alignment for correctness
+// (movaps/movapd instructions will fault on misaligned addresses), even when
+// #pragma pack(8) would normally reduce it. This issue affects Windows MSVC
+// targets where #pragma pack is commonly used (e.g., MSVC STL).
+
+typedef float v4f32 __attribute__((vector_size(16)));
+typedef float v8f32 __attribute__((vector_size(32)));
+
+struct VectorKlass {
+ v4f32 v;
+};
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_vector_klass
+// CHECK: %v = alloca %struct.VectorKlass, align 16
+void test_vector_klass() {
+ VectorKlass v;
+ v.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f};
+}
+
+#pragma pack(push, 8)
+struct PackedVectorKlass {
+ v4f32 v;
+};
+
+struct PackedLargeVectorKlass {
+ v8f32 v;
+};
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_packed_vector
+// CHECK: %pv = alloca %struct.PackedVectorKlass, align 16
+void test_packed_vector() {
+ PackedVectorKlass pv;
+ pv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f};
+}
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_packed_large_vector
+// CHECK: %plv = alloca %struct.PackedLargeVectorKlass, align 32
+void test_packed_large_vector() {
+ PackedLargeVectorKlass plv;
+ plv.v = (v8f32){0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f};
+}
+
+struct InnerWithVector {
+ v4f32 v;
+};
+
+struct OuterWithVector {
+ InnerWithVector inner;
+};
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_nested_vector
+// CHECK: %outer = alloca %struct.OuterWithVector, align 16
+void test_nested_vector() {
+ OuterWithVector outer;
+ outer.inner.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f};
+}
+
+// Test array of structs containing vectors
+template<typename T, unsigned N>
+struct array {
+ T _Elems[N];
+};
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_vector_array_packed
+// CHECK: %varr = alloca %struct.array, align 16
+void test_vector_array_packed() {
+ array<PackedVectorKlass, 4> varr;
+ for (int i = 0; i < 4; i++)
+ varr._Elems[i].v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f};
+}
+#pragma pack(pop)
+
+struct __attribute__((packed)) ExplicitlyPackedVector {
+ v4f32 v;
+};
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_explicitly_packed_vector
+// CHECK: %epv = alloca %struct.ExplicitlyPackedVector, align 1
+void test_explicitly_packed_vector() {
+ ExplicitlyPackedVector epv;
+ epv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f};
+}
+
+#pragma pack(push, 8)
+struct FieldPackedVector {
+ v4f32 v __attribute__((packed));
+};
+#pragma pack(pop)
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_field_packed_vector
+// CHECK: %fpv = alloca %struct.FieldPackedVector, align 1
+void test_field_packed_vector() {
+ FieldPackedVector fpv;
+ fpv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f};
+}
+
+#pragma pack(push, 8)
+struct alignas(16) ExplicitAlignedInner {
+ long double x;
+};
+
+struct OuterWithExplicitAligned {
+ ExplicitAlignedInner inner;
+};
+
+struct ImplicitAlignedInner {
+ long double x;
+};
+
+struct OuterWithImplicitAligned {
+ ImplicitAlignedInner inner;
+};
+#pragma pack(pop)
+
+// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_explicit_aligned_nested
+// CHECK-FP80: %outer = alloca %struct.OuterWithExplicitAligned, align 16
+void test_explicit_aligned_nested() {
+ OuterWithExplicitAligned outer;
+ outer.inner.x = 0.0L;
+}
+
+// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_implicit_aligned_nested
+// CHECK-FP80: %outer = alloca %struct.OuterWithImplicitAligned, align 16
+void test_implicit_aligned_nested() {
+ OuterWithImplicitAligned outer;
+ outer.inner.x = 0.0L;
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/208256
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
