r218577 - CodeGen: Don't crash when initializing pointer-to-member fields in bases
David Majnemer
david.majnemer at gmail.com
Sat Sep 27 23:39:30 PDT 2014
Author: majnemer
Date: Sun Sep 28 01:39:30 2014
New Revision: 218577
URL: http://llvm.org/viewvc/llvm-project?rev=218577&view=rev
Log:
CodeGen: Don't crash when initializing pointer-to-member fields in bases
Clang uses two types to talk about a C++ class, the
NonVirtualBaseLLVMType and the LLVMType. Previously, we would allow one
of these to be packed and the other not.
This is problematic. If both don't agree on a common subset of fields,
then routines like getLLVMFieldNo will point to the wrong field. Solve
this by copying the 'packed'-ness of the complete type to the
non-virtual subobject. For this to work, we need to take into account
the non-virtual subobject's size and alignment when we are computing the
layout of the complete object.
This fixes PR21089.
Modified:
cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp
cfe/trunk/test/CodeGenCXX/class-layout.cpp
cfe/trunk/test/CodeGenCXX/pointers-to-data-members.cpp
cfe/trunk/test/CodeGenCXX/pr18962.cpp
Modified: cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp?rev=218577&r1=218576&r2=218577&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp Sun Sep 28 01:39:30 2014
@@ -93,7 +93,7 @@ struct CGRecordLowering {
bool operator <(const MemberInfo& a) const { return Offset < a.Offset; }
};
// The constructor.
- CGRecordLowering(CodeGenTypes &Types, const RecordDecl *D);
+ CGRecordLowering(CodeGenTypes &Types, const RecordDecl *D, bool Packed);
// Short helper routines.
/// \brief Constructs a MemberInfo instance from an offset and llvm::Type *.
MemberInfo StorageInfo(CharUnits Offset, llvm::Type *Data) {
@@ -174,7 +174,7 @@ struct CGRecordLowering {
/// padding that is or can potentially be used.
void clipTailPadding();
/// \brief Determines if we need a packed llvm struct.
- void determinePacked();
+ void determinePacked(bool NVBaseType);
/// \brief Inserts padding everwhere it's needed.
void insertPadding();
/// \brief Fills out the structures that are ultimately consumed.
@@ -203,12 +203,12 @@ private:
};
} // namespace {
-CGRecordLowering::CGRecordLowering(CodeGenTypes &Types, const RecordDecl *D)
+CGRecordLowering::CGRecordLowering(CodeGenTypes &Types, const RecordDecl *D, bool Packed)
: Types(Types), Context(Types.getContext()), D(D),
RD(dyn_cast<CXXRecordDecl>(D)),
Layout(Types.getContext().getASTRecordLayout(D)),
DataLayout(Types.getDataLayout()), IsZeroInitializable(true),
- IsZeroInitializableAsBase(true), Packed(false) {}
+ IsZeroInitializableAsBase(true), Packed(Packed) {}
void CGRecordLowering::setBitFieldInfo(
const FieldDecl *FD, CharUnits StartOffset, llvm::Type *StorageType) {
@@ -269,7 +269,7 @@ void CGRecordLowering::lower(bool NVBase
std::stable_sort(Members.begin(), Members.end());
Members.push_back(StorageInfo(Size, getIntNType(8)));
clipTailPadding();
- determinePacked();
+ determinePacked(NVBaseType);
insertPadding();
Members.pop_back();
calculateZeroInit();
@@ -533,8 +533,13 @@ void CGRecordLowering::clipTailPadding()
}
}
-void CGRecordLowering::determinePacked() {
+void CGRecordLowering::determinePacked(bool NVBaseType) {
+ if (Packed)
+ return;
CharUnits Alignment = CharUnits::One();
+ CharUnits NVAlignment = CharUnits::One();
+ CharUnits NVSize =
+ !NVBaseType && RD ? Layout.getNonVirtualSize() : CharUnits::Zero();
for (std::vector<MemberInfo>::const_iterator Member = Members.begin(),
MemberEnd = Members.end();
Member != MemberEnd; ++Member) {
@@ -544,12 +549,19 @@ void CGRecordLowering::determinePacked()
// then the entire record must be packed.
if (Member->Offset % getAlignment(Member->Data))
Packed = true;
+ if (Member->Offset < NVSize)
+ NVAlignment = std::max(NVAlignment, getAlignment(Member->Data));
Alignment = std::max(Alignment, getAlignment(Member->Data));
}
// If the size of the record (the capstone's offset) is not a multiple of the
// record's alignment, it must be packed.
if (Members.back().Offset % Alignment)
Packed = true;
+ // If the non-virtual sub-object is not a multiple of the non-virtual
+ // sub-object's alignment, it must be packed. We cannot have a packed
+ // non-virtual sub-object and an unpacked complete object or vise versa.
+ if (NVSize % NVAlignment)
+ Packed = true;
// Update the alignment of the sentinal.
if (!Packed)
Members.back().Data = getIntNType(Context.toBits(Alignment));
@@ -641,20 +653,24 @@ CGBitFieldInfo CGBitFieldInfo::MakeInfo(
CGRecordLayout *CodeGenTypes::ComputeRecordLayout(const RecordDecl *D,
llvm::StructType *Ty) {
- CGRecordLowering Builder(*this, D);
+ CGRecordLowering Builder(*this, D, /*Packed=*/false);
- Builder.lower(false);
+ Builder.lower(/*NonVirtualBaseType=*/false);
// If we're in C++, compute the base subobject type.
llvm::StructType *BaseTy = nullptr;
if (isa<CXXRecordDecl>(D) && !D->isUnion() && !D->hasAttr<FinalAttr>()) {
BaseTy = Ty;
if (Builder.Layout.getNonVirtualSize() != Builder.Layout.getSize()) {
- CGRecordLowering BaseBuilder(*this, D);
- BaseBuilder.lower(true);
+ CGRecordLowering BaseBuilder(*this, D, /*Packed=*/Builder.Packed);
+ BaseBuilder.lower(/*NonVirtualBaseType=*/true);
BaseTy = llvm::StructType::create(
getLLVMContext(), BaseBuilder.FieldTypes, "", BaseBuilder.Packed);
addRecordTypeName(D, BaseTy, ".base");
+ // BaseTy and Ty must agree on their packedness for getLLVMFieldNo to work
+ // on both of them with the same index.
+ assert(Builder.Packed == BaseBuilder.Packed &&
+ "Non-virtual and complete types must agree on packedness");
}
}
Modified: cfe/trunk/test/CodeGenCXX/class-layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/class-layout.cpp?rev=218577&r1=218576&r2=218577&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/class-layout.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/class-layout.cpp Sun Sep 28 01:39:30 2014
@@ -14,7 +14,7 @@ namespace Test2 {
namespace Test3 {
// C should have a vtable pointer.
- // CHECK: %"struct.Test3::A" = type { i32 (...)**, i32 }
+ // CHECK: %"struct.Test3::A" = type <{ i32 (...)**, i32, [4 x i8] }>
struct A { virtual void f(); int a; } *a;
}
Modified: cfe/trunk/test/CodeGenCXX/pointers-to-data-members.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pointers-to-data-members.cpp?rev=218577&r1=218576&r2=218577&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/pointers-to-data-members.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/pointers-to-data-members.cpp Sun Sep 28 01:39:30 2014
@@ -55,7 +55,7 @@ namespace ZeroInit {
};
struct C : A, B { int j; };
- // CHECK-GLOBAL: @_ZN8ZeroInit1cE = global {{%.*}} { %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::B" { [10 x %"struct.ZeroInit::A"] [%"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }], i8 0, i64 -1 }, i32 0 }, align 8
+ // CHECK-GLOBAL: @_ZN8ZeroInit1cE = global {{%.*}} <{ %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::B" { [10 x %"struct.ZeroInit::A"] [%"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }, %"struct.ZeroInit::A" { i64 -1, i32 0 }], i8 0, i64 -1 }, i32 0, [4 x i8] zeroinitializer }>, align 8
C c;
}
@@ -255,4 +255,17 @@ namespace PR13097 {
// CHECK: call void @_ZN7PR130971XC1ERKS0_
}
+namespace PR21089 {
+struct A {
+ bool : 1;
+ int A::*x;
+ bool y;
+ A();
+};
+struct B : A {
+};
+B b;
+// CHECK-GLOBAL: @_ZN7PR210891bE = global %"struct.PR21089::B" { %"struct.PR21089::A.base" <{ i8 0, [7 x i8] zeroinitializer, i64 -1, i8 0 }>, [7 x i8] zeroinitializer }, align 8
+}
+
// CHECK-O3: attributes [[NUW]] = { nounwind readnone{{.*}} }
Modified: cfe/trunk/test/CodeGenCXX/pr18962.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pr18962.cpp?rev=218577&r1=218576&r2=218577&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/pr18962.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/pr18962.cpp Sun Sep 28 01:39:30 2014
@@ -26,7 +26,7 @@ fn2(C *) {
// We end up using an opaque type for 'append' to avoid circular references.
// CHECK: %class.A = type { {}* }
-// CHECK: %class.C = type { %class.D*, %class.B }
+// CHECK: %class.C = type <{ %class.D*, %class.B, [3 x i8] }>
// CHECK: %class.D = type { %class.C.base, [3 x i8] }
// CHECK: %class.C.base = type <{ %class.D*, %class.B }>
// CHECK: %class.B = type { i8 }
More information about the cfe-commits
mailing list