Skip to content

release/21.x: [Clang] Fix FE crash during CGCoroutine GRO Alloca Emission (#148962) #149013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: release/21.x
Choose a base branch
from

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Jul 16, 2025

Backport c36156d

Requested by: @efriedma-quic

)

Fixes: llvm#148953

Currently when coroutine return object type is const qualified, we don't
do direct emission. The regular emission logic assumed that the auto var
emission will always result in an `AllocaInst`. However, based on my
findings, NRVO var emissions don't result in `AllocaInst`s. Therefore,
this
[assertion](https://github.com/llvm/llvm-project/blob/1a940bfff9176fb38a71b0045d7ae25df9a4ca7d/clang/lib/CodeGen/CGCoroutine.cpp#L712)
will fail.

Since the NRVOed returned object don't live on the coroutine frame, we
won't have the problem of it outliving the coroutine frame, therefore,
we can safely omit this metadata.

(cherry picked from commit c36156d)
@llvmbot
Copy link
Member Author

llvmbot commented Jul 16, 2025

@bcardosolopes What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from bcardosolopes July 16, 2025 03:35
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. coroutines C++20 coroutines labels Jul 16, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Jul 16, 2025

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-coroutines

Author: None (llvmbot)

Changes

Backport c36156d

Requested by: @efriedma-quic


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGCoroutine.cpp (+9-5)
  • (modified) clang/test/CodeGenCoroutines/coro-gro.cpp (+28-1)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index 0fc488e98aaf0..117ef3d16e21b 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -707,11 +707,15 @@ struct GetReturnObjectManager {
     Builder.CreateStore(Builder.getFalse(), GroActiveFlag);
 
     GroEmission = CGF.EmitAutoVarAlloca(*GroVarDecl);
-    auto *GroAlloca = dyn_cast_or_null<llvm::AllocaInst>(
-        GroEmission.getOriginalAllocatedAddress().getPointer());
-    assert(GroAlloca && "expected alloca to be emitted");
-    GroAlloca->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
-                           llvm::MDNode::get(CGF.CGM.getLLVMContext(), {}));
+
+    if (!GroVarDecl->isNRVOVariable()) {
+      // NRVO variables don't have allocas and won't have the same issue.
+      auto *GroAlloca = dyn_cast_or_null<llvm::AllocaInst>(
+          GroEmission.getOriginalAllocatedAddress().getPointer());
+      assert(GroAlloca && "expected alloca to be emitted");
+      GroAlloca->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
+                             llvm::MDNode::get(CGF.CGM.getLLVMContext(), {}));
+    }
 
     // Remember the top of EHStack before emitting the cleanup.
     auto old_top = CGF.EHStack.stable_begin();
diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp b/clang/test/CodeGenCoroutines/coro-gro.cpp
index b62134317cef2..037fd03349e76 100644
--- a/clang/test/CodeGenCoroutines/coro-gro.cpp
+++ b/clang/test/CodeGenCoroutines/coro-gro.cpp
@@ -106,4 +106,31 @@ invoker g() {
   // CHECK: call void @_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]]
   co_return;
 }
-// CHECK: ![[OutFrameMetadata]] = !{}
\ No newline at end of file
+
+namespace gh148953 {
+
+struct Task {
+  struct promise_type {
+    Task get_return_object();
+    std::suspend_always initial_suspend() { return {}; }
+    std::suspend_always final_suspend() noexcept { return {}; }
+    void return_void() {}
+    void unhandled_exception() {}
+  };
+  Task() {}
+  // Different from `invoker`, this Task is copy constructible.
+  Task(const Task&) {};
+};
+
+// NRVO on const qualified return type should work.
+// CHECK: define{{.*}} void @_ZN8gh1489537exampleEv({{.*}} sret(%"struct.gh148953::Task") align 1 %[[NrvoRes:.+]])
+const Task example() {
+  // CHECK: %[[ResultPtr:.+]] = alloca ptr
+  // CHECK: store ptr %[[NrvoRes]], ptr %[[ResultPtr]]
+  // CHECK: coro.init:
+  // CHECK: call void @_ZN8gh1489534Task12promise_type17get_return_objectEv({{.*}} %[[NrvoRes:.+]], {{.*}})
+  co_return;
+}
+
+} // namespace gh148953
+// CHECK: ![[OutFrameMetadata]] = !{}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category coroutines C++20 coroutines
Projects
Status: Needs Triage
Development

Successfully merging this pull request may close these issues.

2 participants