[CIR] Call base class destructors

[andykaylor]

This adds handling for calling virtual and non-virtual base class destructors. Non-virtual base class destructors are call from the base (D2) destructor body for derived classes. Virtual base class destructors are called only from the complete (D1) destructor.

[llvmbot]

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

This adds handling for calling virtual and non-virtual base class destructors. Non-virtual base class destructors are call from the base (D2) destructor body for derived classes. Virtual base class destructors are called only from the complete (D1) destructor.

---

Full diff: https://github.com/llvm/llvm-project/pull/162562.diff

4 Files Affected:

• (modified) clang/lib/CIR/CodeGen/CIRGenCXXABI.h (+8)
• (modified) clang/lib/CIR/CodeGen/CIRGenClass.cpp (+47-4)
• (modified) clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp (+14-6)
• (modified) clang/test/CIR/CodeGen/dtors.cpp (+81)

diff --git a/clang/lib/CIR/CodeGen/CIRGenCXXABI.h b/clang/lib/CIR/CodeGen/CIRGenCXXABI.h index 2465a68b068bf..b22babdd89022 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCXXABI.h +++ b/clang/lib/CIR/CodeGen/CIRGenCXXABI.h @@ -149,6 +149,14 @@ class CIRGenCXXABI { /// Loads the incoming C++ this pointer as it was passed by the caller. mlir::Value loadIncomingCXXThis(CIRGenFunction &cgf); + /// Get the implicit (second) parameter that comes after the "this" pointer, + /// or nullptr if there is isn't one. + virtual mlir::Value getCXXDestructorImplicitParam(CIRGenFunction &cgf, + const CXXDestructorDecl *dd, + CXXDtorType type, + bool forVirtualBase, + bool delegating) = 0; + /// Emit constructor variants required by this ABI. virtual void emitCXXConstructors(const clang::CXXConstructorDecl *d) = 0; diff --git a/clang/lib/CIR/CodeGen/CIRGenClass.cpp b/clang/lib/CIR/CodeGen/CIRGenClass.cpp index d9ebf19534dc4..a26982ec92565 100644 --- a/clang/lib/CIR/CodeGen/CIRGenClass.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenClass.cpp @@ -126,6 +126,36 @@ static bool isInitializerOfDynamicClass(const CXXCtorInitializer *baseInit) { } namespace { +/// Call the destructor for a direct base class. +struct CallBaseDtor final : EHScopeStack::Cleanup { + const CXXRecordDecl *baseClass; + bool baseIsVirtual; + CallBaseDtor(const CXXRecordDecl *base, bool baseIsVirtual) + : baseClass(base), baseIsVirtual(baseIsVirtual) {} + + void emit(CIRGenFunction &cgf) override { + const CXXRecordDecl *derivedClass = + cast<CXXMethodDecl>(cgf.curFuncDecl)->getParent(); + + const CXXDestructorDecl *d = baseClass->getDestructor(); + // We are already inside a destructor, so presumably the object being + // destroyed should have the expected type. + QualType thisTy = d->getFunctionObjectParameterType(); + assert(cgf.currSrcLoc && "expected source location"); + Address addr = cgf.getAddressOfDirectBaseInCompleteClass( + *cgf.currSrcLoc, cgf.loadCXXThisAddress(), derivedClass, baseClass, + baseIsVirtual); + cgf.emitCXXDestructorCall(d, Dtor_Base, baseIsVirtual, + /*delegating=*/false, addr, thisTy); + } + + // This is a placeholder until EHCleanupScope is implemented. + size_t getSize() const override { + assert(!cir::MissingFeatures::ehCleanupScope()); + return sizeof(CallBaseDtor); + } +}; + /// A visitor which checks whether an initializer uses 'this' in a /// way which requires the vtable to be properly set. struct DynamicThisUseChecker @@ -928,8 +958,21 @@ void CIRGenFunction::enterDtorCleanups(const CXXDestructorDecl *dd, if (dtorType == Dtor_Complete) { assert(!cir::MissingFeatures::sanitizers()); - if (classDecl->getNumVBases()) - cgm.errorNYI(dd->getSourceRange(), "virtual base destructor cleanups"); + // We push them in the forward order so that they'll be popped in + // the reverse order. + for (const CXXBaseSpecifier &base : classDecl->vbases()) { + auto *baseClassDecl = base.getType()->castAsCXXRecordDecl(); + + if (baseClassDecl->hasTrivialDestructor()) { + // Under SanitizeMemoryUseAfterDtor, poison the trivial base class + // memory. For non-trival base classes the same is done in the class + // destructor. + assert(!cir::MissingFeatures::sanitizers()); + } else { + ehStack.pushCleanup<CallBaseDtor>(NormalAndEHCleanup, baseClassDecl, + /*baseIsVirtual=*/true); + } + } return; } @@ -948,8 +991,8 @@ void CIRGenFunction::enterDtorCleanups(const CXXDestructorDecl *dd, if (baseClassDecl->hasTrivialDestructor()) assert(!cir::MissingFeatures::sanitizers()); else - cgm.errorNYI(dd->getSourceRange(), - "non-trivial base destructor cleanups"); + ehStack.pushCleanup<CallBaseDtor>(NormalAndEHCleanup, baseClassDecl, + /*baseIsVirtual=*/false); } assert(!cir::MissingFeatures::sanitizers()); diff --git a/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp b/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp index 04181740ccf6e..afa509658b78f 100644 --- a/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp @@ -59,7 +59,11 @@ class CIRGenItaniumCXXABI : public CIRGenCXXABI { void addImplicitStructorParams(CIRGenFunction &cgf, QualType &resTy, FunctionArgList &params) override; - + mlir::Value getCXXDestructorImplicitParam(CIRGenFunction &cgf, + const CXXDestructorDecl *dd, + CXXDtorType type, + bool forVirtualBase, + bool delegating) override; void emitCXXConstructors(const clang::CXXConstructorDecl *d) override; void emitCXXDestructors(const clang::CXXDestructorDecl *d) override; void emitCXXStructor(clang::GlobalDecl gd) override; @@ -1492,11 +1496,8 @@ void CIRGenItaniumCXXABI::emitDestructorCall( CIRGenFunction &cgf, const CXXDestructorDecl *dd, CXXDtorType type, bool forVirtualBase, bool delegating, Address thisAddr, QualType thisTy) { GlobalDecl gd(dd, type); - if (needsVTTParameter(gd)) { - cgm.errorNYI(dd->getSourceRange(), "emitDestructorCall: VTT"); - } - - mlir::Value vtt = nullptr; + mlir::Value vtt = + getCXXDestructorImplicitParam(cgf, dd, type, forVirtualBase, delegating); ASTContext &astContext = cgm.getASTContext(); QualType vttTy = astContext.getPointerType(astContext.VoidPtrTy); assert(!cir::MissingFeatures::appleKext()); @@ -1507,6 +1508,13 @@ void CIRGenItaniumCXXABI::emitDestructorCall( vttTy, nullptr); } +mlir::Value CIRGenItaniumCXXABI::getCXXDestructorImplicitParam( + CIRGenFunction &cgf, const CXXDestructorDecl *dd, CXXDtorType type, + bool forVirtualBase, bool delegating) { + GlobalDecl gd(dd, type); + return cgf.getVTTParameter(gd, forVirtualBase, delegating); +} + // The idea here is creating a separate block for the throw with an // `UnreachableOp` as the terminator. So, we branch from the current block // to the throw block and create a block for the remaining operations. diff --git a/clang/test/CIR/CodeGen/dtors.cpp b/clang/test/CIR/CodeGen/dtors.cpp index 49952a79c2cec..7fb09757a27bf 100644 --- a/clang/test/CIR/CodeGen/dtors.cpp +++ b/clang/test/CIR/CodeGen/dtors.cpp @@ -208,3 +208,84 @@ void test_nested_dtor() { // OGCG: define {{.*}} void @_ZN1DD2Ev // OGCG: %[[C:.*]] = getelementptr inbounds i8, ptr %{{.*}}, i64 4 // OGCG: call void @_ZN1CD1Ev(ptr {{.*}} %[[C]]) + +struct E { + ~E(); +}; + +struct F : public E { + int n; + ~F() {} +}; + +// CIR: cir.func {{.*}} @_ZN1FD2Ev +// CIR: %[[BASE_E:.*]] = cir.base_class_addr %{{.*}} : !cir.ptr<!rec_F> nonnull [0] -> !cir.ptr<!rec_E> +// CIR: cir.call @_ZN1ED2Ev(%[[BASE_E]]) nothrow : (!cir.ptr<!rec_E>) -> () + +// Because E is at offset 0 in F, there is no getelementptr needed. + +// LLVM: define {{.*}} void @_ZN1FD2Ev +// LLVM: call void @_ZN1ED2Ev(ptr %{{.*}}) + +// This destructor is defined after the calling function in OGCG. + +void test_base_dtor_call() { + F f; +} + +// CIR: cir.func {{.*}} @_Z19test_base_dtor_callv() +// cir.call @_ZN1FD2Ev(%{{.*}}) nothrow : (!cir.ptr<!rec_F>) -> () + +// LLVM: define {{.*}} void @_Z19test_base_dtor_callv() +// LLVM: call void @_ZN1FD2Ev(ptr %{{.*}}) + +// OGCG: define {{.*}} void @_Z19test_base_dtor_callv() +// OGCG: call void @_ZN1FD2Ev(ptr {{.*}} %{{.*}}) + +// OGCG: define {{.*}} void @_ZN1FD2Ev +// OGCG: call void @_ZN1ED2Ev(ptr {{.*}} %{{.*}}) + +struct VirtualBase { + ~VirtualBase(); +}; + +struct Derived : virtual VirtualBase { + ~Derived() {} +}; + +void test_base_dtor_call_virtual_base() { + Derived d; +} + +// Derived D2 (base) destructor -- does not call VirtualBase destructor + +// CIR: cir.func {{.*}} @_ZN7DerivedD2Ev +// CIR-NOT: cir.call{{.*}} @_ZN11VirtualBaseD2Ev +// CIR: cir.return + +// LLVM: define {{.*}} void @_ZN7DerivedD2Ev +// LLVM-NOT: call{{.*}} @_ZN11VirtualBaseD2Ev +// LLVM: ret + +// Derived D1 (complete) destructor -- does call VirtualBase destructor + +// CIR: cir.func {{.*}} @_ZN7DerivedD1Ev +// CIR: %[[THIS:.*]] = cir.load %{{.*}} +// CIR: %[[VTT:.*]] = cir.vtt.address_point @_ZTT7Derived, offset = 0 -> !cir.ptr<!cir.ptr<!void>> +// CIR: cir.call @_ZN7DerivedD2Ev(%[[THIS]], %[[VTT]]) +// CIR: %[[VIRTUAL_BASE:.*]] = cir.base_class_addr %[[THIS]] : !cir.ptr<!rec_Derived> nonnull [0] -> !cir.ptr<!rec_VirtualBase> +// CIR: cir.call @_ZN11VirtualBaseD2Ev(%[[VIRTUAL_BASE]]) + +// LLVM: define {{.*}} void @_ZN7DerivedD1Ev +// LLVM: call void @_ZN7DerivedD2Ev(ptr %{{.*}}, ptr @_ZTT7Derived) +// LLVM: call void @_ZN11VirtualBaseD2Ev(ptr %{{.*}}) + +// OGCG emits these destructors in reverse order + +// OGCG: define {{.*}} void @_ZN7DerivedD1Ev +// OGCG: call void @_ZN7DerivedD2Ev(ptr {{.*}} %{{.*}}, ptr {{.*}} @_ZTT7Derived) +// OGCG: call void @_ZN11VirtualBaseD2Ev(ptr {{.*}} %{{.*}}) + +// OGCG: define {{.*}} void @_ZN7DerivedD2Ev +// OGCG-NOT: call{{.*}} @_ZN11VirtualBaseD2Ev +// OGCG: ret 

[bcardosolopes]

Nice, LGTM