r192822 - [-cxx-abi microsoft] Fix this argument/parameter offsets for virtual destructors in the presence of virtual bases

[Reid Kleckner]

On Wed, Oct 16, 2013 at 11:24 AM, Timur Iskhodzhanov <timurrrr at google.com>wrote:

> Author: timurrrr
> Date: Wed Oct 16 13:24:06 2013
> New Revision: 192822
>
> URL: http://llvm.org/viewvc/llvm-project?rev=192822&view=rev
> Log:
> [-cxx-abi microsoft] Fix this argument/parameter offsets for virtual
> destructors in the presence of virtual bases
>
> Reviewed at http://llvm-reviews.chandlerc.com/D1939
>
> Modified:
>     cfe/trunk/lib/AST/VTableBuilder.cpp
>     cfe/trunk/lib/CodeGen/CGClass.cpp
>     cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
>
> cfe/trunk/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
>     cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
>
> cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance.cpp
>     cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
>


> Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=192822&r1=192821&r2=192822&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
> +++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Wed Oct 16 13:24:06 2013
> @@ -563,21 +563,65 @@ llvm::Value *MicrosoftCXXABI::adjustThis
>      CodeGenFunction &CGF, GlobalDecl GD, llvm::Value *This) {
>    GD = GD.getCanonicalDecl();
>    const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
> -  if (isa<CXXDestructorDecl>(MD))
> -    return This;
> +  // FIXME: consider splitting the vdtor vs regular method code into two
> +  // functions.
>
> +  GlobalDecl LookupGD = GD;
> +  if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
> +    // Complete dtors take a pointer to the complete object,
> +    // thus don't need adjustment.
> +    if (GD.getDtorType() == Dtor_Complete)
> +      return This;
> +
> +    // There's only Dtor_Deleting in vftable but it shares the this
> adjustment
> +    // with the base one, so look up the deleting one instead.
> +    LookupGD = GlobalDecl(DD, Dtor_Deleting);
> +  }
>    MicrosoftVFTableContext::MethodVFTableLocation ML =
> -      CGM.getVFTableContext().getMethodVFTableLocation(GD);
> +      CGM.getVFTableContext().getMethodVFTableLocation(LookupGD);
>
>    unsigned AS =
> cast<llvm::PointerType>(This->getType())->getAddressSpace();
>    llvm::Type *charPtrTy = CGF.Int8Ty->getPointerTo(AS);
> +  CharUnits StaticOffset = ML.VFTableOffset;
>    if (ML.VBase) {
> -    This = CGF.Builder.CreateBitCast(This, charPtrTy);
> -    llvm::Value *VBaseOffset = CGM.getCXXABI()
> -        .GetVirtualBaseClassOffset(CGF, This, MD->getParent(), ML.VBase);
> -    This = CGF.Builder.CreateInBoundsGEP(This, VBaseOffset);
> +    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.
> +      const CXXRecordDecl *CurRD =
> +          cast<CXXDestructorDecl>(CGF.CurGD.getDecl())->getParent();
>

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.


> +
> +      if (MD->getParent() == CurRD) {
> +        assert(CGF.CurGD.getDtorType() == Dtor_Complete);
> +        // We're calling the main base dtor from a complete dtor, 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 (CXXRecordDecl::base_class_const_iterator I =
> CurRD->bases_begin(),
> +             E = CurRD->bases_end(); I != E; ++I) {
> +          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.
> +          AvoidVirtualOffset = true;
> +          break;
> +        }
> +      }
> +    }
> +
> +    if (AvoidVirtualOffset) {
> +      const ASTRecordLayout &Layout =
> +          CGF.getContext().getASTRecordLayout(MD->getParent());
> +      // This reflects the logic from VFTableBuilder::ComputeThisOffset().
> +      StaticOffset += Layout.getVBaseClassOffset(ML.VBase);
> +    } else {
> +      This = CGF.Builder.CreateBitCast(This, charPtrTy);
> +      llvm::Value *VBaseOffset = CGM.getCXXABI()
> +          .GetVirtualBaseClassOffset(CGF, This, MD->getParent(),
> ML.VBase);
> +      This = CGF.Builder.CreateInBoundsGEP(This, VBaseOffset);
> +    }
>    }
> -  CharUnits StaticOffset = ML.VFTableOffset;
>    if (!StaticOffset.isZero()) {
>      assert(StaticOffset.isPositive());
>      This = CGF.Builder.CreateBitCast(This, charPtrTy);
> @@ -625,8 +669,18 @@ llvm::Value *MicrosoftCXXABI::adjustThis
>      CodeGenFunction &CGF, GlobalDecl GD, llvm::Value *This) {
>    GD = GD.getCanonicalDecl();
>    const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
> -  if (isa<CXXDestructorDecl>(MD))
> -    return This;
> +
> +  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);
> +  }
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131016/25a25d3b/attachment.html>