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

[Reid Kleckner]

Further reduced:

struct C { virtual ~C(); };
struct A : virtual C { };
struct B : A { B(); };
void foo();
B::B() { foo(); }

During the codegen for B::B, foo could throw, in which case we have to
destroy the partially constructed object by invoking A's base dtor.

I think this can be avoided with dyn_cast for now.


On Wed, Oct 16, 2013 at 4:52 PM, Reid Kleckner <rnk at google.com> wrote:

> Well, it's not reduced, but this will assert while generating the
> constructor for basic_ostringstream:
>
> #include <sstream>
> int main() {
>   std::stringstream ss;
> }
>
>
> On Wed, Oct 16, 2013 at 4:35 PM, Reid Kleckner <rnk at google.com> wrote:
>
>> 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/8be80421/attachment.html>