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

[Timur Iskhodzhanov]

Thanks for a great test case!
Should be fixed by r192875.

2013/10/17 Reid Kleckner <rnk at google.com>:
> 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);
>>>> +  }
>>
>>
>