https://github.com/bcardosolopes updated https://github.com/llvm/llvm-project/pull/216210
>From 8f26a6dcaeee483933a560d2930b2cd053e636ac Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes <[email protected]> Date: Wed, 12 Aug 2026 23:50:55 -0700 Subject: [PATCH 1/2] [CIR] Destroy a static local's extended temporary in its cir.local_init A temporary lifetime-extended by a function-local static had its destructor hung off the cir.global, which the verifier rejects: a static local is initialized and destroyed in-function under its guard, via cir.local_init. Thread the region emitCXXSpecialVarDeclInit already picks down to pushTemporaryCleanup, which registers into it in reverse construction order. Fixes yaml-cpp's IsValidPlainScalar (736.ocio_r, 772/872.marian). --- clang/lib/CIR/CodeGen/CIRGenCXX.cpp | 5 ++ clang/lib/CIR/CodeGen/CIRGenExpr.cpp | 31 +++---- clang/lib/CIR/CodeGen/CIRGenFunction.h | 8 ++ .../CIR/CodeGen/local-static-temp-dtor.cpp | 84 +++++++++++++++++++ 4 files changed, 113 insertions(+), 15 deletions(-) create mode 100644 clang/test/CIR/CodeGen/local-static-temp-dtor.cpp diff --git a/clang/lib/CIR/CodeGen/CIRGenCXX.cpp b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp index 812ecc1016dcc..db5aeb802d269 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCXX.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp @@ -247,6 +247,11 @@ void CIRGenModule::emitCXXSpecialVarDeclInit(const VarDecl *varDecl, assert(curCGF && "Special var init only available inside of a function"); CIRGenFunction &cgf = *curCGF; + // A temporary whose lifetime this initializer extends is destroyed alongside + // the variable itself, so pushTemporaryCleanup needs to know where that is. + llvm::SaveAndRestore<mlir::Region *> savedDtorRegion( + cgf.curStaticVarDtorRegion, &dtorRegion); + // TODO: handle address space // The address space of a static local variable (addr) may be different // from the address space of the "this" argument of the constructor. In that diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp index eef1cda1c90f9..e854276880e96 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp @@ -1939,27 +1939,30 @@ static void pushTemporaryCleanup(CIRGenFunction &cgf, return; // Classic codegen calls registerGlobalDtor here, passing either the - // destructor or a generated array-destroy helper. CIR handles globals with - // non-trivial destructors by attaching a dtor region to the cir.global op. + // destructor or a generated array-destroy helper. CIR instead emits the + // destruction into the dtor region of whatever destroys the variable that + // extended the temporary: the cir.global's own region at namespace scope, + // and the enclosing cir.local_init's for a function-local static, which the + // verifier requires to be destroyed in-function under its guard. CIRGenModule &cgm = cgf.cgm; auto globalOp = mlir::cast<cir::GlobalOp>(cgm.getAddrOfGlobalTemporary(m, e)); - // The destruction of the reference temporary is done in the dtor - // region of the global object it is associated with. - const auto *extendingDecl = cast<VarDecl>(m->getExtendingDecl()); - cir::GlobalOp extendingGlobalOp = cgm.getOrCreateCIRGlobal( - extendingDecl, /*ty=*/nullptr, NotForDefinition); + mlir::Region *dtorRegion = cgf.curStaticVarDtorRegion; + assert(dtorRegion && "temporary extended outside a static initializer"); CIRGenBuilderTy &builder = cgm.getBuilder(); mlir::OpBuilder::InsertionGuard guard(builder); - assert(extendingGlobalOp.getDtorRegion().empty() && - "extending global already has a dtor region"); - mlir::Block *block = - builder.createBlock(&extendingGlobalOp.getDtorRegion()); - builder.setInsertionPointToStart(block); - mlir::Location loc = cgm.getLoc(m->getSourceRange()); + + // Temporaries are destroyed in reverse order of construction, so each one + // goes in front of those registered before it. + if (dtorRegion->empty()) { + builder.setInsertionPointToStart(builder.createBlock(dtorRegion)); + cir::YieldOp::create(builder, loc); + } + builder.setInsertionPointToStart(&dtorRegion->front()); + mlir::Value tempAddr = builder.createGetGlobal(globalOp); if (e->getType()->isArrayType()) { @@ -1974,8 +1977,6 @@ static void pushTemporaryCleanup(CIRGenFunction &cgf, cir::FuncOp dtorFn = cgm.getAddrAndTypeOfCXXStructor(gd).second; builder.createCallOp(loc, dtorFn, mlir::ValueRange{tempAddr}); } - - cir::YieldOp::create(builder, loc); break; } diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index 9f8454309f13a..3913836d9c984 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h @@ -237,6 +237,14 @@ class CIRGenFunction : public CIRGenTypeCache { /// global initializers. mlir::Operation *curFn = nullptr; + /// While the initializer of a variable with static storage duration is being + /// emitted, the region that destructors registered by that initializer belong + /// in: the cir.global's own dtor region for a namespace-scope variable, and + /// the enclosing cir.local_init's for a function-local static, which has to + /// be destroyed in-function under its guard. Null outside such an + /// initializer. + mlir::Region *curStaticVarDtorRegion = nullptr; + /// Save Parameter Decl for coroutine. llvm::SmallVector<const ParmVarDecl *> fnArgs; diff --git a/clang/test/CIR/CodeGen/local-static-temp-dtor.cpp b/clang/test/CIR/CodeGen/local-static-temp-dtor.cpp new file mode 100644 index 0000000000000..8e06adef0f312 --- /dev/null +++ b/clang/test/CIR/CodeGen/local-static-temp-dtor.cpp @@ -0,0 +1,84 @@ +// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir -mmlir --mlir-print-ir-before=cir-lowering-prepare %s -o %t.cir 2> %t-before.cir +// RUN: FileCheck --input-file=%t-before.cir %s --check-prefix=CIR-BEFORE +// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll +// RUN: FileCheck --input-file=%t-cir.ll %s --check-prefixes=LLVM,LLVMCIR +// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll +// RUN: FileCheck --input-file=%t.ll %s --check-prefixes=LLVM,OGCG + +// The same lifetime-extended reference temporaries as global-temp-dtor.cpp, but +// extended by a function-local static. Those are initialized and destroyed +// in-function under their guard variable, so the temporary's destructor belongs +// in the cir.local_init's dtor region rather than on the cir.global. + +struct NonTrivial { + NonTrivial(); + ~NonTrivial(); + int x; +}; + +typedef NonTrivial NonTrivialArr[2]; + +void use(const NonTrivial &); + +void ref() { + static const NonTrivial &r = NonTrivial(); + use(r); +} + +// CIR-BEFORE-LABEL: cir.func {{.*}}@_Z3refv +// CIR-BEFORE: cir.local_init static_local @_ZZ3refvE1r ctor { +// CIR-BEFORE: %[[VAR:.*]] = cir.get_global static_local @_ZZ3refvE1r +// CIR-BEFORE: %[[TEMP:.*]] = cir.get_global @_ZGRZ3refvE1r_ +// CIR-BEFORE: cir.call @_ZN10NonTrivialC1Ev(%[[TEMP]]) +// CIR-BEFORE: cir.store{{.*}} %[[TEMP]], %[[VAR]] +// CIR-BEFORE: } dtor { +// CIR-BEFORE: %[[TEMP_DTOR:.*]] = cir.get_global @_ZGRZ3refvE1r_ +// CIR-BEFORE: cir.call @_ZN10NonTrivialD1Ev(%[[TEMP_DTOR]]) +// CIR-BEFORE: } + +// LLVM-DAG: @_ZGRZ3refvE1r_ = internal global %struct.NonTrivial zeroinitializer +// LLVM-DAG: @_ZGRZ3arrvE1a_ = internal global [2 x %struct.NonTrivial] zeroinitializer + +// LLVM-LABEL: define dso_local void @_Z3refv() +// LLVM: call i32 @__cxa_guard_acquire(ptr @_ZGVZ3refvE1r) +// LLVMCIR: call void @_ZN10NonTrivialC1Ev(ptr {{.*}} @_ZGRZ3refvE1r_) +// LLVMCIR: store ptr @_ZGRZ3refvE1r_, ptr @_ZZ3refvE1r +// LLVM: call i32 @__cxa_atexit(ptr @_ZN10NonTrivialD1Ev, ptr @_ZGRZ3refvE1r_, ptr @__dso_handle) +// OGCG: store ptr @_ZGRZ3refvE1r_, ptr @_ZZ3refvE1r +// LLVM: call void @__cxa_guard_release(ptr @_ZGVZ3refvE1r) + +void arr() { + static const NonTrivialArr &a = NonTrivialArr{}; + use(a[0]); +} + +// CIR-BEFORE-LABEL: cir.func {{.*}}@_Z3arrv +// CIR-BEFORE: cir.local_init static_local @_ZZ3arrvE1a ctor { +// CIR-BEFORE: } dtor { +// CIR-BEFORE: %[[ARR_TEMP:.*]] = cir.get_global @_ZGRZ3arrvE1a_ +// CIR-BEFORE: cir.array.dtor %[[ARR_TEMP]] : !cir.ptr<!cir.array<!rec_NonTrivial x 2>> { +// CIR-BEFORE: ^bb0(%[[ELEMENT:.*]]: !cir.ptr<!rec_NonTrivial>): +// CIR-BEFORE: cir.call @_ZN10NonTrivialD1Ev(%[[ELEMENT]]) +// CIR-BEFORE: } +// CIR-BEFORE: } + +// The array destroyer is hoisted into a helper either way, and the two arms +// differ in what they hand it: classic codegen bakes the array's address into +// the helper and ignores the incoming parameter, so it registers a null +// __cxa_atexit argument, while CIR's helper destroys whatever it is given and +// so is registered with the address. Both are correct today, and this is not +// specific to a static local -- CIR emits a namespace-scope array temporary +// the same way. +// +// TODO(cir): we want the classic shape here. Besides being the output these +// tests are written against, it settles a duplication CIR currently pays for +// and does not use: its helper is parameterized, so it is the same function +// for every array of the same element type and count, yet one is still emitted +// per global (@__cxx_global_array_dtor, @__cxx_global_array_dtor.1, ...). +// Closing over the array instead would let these two checks become one. + +// LLVM-LABEL: define dso_local void @_Z3arrv() +// LLVM: call i32 @__cxa_guard_acquire(ptr @_ZGVZ3arrvE1a) +// LLVMCIR: call i32 @__cxa_atexit(ptr @__cxx_global_array_dtor, ptr @_ZGRZ3arrvE1a_, ptr @__dso_handle) +// OGCG: call i32 @__cxa_atexit(ptr @__cxx_global_array_dtor, ptr null, ptr @__dso_handle) +// LLVM: call void @__cxa_guard_release(ptr @_ZGVZ3arrvE1a) >From fae5d2cee6b853685b4ab045bd6ba842db73d2ba Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes <[email protected]> Date: Mon, 17 Aug 2026 12:00:30 -0700 Subject: [PATCH 2/2] [CIR] Call __cxa_guard_abort when a static local's initializer throws A block-scope static whose initializer exits by throwing is not initialized, and the next entry has to run the initializer again ([stmt.dcl]p4). The Itanium ABI hands the guard back with __cxa_guard_abort on the unwind path; CIR never emitted it, so the guard stayed held and the retry died with __gnu_cxx::recursive_init_error instead of re-running the initializer. Wrap the guarded initializer in an EH-only cir.cleanup.scope holding the abort call, at the two points LoweringPrepare already marked for it. The scope is structural, unlike OG's cleanup stack, so it is only built when exceptions are enabled -- otherwise it would manufacture an unwind path that -fno-exceptions does not have. One difference from OG remains inside that scope: the __cxa_atexit registration becomes an invoke where OG emits a plain nounwind call, because LoweringPrepare's buildRuntimeFunction cannot mark a runtime declaration nothrow. It reuses the landing pad the constructor already needs, so it costs nothing and is correct; marking those declarations is left to a separate change, since it touches call sites well outside this one. The abort call itself sets nothrow on the call op to stay out of an invoke here. --- clang/include/clang/CIR/MissingFeatures.h | 1 - .../Dialect/Transforms/LoweringPrepare.cpp | 52 ++++++++++++++--- .../CIR/CodeGen/static-local-guard-abort.cpp | 56 +++++++++++++++++++ 3 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 clang/test/CIR/CodeGen/static-local-guard-abort.cpp diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h index 02475b70c5dcd..18e0f35a05374 100644 --- a/clang/include/clang/CIR/MissingFeatures.h +++ b/clang/include/clang/CIR/MissingFeatures.h @@ -247,7 +247,6 @@ struct MissingFeatures { static bool generateDebugInfo() { return false; } static bool getRuntimeFunctionDecl() { return false; } static bool globalViewIntLowering() { return false; } - static bool guardAbortOnException() { return false; } static bool handleBuiltinICEArguments() { return false; } static bool hip() { return false; } static bool incrementProfileCounter() { return false; } diff --git a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp index 12d544fecbcc9..37ac1593c0901 100644 --- a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp +++ b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp @@ -189,6 +189,9 @@ struct LoweringPreparePass /// Get or create __cxa_guard_release function. cir::FuncOp getGuardReleaseFn(cir::PointerType guardPtrTy); + /// Get or create __cxa_guard_abort function. + cir::FuncOp getGuardAbortFn(cir::PointerType guardPtrTy); + /// Get or create the __init_tls function. cir::FuncOp getTlsInitFn(); @@ -466,15 +469,37 @@ struct LoweringPreparePass mlir::OpBuilder::InsertionGuard insertGuard(builder); builder.setInsertionPointToStart(&ifOp.getThenRegion().front()); - // Call __cxa_guard_abort along the exceptional edge. + // An exception out of the initializer leaves the variable uninitialized + // and the next entry has to run the initializer again ([stmt.dcl]p4), so + // the guard has to be given back along the exceptional edge. Without + // this the guard stays held and that retry terminates instead. + // + // OG pushes an EH cleanup here and pops it after the initializer, which + // costs nothing when there is no unwind path. A cir.cleanup.scope is + // structural, so it is only worth building when there can be one. // OG: CGF.EHStack.pushCleanup<CallGuardAbort>(EHCleanup, guard); - assert(!cir::MissingFeatures::guardAbortOnException()); - - emitBody(); - - // Pop the guard-abort cleanup if we pushed one. - // OG: CGF.PopCleanupBlock(); - assert(!cir::MissingFeatures::guardAbortOnException()); + // ... CGF.PopCleanupBlock(); + if (astCtx->getLangOpts().Exceptions) { + cir::CleanupScopeOp::create( + builder, loc, cir::CleanupKind::EH, + [&](mlir::OpBuilder &, mlir::Location bodyLoc) { + emitBody(); + builder.createYield(bodyLoc); + }, + [&](mlir::OpBuilder &, mlir::Location cleanupLoc) { + cir::CallOp abortCall = + builder.createCallOp(cleanupLoc, getGuardAbortFn(guardPtrTy), + mlir::ValueRange{guardPtr}); + // __cxa_guard_abort is declared noexcept, and marking it so keeps + // this out of an invoke: a cleanup that can throw while unwinding + // needs a terminate edge, which is dead weight here. + abortCall.setNothrowAttr(builder.getUnitAttr()); + builder.createYield(cleanupLoc); + }); + builder.setInsertionPointToEnd(&ifOp.getThenRegion().front()); + } else { + emitBody(); + } // Call __cxa_guard_release. This cannot throw. builder.createCallOp(loc, getGuardReleaseFn(guardPtrTy), @@ -1270,6 +1295,17 @@ LoweringPreparePass::getGuardReleaseFn(cir::PointerType guardPtrTy) { return buildRuntimeFunction(builder, "__cxa_guard_release", loc, fnType); } +cir::FuncOp LoweringPreparePass::getGuardAbortFn(cir::PointerType guardPtrTy) { + // void __cxa_guard_abort(__guard *guard_object); + CIRBaseBuilderTy builder(getContext()); + mlir::OpBuilder::InsertionGuard ipGuard{builder}; + builder.setInsertionPointToStart(mlirModule.getBody()); + mlir::Location loc = mlirModule.getLoc(); + cir::VoidType voidTy = cir::VoidType::get(&getContext()); + auto fnType = cir::FuncType::get({guardPtrTy}, voidTy); + return buildRuntimeFunction(builder, "__cxa_guard_abort", loc, fnType); +} + cir::FuncOp LoweringPreparePass::getTlsInitFn() { // void __tls_init(void); CIRBaseBuilderTy builder(getContext()); diff --git a/clang/test/CIR/CodeGen/static-local-guard-abort.cpp b/clang/test/CIR/CodeGen/static-local-guard-abort.cpp new file mode 100644 index 0000000000000..c4c8f41d3b6f3 --- /dev/null +++ b/clang/test/CIR/CodeGen/static-local-guard-abort.cpp @@ -0,0 +1,56 @@ +// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fcxx-exceptions -fexceptions -fclangir -emit-cir %s -o - | FileCheck %s --check-prefix=CIR +// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fcxx-exceptions -fexceptions -fclangir -emit-llvm %s -o - | FileCheck %s --check-prefix=LLVM +// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fcxx-exceptions -fexceptions -emit-llvm %s -o - | FileCheck %s --check-prefix=LLVM +// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o - | FileCheck %s --check-prefix=NOEXC +// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fcxx-exceptions -fexceptions -fno-threadsafe-statics -fclangir -emit-llvm %s -o - | FileCheck %s --check-prefix=NOTSS + +// An exception out of a block-scope static's initializer leaves it +// uninitialized and the next entry has to run the initializer again +// ([stmt.dcl]p4), which the Itanium ABI implements by handing the guard back +// with __cxa_guard_abort on the unwind path. Without it the guard stays held +// and that retry terminates. + +struct S { + S(); + ~S(); +}; + +void f() { static S s; } + +// The initializer -- and the __cxa_atexit registration that follows it -- sits +// in the body of an EH-only cleanup scope holding the abort call. + +// CIR-LABEL: cir.func {{.*}}@_Z1fv +// CIR: %[[GUARD:.*]] = cir.get_global @_ZGVZ1fvE1s +// CIR: cir.call @__cxa_guard_acquire(%[[GUARD]]) +// CIR: cir.cleanup.scope { +// CIR: cir.call @_ZN1SC1Ev +// CIR: cir.call @__cxa_atexit +// CIR: } cleanup eh { +// CIR: cir.call @__cxa_guard_abort(%[[GUARD]]) nothrow +// CIR: } +// CIR: cir.call @__cxa_guard_release(%[[GUARD]]) + +// LLVM-LABEL: define {{.*}}void @_Z1fv() +// LLVM: call i32 @__cxa_guard_acquire(ptr @_ZGVZ1fvE1s) +// LLVM: invoke void @_ZN1SC1Ev +// LLVM: landingpad { ptr, i32 } +// LLVM: cleanup +// LLVM: call void @__cxa_guard_abort(ptr @_ZGVZ1fvE1s) +// LLVM: resume { ptr, i32 } + +// With exceptions off there is no unwind path to hand the guard back on, and +// building the cleanup scope anyway would manufacture one. + +// NOEXC-LABEL: define {{.*}}void @_Z1fv() +// NOEXC-NOT: __cxa_guard_abort +// NOEXC-NOT: landingpad +// NOEXC: ret void + +// Without thread-safe statics there is no lock to release: the guard byte is +// simply stored after the initializer completes, so an exception skips it. + +// NOTSS-LABEL: define {{.*}}void @_Z1fv() +// NOTSS-NOT: __cxa_guard_abort +// NOTSS-NOT: __cxa_guard_acquire +// NOTSS: ret void _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
