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

[Reid Kleckner]

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/23ef2e3c/attachment.html>