<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Oct 16, 2013 at 11:24 AM, Timur Iskhodzhanov <span dir="ltr"><<a href="mailto:timurrrr@google.com" target="_blank">timurrrr@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: timurrrr<br>
Date: Wed Oct 16 13:24:06 2013<br>
New Revision: 192822<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=192822&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=192822&view=rev</a><br>
Log:<br>
[-cxx-abi microsoft] Fix this argument/parameter offsets for virtual destructors in the presence of virtual bases<br>
<br>
Reviewed at <a href="http://llvm-reviews.chandlerc.com/D1939" target="_blank">http://llvm-reviews.chandlerc.com/D1939</a><br>
<br>
Modified:<br>
cfe/trunk/lib/AST/VTableBuilder.cpp<br>
cfe/trunk/lib/CodeGen/CGClass.cpp<br>
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp<br>
cfe/trunk/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp<br>
cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp<br>
cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance.cpp<br>
cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp<br></blockquote><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=192822&r1=192821&r2=192822&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=192822&r1=192821&r2=192822&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Wed Oct 16 13:24:06 2013<br>
@@ -563,21 +563,65 @@ llvm::Value *MicrosoftCXXABI::adjustThis<br>
CodeGenFunction &CGF, GlobalDecl GD, llvm::Value *This) {<br>
GD = GD.getCanonicalDecl();<br>
const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());<br>
- if (isa<CXXDestructorDecl>(MD))<br>
- return This;<br>
+ // FIXME: consider splitting the vdtor vs regular method code into two<br>
+ // functions.<br>
<br>
+ GlobalDecl LookupGD = GD;<br>
+ if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {<br>
+ // Complete dtors take a pointer to the complete object,<br>
+ // thus don't need adjustment.<br>
+ if (GD.getDtorType() == Dtor_Complete)<br>
+ return This;<br>
+<br>
+ // There's only Dtor_Deleting in vftable but it shares the this adjustment<br>
+ // with the base one, so look up the deleting one instead.<br>
+ LookupGD = GlobalDecl(DD, Dtor_Deleting);<br>
+ }<br>
MicrosoftVFTableContext::MethodVFTableLocation ML =<br>
- CGM.getVFTableContext().getMethodVFTableLocation(GD);<br>
+ CGM.getVFTableContext().getMethodVFTableLocation(LookupGD);<br>
<br>
unsigned AS = cast<llvm::PointerType>(This->getType())->getAddressSpace();<br>
llvm::Type *charPtrTy = CGF.Int8Ty->getPointerTo(AS);<br>
+ CharUnits StaticOffset = ML.VFTableOffset;<br>
if (ML.VBase) {<br>
- This = CGF.Builder.CreateBitCast(This, charPtrTy);<br>
- llvm::Value *VBaseOffset = CGM.getCXXABI()<br>
- .GetVirtualBaseClassOffset(CGF, This, MD->getParent(), ML.VBase);<br>
- This = CGF.Builder.CreateInBoundsGEP(This, VBaseOffset);<br>
+ bool AvoidVirtualOffset = false;<br>
+ if (isa<CXXDestructorDecl>(MD) && GD.getDtorType() == Dtor_Base) {<br>
+ // A base destructor can only be called from a complete destructor of the<br>
+ // same record type or another destructor of a more derived type.<br>
+ const CXXRecordDecl *CurRD =<br>
+ cast<CXXDestructorDecl>(CGF.CurGD.getDecl())->getParent();<br></blockquote><div><br></div><div>This cast is failing. I'm trying to reduce it. I believe there are ways to call the base dtor for something other than the complete dtor. The hack I added that makes us call the base dtor instead of complete dtor, for example.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+ if (MD->getParent() == CurRD) {<br>
+ assert(CGF.CurGD.getDtorType() == Dtor_Complete);<br>
+ // We're calling the main base dtor from a complete dtor, so we know the<br>
+ // "this" offset statically.<br>
+ AvoidVirtualOffset = true;<br>
+ } else {<br>
+ // Let's see if we try to call a destructor of a non-virtual base.<br>
+ for (CXXRecordDecl::base_class_const_iterator I = CurRD->bases_begin(),<br>
+ E = CurRD->bases_end(); I != E; ++I) {<br>
+ if (I->getType()->getAsCXXRecordDecl() != MD->getParent())<br>
+ continue;<br>
+ // If we call a base destructor for a non-virtual base, we statically<br>
+ // know where it expects the vfptr and "this" to be.<br>
+ AvoidVirtualOffset = true;<br>
+ break;<br>
+ }<br>
+ }<br>
+ }<br>
+<br>
+ if (AvoidVirtualOffset) {<br>
+ const ASTRecordLayout &Layout =<br>
+ CGF.getContext().getASTRecordLayout(MD->getParent());<br>
+ // This reflects the logic from VFTableBuilder::ComputeThisOffset().<br>
+ StaticOffset += Layout.getVBaseClassOffset(ML.VBase);<br>
+ } else {<br>
+ This = CGF.Builder.CreateBitCast(This, charPtrTy);<br>
+ llvm::Value *VBaseOffset = CGM.getCXXABI()<br>
+ .GetVirtualBaseClassOffset(CGF, This, MD->getParent(), ML.VBase);<br>
+ This = CGF.Builder.CreateInBoundsGEP(This, VBaseOffset);<br>
+ }<br>
}<br>
- CharUnits StaticOffset = ML.VFTableOffset;<br>
if (!StaticOffset.isZero()) {<br>
assert(StaticOffset.isPositive());<br>
This = CGF.Builder.CreateBitCast(This, charPtrTy);<br>
@@ -625,8 +669,18 @@ llvm::Value *MicrosoftCXXABI::adjustThis<br>
CodeGenFunction &CGF, GlobalDecl GD, llvm::Value *This) {<br>
GD = GD.getCanonicalDecl();<br>
const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());<br>
- if (isa<CXXDestructorDecl>(MD))<br>
- return This;<br>
+<br>
+ GlobalDecl LookupGD = GD;<br>
+ if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {<br>
+ // Complete destructors take a pointer to the complete object as a<br>
+ // parameter, thus don't need this adjustment.<br>
+ if (GD.getDtorType() == Dtor_Complete)<br>
+ return This;<br>
+<br>
+ // There's no Dtor_Base in vftable but it shares the this adjustment with<br>
+ // the deleting one, so look it up instead.<br>
+ LookupGD = GlobalDecl(DD, Dtor_Deleting);<br>
+ }<br></blockquote></div></div></div>