From b6487322302f944b82eabbe8088105eece016682 Mon Sep 17 00:00:00 2001 From: Timur Iskhodzhanov Date: Wed, 9 Oct 2013 18:16:58 +0000 Subject: [PATCH] Initialize vtorDisp in class constructors and destructors Reviewed at http://llvm-reviews.chandlerc.com/D1867 llvm-svn: 192312 --- clang/lib/CodeGen/CGCXXABI.h | 6 ++ clang/lib/CodeGen/CGClass.cpp | 3 + clang/lib/CodeGen/MicrosoftCXXABI.cpp | 58 ++++++++++ .../microsoft-abi-virtual-inheritance.cpp | 100 ++++++++++++++++-- 4 files changed, 160 insertions(+), 7 deletions(-) diff --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h index eaeb971dc425..c76769f4252c 100644 --- a/clang/lib/CodeGen/CGCXXABI.h +++ b/clang/lib/CodeGen/CGCXXABI.h @@ -234,6 +234,12 @@ public: virtual llvm::BasicBlock *EmitCtorCompleteObjectHandler(CodeGenFunction &CGF, const CXXRecordDecl *RD); + /// Emit the code to initialize hidden members required + /// to handle virtual inheritance, if needed by the ABI. + virtual void + initializeHiddenVirtualInheritanceMembers(CodeGenFunction &CGF, + const CXXRecordDecl *RD) {} + /// Emit constructor variants required by this ABI. virtual void EmitCXXConstructors(const CXXConstructorDecl *D) = 0; diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp index 137d793c1177..91f42e2f5799 100644 --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -1992,6 +1992,9 @@ void CodeGenFunction::InitializeVTablePointers(const CXXRecordDecl *RD) { /*NearestVBase=*/0, /*OffsetFromNearestVBase=*/CharUnits::Zero(), /*BaseIsNonVirtualPrimaryBase=*/false, RD, VBases); + + if (RD->getNumVBases()) + CGM.getCXXABI().initializeHiddenVirtualInheritanceMembers(*this, RD); } llvm::Value *CodeGenFunction::GetVTablePtr(llvm::Value *This, diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index 7452c860d0be..fd677712e78d 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -67,6 +67,9 @@ public: llvm::BasicBlock *EmitCtorCompleteObjectHandler(CodeGenFunction &CGF, const CXXRecordDecl *RD); + void initializeHiddenVirtualInheritanceMembers(CodeGenFunction &CGF, + const CXXRecordDecl *RD); + void EmitCXXConstructors(const CXXConstructorDecl *D); // Background on MSVC destructors @@ -456,6 +459,61 @@ MicrosoftCXXABI::EmitCtorCompleteObjectHandler(CodeGenFunction &CGF, return SkipVbaseCtorsBB; } +void MicrosoftCXXABI::initializeHiddenVirtualInheritanceMembers( + CodeGenFunction &CGF, const CXXRecordDecl *RD) { + // In most cases, an override for a vbase virtual method can adjust + // the "this" parameter by applying a constant offset. + // However, this is not enough while a constructor or a destructor of some + // class X is being executed if all the following conditions are met: + // - X has virtual bases, (1) + // - X overrides a virtual method M of a vbase Y, (2) + // - X itself is a vbase of the most derived class. + // + // If (1) and (2) are true, the vtorDisp for vbase Y is a hidden member of X + // which holds the extra amount of "this" adjustment we must do when we use + // the X vftables (i.e. during X ctor or dtor). + // Outside the ctors and dtors, the values of vtorDisps are zero. + + const ASTRecordLayout &Layout = getContext().getASTRecordLayout(RD); + typedef ASTRecordLayout::VBaseOffsetsMapTy VBOffsets; + const VBOffsets &VBaseMap = Layout.getVBaseOffsetsMap(); + CGBuilderTy &Builder = CGF.Builder; + + unsigned AS = + cast(getThisValue(CGF)->getType())->getAddressSpace(); + llvm::Value *Int8This = 0; // Initialize lazily. + + for (VBOffsets::const_iterator I = VBaseMap.begin(), E = VBaseMap.end(); + I != E; ++I) { + if (!I->second.hasVtorDisp()) + continue; + + llvm::Value *VBaseOffset = CGM.getCXXABI().GetVirtualBaseClassOffset( + CGF, getThisValue(CGF), RD, I->first); + // FIXME: it doesn't look right that we SExt in GetVirtualBaseClassOffset() + // just to Trunc back immediately. + VBaseOffset = Builder.CreateTruncOrBitCast(VBaseOffset, CGF.Int32Ty); + uint64_t ConstantVBaseOffset = + Layout.getVBaseClassOffset(I->first).getQuantity(); + + // vtorDisp_for_vbase = vbptr[vbase_idx] - offsetof(RD, vbase). + llvm::Value *VtorDispValue = Builder.CreateSub( + VBaseOffset, llvm::ConstantInt::get(CGM.Int32Ty, ConstantVBaseOffset), + "vtordisp.value"); + + if (!Int8This) + Int8This = Builder.CreateBitCast(getThisValue(CGF), + CGF.Int8Ty->getPointerTo(AS)); + llvm::Value *VtorDispPtr = Builder.CreateInBoundsGEP(Int8This, VBaseOffset); + // vtorDisp is always the 32-bits before the vbase in the class layout. + VtorDispPtr = Builder.CreateConstGEP1_32(VtorDispPtr, -4); + VtorDispPtr = Builder.CreateBitCast( + VtorDispPtr, CGF.Int32Ty->getPointerTo(AS), "vtordisp.ptr"); + + Builder.CreateStore(VtorDispValue, VtorDispPtr); + } +} + void MicrosoftCXXABI::EmitCXXConstructors(const CXXConstructorDecl *D) { // There's only one constructor type in this ABI. CGM.EmitGlobal(GlobalDecl(D, Ctor_Complete)); diff --git a/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp b/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp index 3805243c6a4c..b738aba80b16 100644 --- a/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp +++ b/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp @@ -1,6 +1,10 @@ // RUN: %clang_cc1 %s -fno-rtti -cxx-abi microsoft -triple=i386-pc-win32 -emit-llvm -o - | FileCheck %s +// For now, just make sure x86_64 doesn't crash. +// RUN: %clang_cc1 %s -fno-rtti -cxx-abi microsoft -triple=x86_64-pc-win32 -emit-llvm -o %t + struct VBase { + virtual ~VBase(); virtual void foo(); virtual void bar(); int field; @@ -8,27 +12,74 @@ struct VBase { struct B : virtual VBase { B(); + virtual ~B(); virtual void foo(); virtual void bar(); }; B::B() { - // CHECK: @"\01??0B@@QAE@XZ" + // CHECK-LABEL: define x86_thiscallcc %struct.B* @"\01??0B@@QAE@XZ" // CHECK: %[[THIS:.*]] = load %struct.B** // CHECK: br i1 %{{.*}}, label %[[INIT_VBASES:.*]], label %[[SKIP_VBASES:.*]] + + // Don't check the INIT_VBASES case as it's covered by the ctor tests. + // CHECK: %[[SKIP_VBASES]] // CHECK: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8* // CHECK: %[[VBPTR:.*]] = getelementptr inbounds i8* %[[THIS_i8]], i32 0 + // ... // CHECK: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8* // CHECK: %[[VFPTR_i8:.*]] = getelementptr inbounds i8* %[[THIS_i8]], i32 %{{.*}} - // CHECK: %[[VFPTR:.*]] = bitcast i8* %[[VFPTR_i8]] to [2 x i8*]** - // CHECK: store [2 x i8*]* @"\01??_7B@@6B@", [2 x i8*]** %[[VFPTR]] - // FIXME: Should initialize the vtorDisp here. + // CHECK: %[[VFPTR:.*]] = bitcast i8* %[[VFPTR_i8]] to [3 x i8*]** + // CHECK: store [3 x i8*]* @"\01??_7B@@6B@", [3 x i8*]** %[[VFPTR]] + + // Initialize vtorDisp: + // CHECK: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8* + // CHECK: %[[VBPTR:.*]] = getelementptr inbounds i8* %[[THIS_i8]], i32 0 + // ... + // CHECK: %[[VBASE_OFFSET:.*]] = add nsw i32 0, %{{.*}} + // CHECK: %[[VTORDISP_VAL:.*]] = sub i32 %[[VBASE_OFFSET]], 8 + // CHECK: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8* + // CHECK: %[[VBASE_i8:.*]] = getelementptr inbounds i8* %[[THIS_i8]], i32 %[[VBASE_OFFSET]] + // CHECK: %[[VTORDISP_i8:.*]] = getelementptr i8* %[[VBASE_i8]], i32 -4 + // CHECK: %[[VTORDISP_PTR:.*]] = bitcast i8* %[[VTORDISP_i8]] to i32* + // CHECK: store i32 %[[VTORDISP_VAL]], i32* %[[VTORDISP_PTR]] + + // CHECK: ret +} + +B::~B() { + // CHECK-LABEL: define x86_thiscallcc void @"\01??1B@@UAE@XZ" + // CHECK: %[[THIS:.*]] = load %struct.B** + + // Restore the vfptr that could have been changed by a subclass. + // CHECK: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8* + // CHECK: %[[VBPTR:.*]] = getelementptr inbounds i8* %[[THIS_i8]], i32 0 + // ... + // CHECK: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8* + // CHECK: %[[VFPTR_i8:.*]] = getelementptr inbounds i8* %[[THIS_i8]], i32 %{{.*}} + // CHECK: %[[VFPTR:.*]] = bitcast i8* %[[VFPTR_i8]] to [3 x i8*]** + // CHECK: store [3 x i8*]* @"\01??_7B@@6B@", [3 x i8*]** %[[VFPTR]] + + // Initialize vtorDisp: + // CHECK: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8* + // CHECK: %[[VBPTR:.*]] = getelementptr inbounds i8* %[[THIS_i8]], i32 0 + // ... + // CHECK: %[[VBASE_OFFSET:.*]] = add nsw i32 0, %{{.*}} + // CHECK: %[[VTORDISP_VAL:.*]] = sub i32 %[[VBASE_OFFSET]], 8 + // CHECK: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8* + // CHECK: %[[VBASE_i8:.*]] = getelementptr inbounds i8* %[[THIS_i8]], i32 %[[VBASE_OFFSET]] + // CHECK: %[[VTORDISP_i8:.*]] = getelementptr i8* %[[VBASE_i8]], i32 -4 + // CHECK: %[[VTORDISP_PTR:.*]] = bitcast i8* %[[VTORDISP_i8]] to i32* + // CHECK: store i32 %[[VTORDISP_VAL]], i32* %[[VTORDISP_PTR]] + + foo(); // Avoid the "trivial destructor" optimization. + // CHECK: ret } void B::foo() { -// CHECK: define x86_thiscallcc void @"\01?foo@B@@UAEXXZ"(i8* +// CHECK-LABEL: define x86_thiscallcc void @"\01?foo@B@@UAEXXZ"(i8* // // B::foo gets 'this' cast to VBase* in ECX (i.e. this+8) so we // need to adjust 'this' before use. @@ -58,7 +109,7 @@ void B::foo() { } void call_vbase_bar(B *obj) { -// CHECK: define void @"\01?call_vbase_bar@@YAXPAUB@@@Z"(%struct.B* %obj) +// CHECK-LABEL: define void @"\01?call_vbase_bar@@YAXPAUB@@@Z"(%struct.B* %obj) // CHECK: %[[OBJ:.*]] = load %struct.B obj->bar(); @@ -76,7 +127,7 @@ void call_vbase_bar(B *obj) { // CHECK: %[[VBASE_i8:.*]] = getelementptr inbounds i8* %[[OBJ_i8]], i32 %[[VBOFFSET]] // CHECK: %[[VFPTR:.*]] = bitcast i8* %[[VBASE_i8]] to void (i8*)*** // CHECK: %[[VFTABLE:.*]] = load void (i8*)*** %[[VFPTR]] -// CHECK: %[[VFUN:.*]] = getelementptr inbounds void (i8*)** %[[VFTABLE]], i64 1 +// CHECK: %[[VFUN:.*]] = getelementptr inbounds void (i8*)** %[[VFTABLE]], i64 2 // CHECK: %[[VFUN_VALUE:.*]] = load void (i8*)** %[[VFUN]] // // CHECK: %[[OBJ_i8:.*]] = bitcast %struct.B* %[[OBJ]] to i8* @@ -101,3 +152,38 @@ struct C : B { // Used to crash on an assertion. C::C() {} + +namespace multiple_vbases { +struct A { + virtual void a(); +}; + +struct B { + virtual void b(); +}; + +struct C { + virtual void c(); +}; + +struct D : virtual A, virtual B, virtual C { + virtual void a(); + virtual void b(); + virtual void c(); + D(); +}; + +D::D() { + // CHECK-LABEL: define x86_thiscallcc %"struct.multiple_vbases::D"* @"\01??0D@multiple_vbases@@QAE@XZ" + // Just make sure we emit 3 vtordisps after initializing vfptrs. + // CHECK: store [1 x i8*]* @"\01??_7D@multiple_vbases@@6BA@1@@", [1 x i8*]** %{{.*}} + // CHECK: store [1 x i8*]* @"\01??_7D@multiple_vbases@@6BB@1@@", [1 x i8*]** %{{.*}} + // CHECK: store [1 x i8*]* @"\01??_7D@multiple_vbases@@6BC@1@@", [1 x i8*]** %{{.*}} + // ... + // CHECK: store i32 %{{.*}}, i32* %{{.*}} + // CHECK: store i32 %{{.*}}, i32* %{{.*}} + // CHECK: store i32 %{{.*}}, i32* %{{.*}} + // CHECK: ret +} + +}