https://github.com/NewSigma created https://github.com/llvm/llvm-project/pull/206392
When the GRO is returned eagerly, its value is emitted directly to the return-value slot. However, we failed to emit EHCleanup for it, leading to resource leaks if an exception was thrown inside the ramp function. #202279 fixed this for the period before `initial-await-resume-called` is set to `true`. This patch extends that fix to cover the entire ramp function. Close #199627 >From 7cad38042a946921481a5992dd2bcbda39f1aa93 Mon Sep 17 00:00:00 2001 From: NewSigma <[email protected]> Date: Sun, 28 Jun 2026 13:35:36 +0800 Subject: [PATCH 1/2] Enable exception for coro-gro (NFC) --- clang/test/CodeGenCoroutines/coro-gro.cpp | 26 +++++++++++------------ 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp b/clang/test/CodeGenCoroutines/coro-gro.cpp index 99b0afcedccf6..827b5be50aee0 100644 --- a/clang/test/CodeGenCoroutines/coro-gro.cpp +++ b/clang/test/CodeGenCoroutines/coro-gro.cpp @@ -1,6 +1,6 @@ // Verifies lifetime of __gro local variable // Verify that coroutine promise and allocated memory are freed up on exception. -// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s +// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -fexceptions -fcxx-exceptions -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s #include "Inputs/coroutine.h" @@ -17,7 +17,7 @@ template <> struct std::coroutine_traits<int> { suspend_always initial_suspend() noexcept; suspend_always final_suspend() noexcept; void return_void() noexcept; - promise_type(); + promise_type() noexcept; ~promise_type(); void unhandled_exception() noexcept; }; @@ -97,14 +97,14 @@ class invoker { public: class invoker_promise { public: - invoker get_return_object() { return invoker{}; } - auto initial_suspend() { return suspend_always{}; } + invoker get_return_object() noexcept { return invoker{}; } + auto initial_suspend() noexcept { return suspend_always{}; } auto final_suspend() noexcept { return suspend_always{}; } - void return_void() {} - void unhandled_exception() {} + void return_void() noexcept {} + void unhandled_exception() noexcept {} }; using promise_type = invoker_promise; - invoker() {} + invoker() noexcept {} invoker(const invoker &) = delete; invoker &operator=(const invoker &) = delete; invoker(invoker &&) = delete; @@ -132,15 +132,15 @@ namespace gh148953 { struct Task { struct promise_type { - Task get_return_object(); - std::suspend_always initial_suspend() { return {}; } + Task get_return_object() noexcept; + std::suspend_always initial_suspend() noexcept { return {}; } std::suspend_always final_suspend() noexcept { return {}; } - void return_void() {} - void unhandled_exception() {} + void return_void() noexcept {} + void unhandled_exception() noexcept {} }; - Task() {} + Task() noexcept {} // Different from `invoker`, this Task is copy constructible. - Task(const Task&) {}; + Task(const Task&) noexcept {}; }; // NRVO on const qualified return type should work. >From 19275df7135e38d12994a338a1df5b27003f9b09 Mon Sep 17 00:00:00 2001 From: NewSigma <[email protected]> Date: Sun, 28 Jun 2026 13:38:47 +0800 Subject: [PATCH 2/2] [clang][CodeGen][Coro] Fix missing EHCleanup for eagerly initialized GRO --- clang/lib/CodeGen/CGCoroutine.cpp | 29 +++++---------- clang/test/CodeGenCoroutines/coro-cwg2935.cpp | 35 ------------------- clang/test/CodeGenCoroutines/coro-gro.cpp | 23 ++++++++++-- 3 files changed, 28 insertions(+), 59 deletions(-) delete mode 100644 clang/test/CodeGenCoroutines/coro-cwg2935.cpp diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp index 700a8fd4a48c1..c1ea145934143 100644 --- a/clang/lib/CodeGen/CGCoroutine.cpp +++ b/clang/lib/CodeGen/CGCoroutine.cpp @@ -64,12 +64,6 @@ struct clang::CodeGen::CGCoroData { // statements jumps to this point after calling return_xxx promise member. CodeGenFunction::JumpDest FinalJD; - // A cleanup flag for the coroutine return value object when it is initialized - // directly by the get-return-object invocation. It models the standard's - // initial-await-resume-called guard for exceptions thrown during coroutine - // startup, before the initial await_resume starts. - Address InitialReturnObjectActiveFlag = Address::invalid(); - // Stores the llvm.coro.id emitted in the function so that we can supply it // as the first argument to coro.begin, coro.alloc and coro.free intrinsics. // Note: llvm.coro.id returns a token that cannot be directly expressed in a @@ -344,10 +338,6 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co // Emit await_resume expression. CGF.EmitBlock(ReadyBlock); - if (Kind == AwaitKind::Init && Coro.InitialReturnObjectActiveFlag.isValid()) { - Builder.CreateStore(Builder.getFalse(), Coro.InitialReturnObjectActiveFlag); - Coro.InitialReturnObjectActiveFlag = Address::invalid(); - } // Exception handling requires additional IR. If the 'await_resume' function // is marked as 'noexcept', we avoid generating this additional IR. @@ -763,21 +753,18 @@ struct GetReturnObjectManager { } } - void EmitDirectReturnObjectCleanup() { + Address EmitDirectReturnObjectCleanup() { if (!DirectEmit || !CGF.ReturnValue.isValid()) - return; + return Address::invalid(); QualType RetTy = CGF.FnRetTy; QualType::DestructionKind DtorKind = RetTy.isDestructedType(); - if (DtorKind == QualType::DK_none) - return; - if (!CGF.needsEHCleanup(DtorKind)) - return; + if (DtorKind == QualType::DK_none || !CGF.needsEHCleanup(DtorKind)) + return Address::invalid(); Address ActiveFlag = CGF.CreateTempAlloca( Builder.getInt1Ty(), CharUnits::One(), "coro.result.active"); Builder.CreateStore(Builder.getFalse(), ActiveFlag); - CGF.CurCoro.Data->InitialReturnObjectActiveFlag = ActiveFlag; auto OldTop = CGF.EHStack.stable_begin(); CGF.pushDestroy(EHCleanup, CGF.ReturnValue, RetTy, @@ -793,6 +780,7 @@ struct GetReturnObjectManager { Cleanup->setTestFlagInEHCleanup(); } } + return ActiveFlag; } void EmitGroInit() { @@ -810,13 +798,12 @@ struct GetReturnObjectManager { // otherwise the call to get_return_object wouldn't be in front // of initial_suspend. if (CGF.ReturnValue.isValid()) { - EmitDirectReturnObjectCleanup(); + auto ActiveFlag = EmitDirectReturnObjectCleanup(); CGF.EmitAnyExprToMem(S.getReturnValue(), CGF.ReturnValue, S.getReturnValue()->getType().getQualifiers(), /*IsInit*/ true); - if (CGF.CurCoro.Data->InitialReturnObjectActiveFlag.isValid()) - Builder.CreateStore(Builder.getTrue(), - CGF.CurCoro.Data->InitialReturnObjectActiveFlag); + if (ActiveFlag.isValid()) + Builder.CreateStore(Builder.getTrue(), ActiveFlag); } return; } diff --git a/clang/test/CodeGenCoroutines/coro-cwg2935.cpp b/clang/test/CodeGenCoroutines/coro-cwg2935.cpp deleted file mode 100644 index ebb8158ec590c..0000000000000 --- a/clang/test/CodeGenCoroutines/coro-cwg2935.cpp +++ /dev/null @@ -1,35 +0,0 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 \ -// RUN: -fcxx-exceptions -fexceptions -emit-llvm -disable-llvm-passes \ -// RUN: %s -o - | FileCheck %s - -#include "Inputs/coroutine.h" - -struct task { - ~task(); - - struct promise_type { - task get_return_object(); - std::suspend_never initial_suspend(); - std::suspend_never final_suspend() noexcept; - void return_void(); - void unhandled_exception(); - }; -}; - -task f() { - co_return; -} - -// CHECK-LABEL: define{{.*}} void @_Z1fv( -// CHECK: %[[RESULT_ACTIVE:.+]] = alloca i1 -// CHECK: store i1 false, ptr %[[RESULT_ACTIVE]] -// CHECK: invoke void @_ZN4task12promise_type17get_return_objectEv( -// CHECK: store i1 true, ptr %[[RESULT_ACTIVE]] -// CHECK: invoke void @_ZN4task12promise_type15initial_suspendEv( -// CHECK: init.ready: -// CHECK-NEXT: store i1 false, ptr %[[RESULT_ACTIVE]] -// CHECK-NEXT: call void @_ZNSt13suspend_never12await_resumeEv( -// CHECK: %[[IS_ACTIVE:.+]] = load i1, ptr %[[RESULT_ACTIVE]] -// CHECK-NEXT: br i1 %[[IS_ACTIVE]], label %[[CLEANUP_ACTION:.+]], label %[[CLEANUP_DONE:.+]] -// CHECK: [[CLEANUP_ACTION]]: -// CHECK-NEXT: call void @_ZN4taskD1Ev( diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp b/clang/test/CodeGenCoroutines/coro-gro.cpp index 827b5be50aee0..e9f79e10db7ae 100644 --- a/clang/test/CodeGenCoroutines/coro-gro.cpp +++ b/clang/test/CodeGenCoroutines/coro-gro.cpp @@ -105,6 +105,7 @@ class invoker { }; using promise_type = invoker_promise; invoker() noexcept {} + ~invoker() {} invoker(const invoker &) = delete; invoker &operator=(const invoker &) = delete; invoker(invoker &&) = delete; @@ -117,14 +118,30 @@ class invoker { invoker g() { // CHECK: %[[ResultPtr:.+]] = alloca ptr // CHECK-NEXT: %[[Promise:.+]] = alloca %"class.invoker::invoker_promise" + // CHECK-NEXT: %[[RESULT_ACTIVE:.+]] = alloca i1 // CHECK: store ptr %[[AggRes]], ptr %[[ResultPtr]] // CHECK: coro.init: // CHECK: = call ptr @llvm.coro.begin - // delayed GRO pattern stores a GRO active flag, make sure to not emit it. - // CHECK-NOT: store i1 false, ptr - // CHECK: call void @_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]] + // CHECK: store i1 false, ptr %[[RESULT_ACTIVE]] + // CHECK-NEXT: call void @_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]] + // CHECK-NEXT: store i1 true, ptr %[[RESULT_ACTIVE]] + // CHECK-NEXT: call void @llvm.lifetime.start + // CHECK-NEXT: call void @_ZN7invoker15invoker_promise15initial_suspendEv({{.*}} %[[Promise]] + + throw 0; + // Test that GRO is destructed in EHCleanup + // CHECK: call void @llvm.coro.end(ptr null, i1 true, token none) + // CHECK-NEXT: call i1 @llvm.coro.is_in_ramp() + // CHECK-NEXT: br i1 {{.*}}, label %[[CLEANUP_CONT:.+]], label %eh.resume + + // CHECK: [[CLEANUP_CONT]] + // CHECK-NEXT: %[[IS_ACTIVE:.+]] = load i1, ptr %[[RESULT_ACTIVE]] + // CHECK-NEXT: br i1 %[[IS_ACTIVE]], label %[[CLEANUP_ACTION:.+]], label %{{.*}} + + // CHECK: [[CLEANUP_ACTION]]: + // CHECK-NEXT: call void @_ZN7invokerD1Ev({{.*}} %[[AggRes]] co_return; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
