ahatanak created this revision. ahatanak added reviewers: rjmccall, vsk. This patch fixes what looks like a bug in IRGen's member initialization where it uses the alignment of a complete object instead of its non-virtual alignment when emitting the base constructor.
For example, when emitting the base constructor for B, IRGen uses a 16-byte alignment to initialize B::x because B's alignment is 16-byte (which is because A is 16-byte aligned). This is not correct since A is a virtual base of B and therefore we cannot guarantee that the pointer to B that is passed to the base constructor is always 16-byte aligned. struct A { __attribute__((aligned(16))) double data1; }; struct B : public virtual A { B() : x(123) {} double a; int x; }; struct C : public virtual B {}; void test() { B b; C c; } To fix the bug, this patch calls CGF.MakeNaturalAlignPointeeAddrLValue in EmitMemberInitializer to create an LValue that uses the non-virtual alignment of the class. rdar://problem/36382481 https://reviews.llvm.org/D42521 Files: lib/CodeGen/CGClass.cpp test/CodeGenCXX/virtual-bases.cpp Index: test/CodeGenCXX/virtual-bases.cpp =================================================================== --- test/CodeGenCXX/virtual-bases.cpp +++ test/CodeGenCXX/virtual-bases.cpp @@ -46,3 +46,37 @@ D::D() { } } + +namespace virtualBaseAlignment { + +// Check that the store to B::x in the base constructor has an 8-byte alignment. + +// CHECK: define linkonce_odr void @_ZN20virtualBaseAlignment1BC1Ev(%[[STRUCT_B:.*]]* %[[THIS:.*]]) +// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8 +// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], align 8 +// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], align 8 +// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* %[[THIS1]], i32 0, i32 2 +// CHECK: store i32 123, i32* %[[X]], align 16 + +// CHECK: define linkonce_odr void @_ZN20virtualBaseAlignment1BC2Ev(%[[STRUCT_B]]* %[[THIS:.*]], i8** %{{.*}}) +// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8 +// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], align 8 +// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], align 8 +// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* %[[THIS1]], i32 0, i32 2 +// CHECK: store i32 123, i32* %[[X]], align 8 + +struct A { + __attribute__((aligned(16))) double data1; +}; + +struct B : public virtual A { + B() : x(123) {} + double a; + int x; +}; + +struct C : public virtual B {}; + +void test() { B b; C c; } + +} Index: lib/CodeGen/CGClass.cpp =================================================================== --- lib/CodeGen/CGClass.cpp +++ lib/CodeGen/CGClass.cpp @@ -615,7 +615,14 @@ llvm::Value *ThisPtr = CGF.LoadCXXThis(); QualType RecordTy = CGF.getContext().getTypeDeclType(ClassDecl); - LValue LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy); + LValue LHS; + + // If a base constructor is being emitted, create an LValue that has the + // non-virtual alignment. + if (CGF.CurGD.getCtorType() == Ctor_Base) + LHS = CGF.MakeNaturalAlignPointeeAddrLValue(ThisPtr, RecordTy); + else + LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy); EmitLValueForAnyFieldInitialization(CGF, MemberInit, LHS);
Index: test/CodeGenCXX/virtual-bases.cpp =================================================================== --- test/CodeGenCXX/virtual-bases.cpp +++ test/CodeGenCXX/virtual-bases.cpp @@ -46,3 +46,37 @@ D::D() { } } + +namespace virtualBaseAlignment { + +// Check that the store to B::x in the base constructor has an 8-byte alignment. + +// CHECK: define linkonce_odr void @_ZN20virtualBaseAlignment1BC1Ev(%[[STRUCT_B:.*]]* %[[THIS:.*]]) +// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8 +// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], align 8 +// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], align 8 +// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* %[[THIS1]], i32 0, i32 2 +// CHECK: store i32 123, i32* %[[X]], align 16 + +// CHECK: define linkonce_odr void @_ZN20virtualBaseAlignment1BC2Ev(%[[STRUCT_B]]* %[[THIS:.*]], i8** %{{.*}}) +// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8 +// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], align 8 +// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], align 8 +// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* %[[THIS1]], i32 0, i32 2 +// CHECK: store i32 123, i32* %[[X]], align 8 + +struct A { + __attribute__((aligned(16))) double data1; +}; + +struct B : public virtual A { + B() : x(123) {} + double a; + int x; +}; + +struct C : public virtual B {}; + +void test() { B b; C c; } + +} Index: lib/CodeGen/CGClass.cpp =================================================================== --- lib/CodeGen/CGClass.cpp +++ lib/CodeGen/CGClass.cpp @@ -615,7 +615,14 @@ llvm::Value *ThisPtr = CGF.LoadCXXThis(); QualType RecordTy = CGF.getContext().getTypeDeclType(ClassDecl); - LValue LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy); + LValue LHS; + + // If a base constructor is being emitted, create an LValue that has the + // non-virtual alignment. + if (CGF.CurGD.getCtorType() == Ctor_Base) + LHS = CGF.MakeNaturalAlignPointeeAddrLValue(ThisPtr, RecordTy); + else + LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy); EmitLValueForAnyFieldInitialization(CGF, MemberInit, LHS);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits