r331136 - PR37275 packed attribute should not apply to base classes
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Sat Apr 28 21:55:46 PDT 2018
Author: rsmith
Date: Sat Apr 28 21:55:46 2018
New Revision: 331136
URL: http://llvm.org/viewvc/llvm-project?rev=331136&view=rev
Log:
PR37275 packed attribute should not apply to base classes
Clang incorrectly applied the packed attribute to base classes. Per GCC's
documentation and as can be observed from its behavior, packed only applies to
members, not base classes.
This change is conditioned behind -fclang-abi-compat so that an ABI break can
be avoided by users if desired.
Differential Revision: https://reviews.llvm.org/D46218
Modified:
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/test/CodeGenCXX/alignment.cpp
cfe/trunk/test/SemaCXX/class-layout.cpp
Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=331136&r1=331135&r2=331136&view=diff
==============================================================================
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Sat Apr 28 21:55:46 2018
@@ -78,6 +78,12 @@ Non-comprehensive list of changes in thi
standard-layout if all base classes and the first data member (or bit-field)
can be laid out at offset zero.
+- Clang's handling of the GCC ``packed`` class attribute in C++ has been fixed
+ to apply only to non-static data members and not to base classes. This fixes
+ an ABI difference between Clang and GCC, but creates an ABI difference between
+ Clang 7 and earlier versions. The old behavior can be restored by setting
+ ``-fclang-abi-compat`` to ``6`` or earlier.
+
- ...
New Compiler Flags
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=331136&r1=331135&r2=331136&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Sat Apr 28 21:55:46 2018
@@ -967,7 +967,7 @@ void ItaniumRecordLayoutBuilder::Compute
void ItaniumRecordLayoutBuilder::EnsureVTablePointerAlignment(
CharUnits UnpackedBaseAlign) {
- CharUnits BaseAlign = (Packed) ? CharUnits::One() : UnpackedBaseAlign;
+ CharUnits BaseAlign = Packed ? CharUnits::One() : UnpackedBaseAlign;
// The maximum field alignment overrides base align.
if (!MaxFieldAlignment.isZero()) {
@@ -1175,9 +1175,14 @@ ItaniumRecordLayoutBuilder::LayoutBase(c
HasExternalLayout = External.getExternalVBaseOffset(Base->Class, Offset);
}
+ // Clang <= 6 incorrectly applied the 'packed' attribute to base classes.
+ // Per GCC's documentation, it only applies to non-static data members.
CharUnits UnpackedBaseAlign = Layout.getNonVirtualAlignment();
- CharUnits BaseAlign = (Packed) ? CharUnits::One() : UnpackedBaseAlign;
-
+ CharUnits BaseAlign = (Packed && Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver6)
+ ? CharUnits::One()
+ : UnpackedBaseAlign;
+
// If we have an empty base class, try to place it at offset 0.
if (Base->Class->isEmpty() &&
(!HasExternalLayout || Offset == CharUnits::Zero()) &&
Modified: cfe/trunk/test/CodeGenCXX/alignment.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/alignment.cpp?rev=331136&r1=331135&r2=331136&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/alignment.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/alignment.cpp Sat Apr 28 21:55:46 2018
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-apple-darwin10 | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-apple-darwin10 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NOCOMPAT
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-apple-darwin10 -fclang-abi-compat=6.0 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-V6COMPAT
extern int int_source();
extern void int_sink(int x);
@@ -54,11 +55,13 @@ namespace test0 {
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
// CHECK: [[TRUNC:%.*]] = trunc i32 [[CALL]] to i8
- // CHECK: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-V6COMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-NOCOMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = and i8 [[TRUNC]], 3
// CHECK: [[T1:%.*]] = and i8 [[OLD_VALUE]], -4
// CHECK: [[T2:%.*]] = or i8 [[T1]], [[T0]]
- // CHECK: store i8 [[T2]], i8* [[FIELD_P]], align 2
+ // CHECK-V6COMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 2
+ // CHECK-NOCOMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 4
c.onebit = int_source();
// CHECK: [[C_P:%.*]] = load [[C]]*, [[C]]**
@@ -66,7 +69,8 @@ namespace test0 {
// CHECK: [[T1:%.*]] = getelementptr inbounds i8, i8* [[T0]], i64 8
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
- // CHECK: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-V6COMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-NOCOMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = shl i8 [[VALUE]], 6
// CHECK: [[T1:%.*]] = ashr i8 [[T0]], 6
// CHECK: [[T2:%.*]] = sext i8 [[T1]] to i32
@@ -83,11 +87,13 @@ namespace test0 {
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
// CHECK: [[TRUNC:%.*]] = trunc i32 [[CALL]] to i8
- // CHECK: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-V6COMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-NOCOMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = and i8 [[TRUNC]], 3
// CHECK: [[T1:%.*]] = and i8 [[OLD_VALUE]], -4
// CHECK: [[T2:%.*]] = or i8 [[T1]], [[T0]]
- // CHECK: store i8 [[T2]], i8* [[FIELD_P]], align 2
+ // CHECK-V6COMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 2
+ // CHECK-NOCOMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 4
c->onebit = int_source();
// CHECK: [[C_P:%.*]] = load [[C:%.*]]*, [[C]]**
@@ -95,7 +101,8 @@ namespace test0 {
// CHECK: [[T1:%.*]] = getelementptr inbounds i8, i8* [[T0]], i64 8
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B:%.*]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
- // CHECK: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-V6COMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-NOCOMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = shl i8 [[VALUE]], 6
// CHECK: [[T1:%.*]] = ashr i8 [[T0]], 6
// CHECK: [[T2:%.*]] = sext i8 [[T1]] to i32
@@ -107,7 +114,8 @@ namespace test0 {
// in an alignment-2 variable.
// CHECK-LABEL: @_ZN5test01dEv
void d() {
- // CHECK: [[C_P:%.*]] = alloca [[C:%.*]], align 2
+ // CHECK-V6COMPAT: [[C_P:%.*]] = alloca [[C:%.*]], align 2
+ // CHECK-NOCOMPAT: [[C_P:%.*]] = alloca [[C:%.*]], align 4
C c;
// CHECK: [[CALL:%.*]] = call i32 @_Z10int_sourcev()
@@ -116,18 +124,21 @@ namespace test0 {
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
// CHECK: [[TRUNC:%.*]] = trunc i32 [[CALL]] to i8
- // CHECK: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-V6COMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-NOCOMPAT: [[OLD_VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = and i8 [[TRUNC]], 3
// CHECK: [[T1:%.*]] = and i8 [[OLD_VALUE]], -4
// CHECK: [[T2:%.*]] = or i8 [[T1]], [[T0]]
- // CHECK: store i8 [[T2]], i8* [[FIELD_P]], align 2
+ // CHECK-V6COMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 2
+ // CHECK-NOCOMPAT: store i8 [[T2]], i8* [[FIELD_P]], align 4
c.onebit = int_source();
// CHECK: [[T0:%.*]] = bitcast [[C]]* [[C_P]] to i8*
// CHECK: [[T1:%.*]] = getelementptr inbounds i8, i8* [[T0]], i64 8
// CHECK: [[B_P:%.*]] = bitcast i8* [[T1]] to [[B:%.*]]*
// CHECK: [[FIELD_P:%.*]] = bitcast [[B]]* [[B_P]] to i8*
- // CHECK: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-V6COMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 2
+ // CHECK-NOCOMPAT: [[VALUE:%.*]] = load i8, i8* [[FIELD_P]], align 4
// CHECK: [[T0:%.*]] = shl i8 [[VALUE]], 6
// CHECK: [[T1:%.*]] = ashr i8 [[T0]], 6
// CHECK: [[T2:%.*]] = sext i8 [[T1]] to i32
Modified: cfe/trunk/test/SemaCXX/class-layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/class-layout.cpp?rev=331136&r1=331135&r2=331136&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/class-layout.cpp (original)
+++ cfe/trunk/test/SemaCXX/class-layout.cpp Sat Apr 28 21:55:46 2018
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=6 -DCLANG_ABI_COMPAT=6
// expected-no-diagnostics
#define SA(n, p) int a##n[(p) ? 1 : -1]
@@ -570,3 +571,36 @@ namespace test18 {
SA(0, sizeof(might_use_tail_padding) == 80);
}
} // namespace PR16537
+
+namespace PR37275 {
+ struct X { char c; };
+
+ struct A { int n; };
+ _Static_assert(_Alignof(A) == _Alignof(int), "");
+
+ // __attribute__((packed)) does not apply to base classes.
+ struct __attribute__((packed)) B : X, A {};
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 6
+ _Static_assert(_Alignof(B) == 1, "");
+ _Static_assert(__builtin_offsetof(B, n) == 1, "");
+#else
+ _Static_assert(_Alignof(B) == _Alignof(int), "");
+ _Static_assert(__builtin_offsetof(B, n) == 4, "");
+#endif
+
+ // #pragma pack does, though.
+#pragma pack(push, 2)
+ struct C : X, A {};
+ _Static_assert(_Alignof(C) == 2, "");
+ _Static_assert(__builtin_offsetof(C, n) == 2, "");
+
+ struct __attribute__((packed)) D : X, A {};
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 6
+ _Static_assert(_Alignof(D) == 1, "");
+ _Static_assert(__builtin_offsetof(D, n) == 1, "");
+#else
+ _Static_assert(_Alignof(D) == 2, "");
+ _Static_assert(__builtin_offsetof(D, n) == 2, "");
+#endif
+#pragma pack(pop)
+}
More information about the cfe-commits
mailing list