r203949 - Fix PR19104: Incorrect handling of non-virtual calls of virtual methods
Timur Iskhodzhanov
timurrrr at google.com
Fri Mar 14 10:43:38 PDT 2014
Author: timurrrr
Date: Fri Mar 14 12:43:37 2014
New Revision: 203949
URL: http://llvm.org/viewvc/llvm-project?rev=203949&view=rev
Log:
Fix PR19104: Incorrect handling of non-virtual calls of virtual methods
Reviewed at http://llvm-reviews.chandlerc.com/D3054
Modified:
cfe/trunk/lib/CodeGen/CGCXXABI.h
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=203949&r1=203948&r2=203949&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCXXABI.h (original)
+++ cfe/trunk/lib/CodeGen/CGCXXABI.h Fri Mar 14 12:43:37 2014
@@ -258,10 +258,12 @@ public:
}
/// Perform ABI-specific "this" argument adjustment required prior to
- /// a virtual function call.
- virtual llvm::Value *adjustThisArgumentForVirtualCall(CodeGenFunction &CGF,
- GlobalDecl GD,
- llvm::Value *This) {
+ /// a call of a virtual function.
+ /// The "VirtualCall" argument is true iff the call itself is virtual.
+ virtual llvm::Value *
+ adjustThisArgumentForVirtualFunctionCall(CodeGenFunction &CGF, GlobalDecl GD,
+ llvm::Value *This,
+ bool VirtualCall) {
return This;
}
Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=203949&r1=203948&r2=203949&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Fri Mar 14 12:43:37 2014
@@ -220,8 +220,10 @@ RValue CodeGenFunction::EmitCXXMemberCal
}
}
- if (MD->isVirtual())
- This = CGM.getCXXABI().adjustThisArgumentForVirtualCall(*this, MD, This);
+ if (MD->isVirtual()) {
+ This = CGM.getCXXABI().adjustThisArgumentForVirtualFunctionCall(
+ *this, MD, This, UseVirtualCall);
+ }
return EmitCXXMemberCall(MD, CE->getExprLoc(), Callee, ReturnValue, This,
/*ImplicitParam=*/0, QualType(),
Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=203949&r1=203948&r2=203949&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Fri Mar 14 12:43:37 2014
@@ -934,9 +934,6 @@ void ItaniumCXXABI::EmitDestructorCall(C
if (!Callee)
Callee = CGM.GetAddrOfCXXDestructor(DD, Type);
- if (DD->isVirtual())
- This = adjustThisArgumentForVirtualCall(CGF, GD, This);
-
// FIXME: Provide a source location here.
CGF.EmitCXXMemberCall(DD, SourceLocation(), Callee, ReturnValueSlot(), This,
VTT, VTTTy, 0, 0);
Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=203949&r1=203948&r2=203949&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Fri Mar 14 12:43:37 2014
@@ -148,9 +148,10 @@ public:
return MD->getParent();
}
- llvm::Value *adjustThisArgumentForVirtualCall(CodeGenFunction &CGF,
- GlobalDecl GD,
- llvm::Value *This) override;
+ llvm::Value *
+ adjustThisArgumentForVirtualFunctionCall(CodeGenFunction &CGF, GlobalDecl GD,
+ llvm::Value *This,
+ bool VirtualCall) override;
void addImplicitStructorParams(CodeGenFunction &CGF, QualType &ResTy,
FunctionArgList &Params) override;
@@ -282,6 +283,8 @@ private:
return C ? C : getZeroInt();
}
+ CharUnits getVirtualFunctionPrologueThisAdjustment(GlobalDecl GD);
+
void
GetNullMemberPointerFields(const MemberPointerType *MPT,
llvm::SmallVectorImpl<llvm::Constant *> &fields);
@@ -582,12 +585,61 @@ void MicrosoftCXXABI::EmitCXXDestructors
CGM.EmitGlobal(GlobalDecl(D, Dtor_Base));
}
-llvm::Value *MicrosoftCXXABI::adjustThisArgumentForVirtualCall(
- CodeGenFunction &CGF, GlobalDecl GD, llvm::Value *This) {
+CharUnits
+MicrosoftCXXABI::getVirtualFunctionPrologueThisAdjustment(GlobalDecl GD) {
+ GD = GD.getCanonicalDecl();
+ const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
+
+ GlobalDecl LookupGD = GD;
+ if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
+ // Complete destructors take a pointer to the complete object as a
+ // parameter, thus don't need this adjustment.
+ if (GD.getDtorType() == Dtor_Complete)
+ return CharUnits();
+
+ // There's no Dtor_Base in vftable but it shares the this adjustment with
+ // the deleting one, so look it up instead.
+ LookupGD = GlobalDecl(DD, Dtor_Deleting);
+ }
+
+ MicrosoftVTableContext::MethodVFTableLocation ML =
+ CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
+ CharUnits Adjustment = ML.VFPtrOffset;
+
+ // Normal virtual instance methods need to adjust from the vfptr that first
+ // defined the virtual method to the virtual base subobject, but destructors
+ // do not. The vector deleting destructor thunk applies this adjustment for
+ // us if necessary.
+ if (isa<CXXDestructorDecl>(MD))
+ Adjustment = CharUnits::Zero();
+
+ if (ML.VBase) {
+ const ASTRecordLayout &DerivedLayout =
+ CGM.getContext().getASTRecordLayout(MD->getParent());
+ Adjustment += DerivedLayout.getVBaseClassOffset(ML.VBase);
+ }
+
+ return Adjustment;
+}
+
+llvm::Value *MicrosoftCXXABI::adjustThisArgumentForVirtualFunctionCall(
+ CodeGenFunction &CGF, GlobalDecl GD, llvm::Value *This, bool VirtualCall) {
+ if (!VirtualCall) {
+ // If the call of a virtual function is not virtual, we just have to
+ // compensate for the adjustment the virtual function does in its prologue.
+ CharUnits Adjustment = getVirtualFunctionPrologueThisAdjustment(GD);
+ if (Adjustment.isZero())
+ return This;
+
+ unsigned AS = cast<llvm::PointerType>(This->getType())->getAddressSpace();
+ llvm::Type *charPtrTy = CGF.Int8Ty->getPointerTo(AS);
+ This = CGF.Builder.CreateBitCast(This, charPtrTy);
+ assert(Adjustment.isPositive());
+ return CGF.Builder.CreateConstGEP1_32(This, Adjustment.getQuantity());
+ }
+
GD = GD.getCanonicalDecl();
const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
- // FIXME: consider splitting the vdtor vs regular method code into two
- // functions.
GlobalDecl LookupGD = GD;
if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
@@ -614,49 +666,10 @@ llvm::Value *MicrosoftCXXABI::adjustThis
StaticOffset = CharUnits::Zero();
if (ML.VBase) {
- bool AvoidVirtualOffset = false;
- if (isa<CXXDestructorDecl>(MD) && GD.getDtorType() == Dtor_Base) {
- // A base destructor can only be called from a complete destructor of the
- // same record type or another destructor of a more derived type;
- // or a constructor of the same record type if an exception is thrown.
- assert(isa<CXXDestructorDecl>(CGF.CurGD.getDecl()) ||
- isa<CXXConstructorDecl>(CGF.CurGD.getDecl()));
- const CXXRecordDecl *CurRD =
- cast<CXXMethodDecl>(CGF.CurGD.getDecl())->getParent();
-
- if (MD->getParent() == CurRD) {
- if (isa<CXXDestructorDecl>(CGF.CurGD.getDecl()))
- assert(CGF.CurGD.getDtorType() == Dtor_Complete);
- if (isa<CXXConstructorDecl>(CGF.CurGD.getDecl()))
- assert(CGF.CurGD.getCtorType() == Ctor_Complete);
- // We're calling the main base dtor from a complete structor,
- // so we know the "this" offset statically.
- AvoidVirtualOffset = true;
- } else {
- // Let's see if we try to call a destructor of a non-virtual base.
- for (const auto &I : CurRD->bases()) {
- if (I.getType()->getAsCXXRecordDecl() != MD->getParent())
- continue;
- // If we call a base destructor for a non-virtual base, we statically
- // know where it expects the vfptr and "this" to be.
- // The total offset should reflect the adjustment done by
- // adjustThisParameterInVirtualFunctionPrologue().
- AvoidVirtualOffset = true;
- break;
- }
- }
- }
-
- if (AvoidVirtualOffset) {
- const ASTRecordLayout &Layout =
- CGF.getContext().getASTRecordLayout(MD->getParent());
- StaticOffset += Layout.getVBaseClassOffset(ML.VBase);
- } else {
- This = CGF.Builder.CreateBitCast(This, charPtrTy);
- llvm::Value *VBaseOffset =
- GetVirtualBaseClassOffset(CGF, This, MD->getParent(), ML.VBase);
- This = CGF.Builder.CreateInBoundsGEP(This, VBaseOffset);
- }
+ This = CGF.Builder.CreateBitCast(This, charPtrTy);
+ llvm::Value *VBaseOffset =
+ GetVirtualBaseClassOffset(CGF, This, MD->getParent(), ML.VBase);
+ This = CGF.Builder.CreateInBoundsGEP(This, VBaseOffset);
}
if (!StaticOffset.isZero()) {
assert(StaticOffset.isPositive());
@@ -716,44 +729,12 @@ void MicrosoftCXXABI::addImplicitStructo
llvm::Value *MicrosoftCXXABI::adjustThisParameterInVirtualFunctionPrologue(
CodeGenFunction &CGF, GlobalDecl GD, llvm::Value *This) {
- GD = GD.getCanonicalDecl();
- const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
-
- GlobalDecl LookupGD = GD;
- if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
- // Complete destructors take a pointer to the complete object as a
- // parameter, thus don't need this adjustment.
- if (GD.getDtorType() == Dtor_Complete)
- return This;
-
- // There's no Dtor_Base in vftable but it shares the this adjustment with
- // the deleting one, so look it up instead.
- LookupGD = GlobalDecl(DD, Dtor_Deleting);
- }
-
// In this ABI, every virtual function takes a pointer to one of the
// subobjects that first defines it as the 'this' parameter, rather than a
// pointer to the final overrider subobject. Thus, we need to adjust it back
// to the final overrider subobject before use.
// See comments in the MicrosoftVFTableContext implementation for the details.
-
- MicrosoftVTableContext::MethodVFTableLocation ML =
- CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
- CharUnits Adjustment = ML.VFPtrOffset;
-
- // Normal virtual instance methods need to adjust from the vfptr that first
- // defined the virtual method to the virtual base subobject, but destructors
- // do not. The vector deleting destructor thunk applies this adjustment for
- // us if necessary.
- if (isa<CXXDestructorDecl>(MD))
- Adjustment = CharUnits::Zero();
-
- if (ML.VBase) {
- const ASTRecordLayout &DerivedLayout =
- CGF.getContext().getASTRecordLayout(MD->getParent());
- Adjustment += DerivedLayout.getVBaseClassOffset(ML.VBase);
- }
-
+ CharUnits Adjustment = getVirtualFunctionPrologueThisAdjustment(GD);
if (Adjustment.isZero())
return This;
@@ -833,8 +814,12 @@ void MicrosoftCXXABI::EmitDestructorCall
bool Delegating, llvm::Value *This) {
llvm::Value *Callee = CGM.GetAddrOfCXXDestructor(DD, Type);
- if (DD->isVirtual())
- This = adjustThisArgumentForVirtualCall(CGF, GlobalDecl(DD, Type), This);
+ if (DD->isVirtual()) {
+ assert(Type != CXXDtorType::Dtor_Deleting &&
+ "The deleting destructor should only be called via a virtual call");
+ This = adjustThisArgumentForVirtualFunctionCall(CGF, GlobalDecl(DD, Type),
+ This, false);
+ }
// FIXME: Provide a source location here.
CGF.EmitCXXMemberCall(DD, SourceLocation(), Callee, ReturnValueSlot(), This,
@@ -958,7 +943,8 @@ llvm::Value *MicrosoftCXXABI::getVirtual
CGBuilderTy &Builder = CGF.Builder;
Ty = Ty->getPointerTo()->getPointerTo();
- llvm::Value *VPtr = adjustThisArgumentForVirtualCall(CGF, GD, This);
+ llvm::Value *VPtr =
+ adjustThisArgumentForVirtualFunctionCall(CGF, GD, This, true);
llvm::Value *VTable = CGF.GetVTablePtr(VPtr, Ty);
MicrosoftVTableContext::MethodVFTableLocation ML =
@@ -988,7 +974,7 @@ void MicrosoftCXXABI::EmitVirtualDestruc
llvm::ConstantInt::get(llvm::IntegerType::getInt32Ty(CGF.getLLVMContext()),
DtorType == Dtor_Deleting);
- This = adjustThisArgumentForVirtualCall(CGF, GD, This);
+ This = adjustThisArgumentForVirtualFunctionCall(CGF, GD, This, true);
CGF.EmitCXXMemberCall(Dtor, CallLoc, Callee, ReturnValueSlot(), This,
ImplicitParam, Context.IntTy, 0, 0);
}
Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp?rev=203949&r1=203948&r2=203949&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp Fri Mar 14 12:43:37 2014
@@ -339,3 +339,41 @@ void callC() { C x; }
// CHECK2: ret
}
+
+namespace test3 {
+// PR19104: A non-virtual call of a virtual method doesn't use vftable thunks,
+// so requires only static adjustment which is different to the one used
+// for virtual calls.
+struct A {
+ virtual void foo();
+};
+
+struct B : virtual A {
+ virtual void bar();
+};
+
+struct C : virtual A {
+ virtual void foo();
+};
+
+struct D : B, C {
+ virtual void bar();
+ int field; // Laid out between C and A subobjects in D.
+};
+
+void D::bar() {
+ // CHECK-LABEL: define x86_thiscallcc void @"\01?bar at D@test3@@UAEXXZ"(%"struct.test3::D"* %this)
+
+ C::foo();
+ // Shouldn't need any vbtable lookups. All we have to do is adjust to C*,
+ // then compensate for the adjustment performed in the C::foo() prologue.
+ // CHECK-NOT: load i8**
+ // CHECK: %[[OBJ_i8:.*]] = bitcast %"struct.test3::D"* %{{.*}} to i8*
+ // CHECK: %[[C_i8:.*]] = getelementptr inbounds i8* %[[OBJ_i8]], i32 8
+ // CHECK: %[[C:.*]] = bitcast i8* %[[C_i8]] to %"struct.test3::C"*
+ // CHECK: %[[C_i8:.*]] = bitcast %"struct.test3::C"* %[[C]] to i8*
+ // CHECK: %[[ARG:.*]] = getelementptr i8* %[[C_i8]], i32 4
+ // CHECK: call x86_thiscallcc void @"\01?foo at C@test3@@UAEXXZ"(i8* %[[ARG]])
+ // CHECK: ret
+}
+}
More information about the cfe-commits
mailing list