From ecde7ac8eada08e0150aec708de636b27f792a8a Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Fri, 31 Jan 2014 22:28:50 +0000 Subject: [PATCH] [ms-cxxabi] Raise aggregate memptr alignment to 8 for x86_32 With this change, we give different results for __alignof than MSVC, but our record layout is compatible. Some data member pointers also now have a size that is not a multiple of their alignment. Fixes PR18618. Reviewers: majnemer Differential Revision: http://llvm-reviews.chandlerc.com/D2669 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@200585 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/AST/MicrosoftCXXABI.cpp | 16 +++- .../microsoft-abi-member-pointers.cpp | 58 +++++++------- test/Layout/ms-x86-member-pointers.cpp | 80 +++++++++++++++++++ test/SemaCXX/member-pointer-ms.cpp | 27 ++++++- 4 files changed, 147 insertions(+), 34 deletions(-) create mode 100644 test/Layout/ms-x86-member-pointers.cpp diff --git a/lib/AST/MicrosoftCXXABI.cpp b/lib/AST/MicrosoftCXXABI.cpp index ee96e13e4b..27b3e341c4 100644 --- a/lib/AST/MicrosoftCXXABI.cpp +++ b/lib/AST/MicrosoftCXXABI.cpp @@ -197,8 +197,20 @@ std::pair MicrosoftCXXABI::getMemberPointerWidthAndAlign( unsigned PtrSize = Target.getPointerWidth(0); unsigned IntSize = Target.getIntWidth(); uint64_t Width = Ptrs * PtrSize + Ints * IntSize; - unsigned Align = Ptrs > 0 ? Target.getPointerAlign(0) : Target.getIntAlign(); - Width = llvm::RoundUpToAlignment(Width, Align); + unsigned Align; + + // When MSVC does x86_32 record layout, it aligns aggregate member pointers to + // 8 bytes. However, __alignof usually returns 4 for data memptrs and 8 for + // function memptrs. + if (Ptrs + Ints > 1 && Target.getTriple().getArch() == llvm::Triple::x86) + Align = 8 * 8; + else if (Ptrs) + Align = Target.getPointerAlign(0); + else + Align = Target.getIntAlign(); + + if (Target.getTriple().getArch() == llvm::Triple::x86_64) + Width = llvm::RoundUpToAlignment(Width, Align); return std::make_pair(Width, Align); } diff --git a/test/CodeGenCXX/microsoft-abi-member-pointers.cpp b/test/CodeGenCXX/microsoft-abi-member-pointers.cpp index 1da8e1225e..7624cee879 100644 --- a/test/CodeGenCXX/microsoft-abi-member-pointers.cpp +++ b/test/CodeGenCXX/microsoft-abi-member-pointers.cpp @@ -56,20 +56,20 @@ int UnspecSingle::*us_d_memptr; // CHECK: @"\01?p_d_memptr@@3PQPolymorphic@@HQ1@" = global i32 0, align 4 // CHECK: @"\01?m_d_memptr@@3PQMultiple@@HQ1@" = global i32 -1, align 4 // CHECK: @"\01?v_d_memptr@@3PQVirtual@@HQ1@" = global { i32, i32 } -// CHECK: { i32 0, i32 -1 }, align 4 +// CHECK: { i32 0, i32 -1 }, align 8 // CHECK: @"\01?n_d_memptr@@3PQNonZeroVBPtr@@HQ1@" = global { i32, i32 } -// CHECK: { i32 0, i32 -1 }, align 4 +// CHECK: { i32 0, i32 -1 }, align 8 // CHECK: @"\01?u_d_memptr@@3PQUnspecified@@HQ1@" = global { i32, i32, i32 } -// CHECK: { i32 0, i32 0, i32 -1 }, align 4 +// CHECK: { i32 0, i32 0, i32 -1 }, align 8 // CHECK: @"\01?us_d_memptr@@3PQUnspecSingle@@HQ1@" = global { i32, i32, i32 } -// CHECK: { i32 0, i32 0, i32 -1 }, align 4 +// CHECK: { i32 0, i32 0, i32 -1 }, align 8 void (Single ::*s_f_memptr)(); void (Multiple::*m_f_memptr)(); void (Virtual ::*v_f_memptr)(); // CHECK: @"\01?s_f_memptr@@3P8Single@@AEXXZQ1@" = global i8* null, align 4 -// CHECK: @"\01?m_f_memptr@@3P8Multiple@@AEXXZQ1@" = global { i8*, i32 } zeroinitializer, align 4 -// CHECK: @"\01?v_f_memptr@@3P8Virtual@@AEXXZQ1@" = global { i8*, i32, i32 } zeroinitializer, align 4 +// CHECK: @"\01?m_f_memptr@@3P8Multiple@@AEXXZQ1@" = global { i8*, i32 } zeroinitializer, align 8 +// CHECK: @"\01?v_f_memptr@@3P8Virtual@@AEXXZQ1@" = global { i8*, i32, i32 } zeroinitializer, align 8 // We can define Unspecified after locking in the inheritance model. struct Unspecified : Multiple, Virtual { @@ -91,13 +91,13 @@ void (UnspecSingle::*us_f_mp)() = &UnspecSingle::foo; // CHECK: @"\01?s_f_mp@Const@@3P8Single@@AEXXZQ2@" = // CHECK: global i8* bitcast ({{.*}} @"\01?foo@Single@@QAEXXZ" to i8*), align 4 // CHECK: @"\01?m_f_mp@Const@@3P8Multiple@@AEXXZQ2@" = -// CHECK: global { i8*, i32 } { i8* bitcast ({{.*}} @"\01?foo@B2@@QAEXXZ" to i8*), i32 4 }, align 4 +// CHECK: global { i8*, i32 } { i8* bitcast ({{.*}} @"\01?foo@B2@@QAEXXZ" to i8*), i32 4 }, align 8 // CHECK: @"\01?v_f_mp@Const@@3P8Virtual@@AEXXZQ2@" = -// CHECK: global { i8*, i32, i32 } { i8* bitcast ({{.*}} @"\01?foo@Virtual@@QAEXXZ" to i8*), i32 0, i32 0 }, align 4 +// CHECK: global { i8*, i32, i32 } { i8* bitcast ({{.*}} @"\01?foo@Virtual@@QAEXXZ" to i8*), i32 0, i32 0 }, align 8 // CHECK: @"\01?u_f_mp@Const@@3P8Unspecified@@AEXXZQ2@" = -// CHECK: global { i8*, i32, i32, i32 } { i8* bitcast ({{.*}} @"\01?foo@Unspecified@@QAEXXZ" to i8*), i32 0, i32 12, i32 0 }, align 4 +// CHECK: global { i8*, i32, i32, i32 } { i8* bitcast ({{.*}} @"\01?foo@Unspecified@@QAEXXZ" to i8*), i32 0, i32 12, i32 0 }, align 8 // CHECK: @"\01?us_f_mp@Const@@3P8UnspecSingle@@AEXXZQ2@" = -// CHECK: global { i8*, i32, i32, i32 } { i8* bitcast ({{.*}} @"\01?foo@UnspecSingle@@QAEXXZ" to i8*), i32 0, i32 0, i32 0 }, align 4 +// CHECK: global { i8*, i32, i32, i32 } { i8* bitcast ({{.*}} @"\01?foo@UnspecSingle@@QAEXXZ" to i8*), i32 0, i32 0, i32 0 }, align 8 } namespace CastParam { @@ -119,11 +119,11 @@ void (A::*ptr1)(void *) = (void (A::*)(void *)) &A::foo; // Try a reinterpret_cast followed by a memptr conversion. void (C::*ptr2)(void *) = (void (C::*)(void *)) (void (A::*)(void *)) &A::foo; // CHECK: @"\01?ptr2@CastParam@@3P8C@1@AEXPAX@ZQ21@" = -// CHECK: global { i8*, i32 } { i8* bitcast (void ({{.*}})* @"\01?foo@A@CastParam@@QAEXPAU12@@Z" to i8*), i32 4 }, align 4 +// CHECK: global { i8*, i32 } { i8* bitcast (void ({{.*}})* @"\01?foo@A@CastParam@@QAEXPAU12@@Z" to i8*), i32 4 }, align 8 void (C::*ptr3)(void *) = (void (C::*)(void *)) (void (A::*)(void *)) (void (A::*)(A *)) 0; // CHECK: @"\01?ptr3@CastParam@@3P8C@1@AEXPAX@ZQ21@" = -// CHECK: global { i8*, i32 } zeroinitializer, align 4 +// CHECK: global { i8*, i32 } zeroinitializer, align 8 struct D : C { virtual void isPolymorphic(); @@ -156,23 +156,23 @@ void EmitNonVirtualMemberPointers() { void (UnspecWithVBPtr::*u2_f_memptr)() = &UnspecWithVBPtr::foo; // CHECK: define void @"\01?EmitNonVirtualMemberPointers@@YAXXZ"() {{.*}} { // CHECK: alloca i8*, align 4 -// CHECK: alloca { i8*, i32 }, align 4 -// CHECK: alloca { i8*, i32, i32 }, align 4 -// CHECK: alloca { i8*, i32, i32, i32 }, align 4 +// CHECK: alloca { i8*, i32 }, align 8 +// CHECK: alloca { i8*, i32, i32 }, align 8 +// CHECK: alloca { i8*, i32, i32, i32 }, align 8 // CHECK: store i8* bitcast (void (%{{.*}}*)* @"\01?foo@Single@@QAEXXZ" to i8*), i8** %{{.*}}, align 4 // CHECK: store { i8*, i32 } // CHECK: { i8* bitcast (void (%{{.*}}*)* @"\01?foo@Multiple@@QAEXXZ" to i8*), i32 0 }, -// CHECK: { i8*, i32 }* %{{.*}}, align 4 +// CHECK: { i8*, i32 }* %{{.*}}, align 8 // CHECK: store { i8*, i32, i32 } // CHECK: { i8* bitcast (void (%{{.*}}*)* @"\01?foo@Virtual@@QAEXXZ" to i8*), i32 0, i32 0 }, -// CHECK: { i8*, i32, i32 }* %{{.*}}, align 4 +// CHECK: { i8*, i32, i32 }* %{{.*}}, align 8 // CHECK: store { i8*, i32, i32, i32 } // CHECK: { i8* bitcast (void (%{{.*}}*)* @"\01?foo@Unspecified@@QAEXXZ" to i8*), i32 0, i32 12, i32 0 }, -// CHECK: { i8*, i32, i32, i32 }* %{{.*}}, align 4 +// CHECK: { i8*, i32, i32, i32 }* %{{.*}}, align 8 // CHECK: store { i8*, i32, i32, i32 } // CHECK: { i8* bitcast (void (%{{.*}}*)* @"\01?foo@UnspecWithVBPtr@@QAEXXZ" to i8*), // CHECK: i32 0, i32 4, i32 0 }, -// CHECK: { i8*, i32, i32, i32 }* %{{.*}}, align 4 +// CHECK: { i8*, i32, i32, i32 }* %{{.*}}, align 8 // CHECK: ret void // CHECK: } } @@ -219,9 +219,9 @@ void polymorphicMemPtrs() { bool nullTestDataUnspecified(int Unspecified::*mp) { return mp; // CHECK: define zeroext i1 @"\01?nullTestDataUnspecified@@YA_NPQUnspecified@@H@Z"{{.*}} { -// CHECK: %{{.*}} = load { i32, i32, i32 }* %{{.*}}, align 4 -// CHECK: store { i32, i32, i32 } {{.*}} align 4 -// CHECK: %[[mp:.*]] = load { i32, i32, i32 }* %{{.*}}, align 4 +// CHECK: %{{.*}} = load { i32, i32, i32 }* %{{.*}}, align 8 +// CHECK: store { i32, i32, i32 } {{.*}} align 8 +// CHECK: %[[mp:.*]] = load { i32, i32, i32 }* %{{.*}}, align 8 // CHECK: %[[mp0:.*]] = extractvalue { i32, i32, i32 } %[[mp]], 0 // CHECK: %[[cmp0:.*]] = icmp ne i32 %[[mp0]], 0 // CHECK: %[[mp1:.*]] = extractvalue { i32, i32, i32 } %[[mp]], 1 @@ -237,9 +237,9 @@ bool nullTestDataUnspecified(int Unspecified::*mp) { bool nullTestFunctionUnspecified(void (Unspecified::*mp)()) { return mp; // CHECK: define zeroext i1 @"\01?nullTestFunctionUnspecified@@YA_NP8Unspecified@@AEXXZ@Z"{{.*}} { -// CHECK: %{{.*}} = load { i8*, i32, i32, i32 }* %{{.*}}, align 4 -// CHECK: store { i8*, i32, i32, i32 } {{.*}} align 4 -// CHECK: %[[mp:.*]] = load { i8*, i32, i32, i32 }* %{{.*}}, align 4 +// CHECK: %{{.*}} = load { i8*, i32, i32, i32 }* %{{.*}}, align 8 +// CHECK: store { i8*, i32, i32, i32 } {{.*}} align 8 +// CHECK: %[[mp:.*]] = load { i8*, i32, i32, i32 }* %{{.*}}, align 8 // CHECK: %[[mp0:.*]] = extractvalue { i8*, i32, i32, i32 } %[[mp]], 0 // CHECK: %[[cmp0:.*]] = icmp ne i8* %[[mp0]], null // CHECK: ret i1 %[[cmp0]] @@ -252,7 +252,7 @@ int loadDataMemberPointerVirtual(Virtual *o, int Virtual::*memptr) { // data pointer. // CHECK: define i32 @"\01?loadDataMemberPointerVirtual@@YAHPAUVirtual@@PQ1@H@Z"{{.*}} { // CHECK: %[[o:.*]] = load %{{.*}}** %{{.*}}, align 4 -// CHECK: %[[memptr:.*]] = load { i32, i32 }* %{{.*}}, align 4 +// CHECK: %[[memptr:.*]] = load { i32, i32 }* %{{.*}}, align 8 // CHECK: %[[memptr0:.*]] = extractvalue { i32, i32 } %[[memptr:.*]], 0 // CHECK: %[[memptr1:.*]] = extractvalue { i32, i32 } %[[memptr:.*]], 1 // CHECK: %[[v6:.*]] = bitcast %{{.*}}* %[[o]] to i8* @@ -276,7 +276,7 @@ int loadDataMemberPointerUnspecified(Unspecified *o, int Unspecified::*memptr) { // data pointer. // CHECK: define i32 @"\01?loadDataMemberPointerUnspecified@@YAHPAUUnspecified@@PQ1@H@Z"{{.*}} { // CHECK: %[[o:.*]] = load %{{.*}}** %{{.*}}, align 4 -// CHECK: %[[memptr:.*]] = load { i32, i32, i32 }* %{{.*}}, align 4 +// CHECK: %[[memptr:.*]] = load { i32, i32, i32 }* %{{.*}}, align 8 // CHECK: %[[memptr0:.*]] = extractvalue { i32, i32, i32 } %[[memptr:.*]], 0 // CHECK: %[[memptr1:.*]] = extractvalue { i32, i32, i32 } %[[memptr:.*]], 1 // CHECK: %[[memptr2:.*]] = extractvalue { i32, i32, i32 } %[[memptr:.*]], 2 @@ -462,7 +462,7 @@ void (B2::*convertMultipleFuncToB2(void (Multiple::*mp)()))() { // // CHECK: define i32 @"\01?convertMultipleFuncToB2@@YAP8B2@@AEXXZP8Multiple@@AEXXZ@Z"{{.*}} { // CHECK: store -// CHECK: %[[src:.*]] = load { i8*, i32 }* %{{.*}}, align 4 +// CHECK: %[[src:.*]] = load { i8*, i32 }* %{{.*}}, align 8 // CHECK: extractvalue { i8*, i32 } %[[src]], 0 // CHECK: icmp ne i8* %{{.*}}, null // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}} @@ -487,7 +487,7 @@ void (D::*convertCToD(void (C::*mp)()))() { return mp; // CHECK: define void @"\01?convertCToD@Test1@@YAP8D@1@AEXXZP8C@1@AEXXZ@Z"{{.*}} { // CHECK: store -// CHECK: load { i8*, i32, i32 }* %{{.*}}, align 4 +// CHECK: load { i8*, i32, i32 }* %{{.*}}, align 8 // CHECK: extractvalue { i8*, i32, i32 } %{{.*}}, 0 // CHECK: icmp ne i8* %{{.*}}, null // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}} diff --git a/test/Layout/ms-x86-member-pointers.cpp b/test/Layout/ms-x86-member-pointers.cpp new file mode 100644 index 0000000000..89dd211a35 --- /dev/null +++ b/test/Layout/ms-x86-member-pointers.cpp @@ -0,0 +1,80 @@ +// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple i686-pc-win32 -fdump-record-layouts -fms-extensions -fsyntax-only %s 2>&1 | FileCheck %s + +struct __single_inheritance S; +struct __multiple_inheritance M; +struct __virtual_inheritance V; +struct U; + +struct SD { char a; int S::*mp; }; +struct MD { char a; int M::*mp; }; +struct VD { char a; int V::*mp; }; +struct UD { char a; int U::*mp; }; +struct SF { char a; int (S::*mp)(); }; +struct MF { char a; int (M::*mp)(); }; +struct VF { char a; int (V::*mp)(); }; +struct UF { char a; int (U::*mp)(); }; + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct SD +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 4 | int struct S::* mp +// CHECK-NEXT: | [sizeof=8, align=4 +// CHECK-NEXT: | nvsize=8, nvalign=4] + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct MD +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 4 | int struct M::* mp +// CHECK-NEXT: | [sizeof=8, align=4 +// CHECK-NEXT: | nvsize=8, nvalign=4] + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct VD +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 8 | int struct V::* mp +// CHECK-NEXT: | [sizeof=16, align=8 +// CHECK-NEXT: | nvsize=16, nvalign=8] + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct UD +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 8 | int struct U::* mp +// CHECK-NEXT: | [sizeof=24, align=8 +// CHECK-NEXT: | nvsize=24, nvalign=8] + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct SF +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 4 | int (struct S::*)(void) __attribute__((thiscall)) mp +// CHECK-NEXT: | [sizeof=8, align=4 +// CHECK-NEXT: | nvsize=8, nvalign=4] + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct MF +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 8 | int (struct M::*)(void) __attribute__((thiscall)) mp +// CHECK-NEXT: | [sizeof=16, align=8 +// CHECK-NEXT: | nvsize=16, nvalign=8] + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct VF +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 8 | int (struct V::*)(void) __attribute__((thiscall)) mp +// CHECK-NEXT: | [sizeof=24, align=8 +// CHECK-NEXT: | nvsize=24, nvalign=8] + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct UF +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 8 | int (struct U::*)(void) __attribute__((thiscall)) mp +// CHECK-NEXT: | [sizeof=24, align=8 +// CHECK-NEXT: | nvsize=24, nvalign=8] + +char a[sizeof(SD) + + sizeof(MD) + + sizeof(VD) + + sizeof(UD) + + sizeof(SF) + + sizeof(MF) + + sizeof(VF) + + sizeof(UF)]; diff --git a/test/SemaCXX/member-pointer-ms.cpp b/test/SemaCXX/member-pointer-ms.cpp index ca37e07496..aa3caa7428 100644 --- a/test/SemaCXX/member-pointer-ms.cpp +++ b/test/SemaCXX/member-pointer-ms.cpp @@ -19,15 +19,29 @@ struct Foo { }; enum { + kSingleDataAlign = 1 * sizeof(int), + kSingleFunctionAlign = 1 * sizeof(void *), + kMultipleDataAlign = 1 * sizeof(int), + // Everything with more than 1 field is 8 byte aligned, except virtual data + // member pointers on x64 (ugh). + kMultipleFunctionAlign = 8, +#ifdef _M_X64 + kVirtualDataAlign = 4, +#else + kVirtualDataAlign = 8, +#endif + kVirtualFunctionAlign = 8, + kUnspecifiedDataAlign = 8, + kUnspecifiedFunctionAlign = 8, + kSingleDataSize = 1 * sizeof(int), kSingleFunctionSize = 1 * sizeof(void *), kMultipleDataSize = 1 * sizeof(int), kMultipleFunctionSize = 2 * sizeof(void *), kVirtualDataSize = 2 * sizeof(int), kVirtualFunctionSize = 2 * sizeof(int) + 1 * sizeof(void *), - // Unspecified is weird, it's 1 more slot than virtual. - kUnspecifiedDataSize = kVirtualDataSize + 1 * sizeof(int), - kUnspecifiedFunctionSize = kVirtualFunctionSize + 1 * sizeof(void *), + kUnspecifiedDataSize = 3 * sizeof(int), + kUnspecifiedFunctionSize = 2 * sizeof(int) + 2 * sizeof(void *), }; // incomplete types @@ -41,6 +55,13 @@ static_assert(sizeof(void (IncSingle::*)()) == kSingleFunctionSize, ""); static_assert(sizeof(void (IncMultiple::*)()) == kMultipleFunctionSize, ""); static_assert(sizeof(void (IncVirtual::*)()) == kVirtualFunctionSize, ""); +static_assert(__alignof(int IncSingle::*) == kSingleDataAlign, ""); +static_assert(__alignof(int IncMultiple::*) == kMultipleDataAlign, ""); +static_assert(__alignof(int IncVirtual::*) == kVirtualDataAlign, ""); +static_assert(__alignof(void (IncSingle::*)()) == kSingleFunctionAlign, ""); +static_assert(__alignof(void (IncMultiple::*)()) == kMultipleFunctionAlign, ""); +static_assert(__alignof(void (IncVirtual::*)()) == kVirtualFunctionAlign, ""); + // An incomplete type with an unspecified inheritance model seems to take one // more slot than virtual. It's not clear what it's used for yet. class IncUnspecified; -- 2.40.0