[clang] [clang][CodeGen][Coro] Fix missing EHCleanup for eagerly returned GRO (PR #206392)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Jun 28 19:59:43 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Weibo He (NewSigma)
<details>
<summary>Changes</summary>
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
---
Full diff: https://github.com/llvm/llvm-project/pull/206392.diff
3 Files Affected:
- (modified) clang/lib/CodeGen/CGCoroutine.cpp (+8-21)
- (removed) clang/test/CodeGenCoroutines/coro-cwg2935.cpp (-35)
- (modified) clang/test/CodeGenCoroutines/coro-gro.cpp (+33-16)
``````````diff
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 99b0afcedccf6..e9f79e10db7ae 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,15 @@ 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() {}
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;
}
@@ -132,15 +149,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.
``````````
</details>
https://github.com/llvm/llvm-project/pull/206392
More information about the cfe-commits
mailing list