https://github.com/bcardosolopes updated 
https://github.com/llvm/llvm-project/pull/66706

>From 6312dd56ed3a2f47e7291ae32ca044622a317259 Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.card...@gmail.com>
Date: Wed, 20 Sep 2023 15:00:06 -0700
Subject: [PATCH 1/2] [Clang][LLVM][Coroutines] Prevent __coro_gro from
 outliving __promise

When dealing with short-circuiting coroutines (e.g. expected), the deferred
calls that resolve the get_return_object are currently being emitted after we
delete the coroutine frame.

This was caught by ASAN when using optimizations -O1 and above: optimizations
after inlining would place the __coro_gro in the heap, and subsequent delete of
the coroframe followed by the conversion -> BOOM.

This patch forbids the GRO to be placed in the coroutine frame, by adding a new
metadata node that can be attached to `alloca` instructions.

This fixes #49843.
---
 clang/lib/CodeGen/CGCoroutine.cpp            |  5 +++++
 clang/test/CodeGenCoroutines/coro-gro.cpp    |  5 ++++-
 llvm/docs/LangRef.rst                        | 16 ++++++++++++++++
 llvm/include/llvm/IR/FixedMetadataKinds.def  |  1 +
 llvm/lib/Transforms/Coroutines/CoroFrame.cpp |  5 +++++
 5 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 448943084acedf3..58310216ecff1f5 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -535,6 +535,11 @@ 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(), {}));
 
     // 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 b48b769950ae871..ba872e39f4e3de8 100644
--- a/clang/test/CodeGenCoroutines/coro-gro.cpp
+++ b/clang/test/CodeGenCoroutines/coro-gro.cpp
@@ -30,12 +30,13 @@ void doSomething() noexcept;
 int f() {
   // CHECK: %[[RetVal:.+]] = alloca i32
   // CHECK: %[[GroActive:.+]] = alloca i1
+  // CHECK: %[[CoroGro:.+]] = alloca %struct.GroType, {{.*}} 
!coro.outside.frame ![[OutFrameMetadata:.+]]
 
   // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64()
   // CHECK: call noalias noundef nonnull ptr @_Znwm(i64 noundef %[[Size]])
   // CHECK: store i1 false, ptr %[[GroActive]]
   // CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_typeC1Ev(
-  // CHECK: call void 
@_ZNSt16coroutine_traitsIiJEE12promise_type17get_return_objectEv(
+  // CHECK: call void 
@_ZNSt16coroutine_traitsIiJEE12promise_type17get_return_objectEv({{.*}} 
%[[CoroGro]]
   // CHECK: store i1 true, ptr %[[GroActive]]
 
   Cleanup cleanup;
@@ -104,3 +105,5 @@ invoker g() {
   // CHECK: call void 
@_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]]
   co_return;
 }
+
+// CHECK: ![[OutFrameMetadata]] = !{}
\ No newline at end of file
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index f542e70bcfee810..9e3f8962ce1b52e 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -6691,6 +6691,22 @@ sections that the user does not want removed after 
linking.
   ...
   !0 = !{}
 
+'``coro.outside.frame``' Metadata
+^^^^^^^^^^^^^^^^^^^^^^
+
+``coro.outside.frame`` metadata may be attached to an alloca instruction to
+to signify that it shouldn't be promoted to the coroutine frame, useful for
+filtering allocas out by the frontend when emitting internal control 
mechanisms.
+Additionally, this metadata is only used as a flag, so the associated
+node must be empty.
+
+.. code-block:: text
+
+  %__coro_gro = alloca %struct.GroType, align 1, !coro.outside.frame !0
+
+  ...
+  !0 = !{}
+
 '``unpredictable``' Metadata
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/llvm/include/llvm/IR/FixedMetadataKinds.def 
b/llvm/include/llvm/IR/FixedMetadataKinds.def
index 8723bf2a0680c77..b375d0f0912060f 100644
--- a/llvm/include/llvm/IR/FixedMetadataKinds.def
+++ b/llvm/include/llvm/IR/FixedMetadataKinds.def
@@ -50,3 +50,4 @@ LLVM_FIXED_MD_KIND(MD_callsite, "callsite", 35)
 LLVM_FIXED_MD_KIND(MD_kcfi_type, "kcfi_type", 36)
 LLVM_FIXED_MD_KIND(MD_pcsections, "pcsections", 37)
 LLVM_FIXED_MD_KIND(MD_DIAssignID, "DIAssignID", 38)
+LLVM_FIXED_MD_KIND(MD_coro_outside_frame, "coro.outside.frame", 39)
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp 
b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index a12dd710174f3f8..9c1ee322ce0b177 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -2804,6 +2804,11 @@ static void collectFrameAlloca(AllocaInst *AI, 
coro::Shape &Shape,
   if (AI == Shape.SwitchLowering.PromiseAlloca)
     return;
 
+  // The __coro_gro alloca should outlive the promise, make sure we
+  // keep it outside the frame.
+  if (MDNode *MD = AI->getMetadata(LLVMContext::MD_coro_outside_frame))
+    return;
+
   // The code that uses lifetime.start intrinsic does not work for functions
   // with loops without exit. Disable it on ABIs we know to generate such
   // code.

>From 80a1dae0c5dce57ff5a62c59519105c914176345 Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.card...@gmail.com>
Date: Thu, 21 Sep 2023 11:41:31 -0700
Subject: [PATCH 2/2] [Clang][Coroutines] Add one more testcase and fix nits

---
 clang/test/CodeGenCoroutines/coro-gro.cpp     |  1 -
 llvm/docs/Coroutines.rst                      | 19 ++++++
 llvm/docs/LangRef.rst                         | 16 -----
 .../Coroutines/coro-alloca-outside-frame.ll   | 61 +++++++++++++++++++
 4 files changed, 80 insertions(+), 17 deletions(-)
 create mode 100644 llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll

diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp 
b/clang/test/CodeGenCoroutines/coro-gro.cpp
index ba872e39f4e3de8..d4c3ff589e340a8 100644
--- a/clang/test/CodeGenCoroutines/coro-gro.cpp
+++ b/clang/test/CodeGenCoroutines/coro-gro.cpp
@@ -105,5 +105,4 @@ invoker g() {
   // CHECK: call void 
@_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]]
   co_return;
 }
-
 // CHECK: ![[OutFrameMetadata]] = !{}
\ No newline at end of file
diff --git a/llvm/docs/Coroutines.rst b/llvm/docs/Coroutines.rst
index eed160f426d4517..b1d7a5017e612b0 100644
--- a/llvm/docs/Coroutines.rst
+++ b/llvm/docs/Coroutines.rst
@@ -1775,6 +1775,25 @@ CoroCleanup
 This pass runs late to lower all coroutine related intrinsics not replaced by
 earlier passes.
 
+Metadata
+========
+
+'``coro.outside.frame``' Metadata
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+``coro.outside.frame`` metadata may be attached to an alloca instruction to
+to signify that it shouldn't be promoted to the coroutine frame, useful for
+filtering allocas out by the frontend when emitting internal control 
mechanisms.
+Additionally, this metadata is only used as a flag, so the associated
+node must be empty.
+
+.. code-block:: text
+
+  %__coro_gro = alloca %struct.GroType, align 1, !coro.outside.frame !0
+
+  ...
+  !0 = !{}
+
 Areas Requiring Attention
 =========================
 #. When coro.suspend returns -1, the coroutine is suspended, and it's possible
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 9e3f8962ce1b52e..f542e70bcfee810 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -6691,22 +6691,6 @@ sections that the user does not want removed after 
linking.
   ...
   !0 = !{}
 
-'``coro.outside.frame``' Metadata
-^^^^^^^^^^^^^^^^^^^^^^
-
-``coro.outside.frame`` metadata may be attached to an alloca instruction to
-to signify that it shouldn't be promoted to the coroutine frame, useful for
-filtering allocas out by the frontend when emitting internal control 
mechanisms.
-Additionally, this metadata is only used as a flag, so the associated
-node must be empty.
-
-.. code-block:: text
-
-  %__coro_gro = alloca %struct.GroType, align 1, !coro.outside.frame !0
-
-  ...
-  !0 = !{}
-
 '``unpredictable``' Metadata
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll 
b/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
new file mode 100644
index 000000000000000..ac6a5752438cedb
--- /dev/null
+++ b/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
@@ -0,0 +1,61 @@
+; Tests that CoroSplit can succesfully skip allocas that shall not live on the 
frame
+; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S -o %t.ll
+; RUN: FileCheck --input-file=%t.ll %s
+
+define ptr @f(i1 %n) presplitcoroutine {
+entry:
+  %x = alloca i64, !coro.outside.frame !{}
+  %y = alloca i64
+  %id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
+  %size = call i32 @llvm.coro.size.i32()
+  %alloc = call ptr @malloc(i32 %size)
+  %hdl = call ptr @llvm.coro.begin(token %id, ptr %alloc)
+  br i1 %n, label %flag_true, label %flag_false
+
+flag_true:
+  br label %merge
+
+flag_false:
+  br label %merge
+
+merge:
+  %alias_phi = phi ptr [ %x, %flag_true ], [ %y, %flag_false ]
+  %sp1 = call i8 @llvm.coro.suspend(token none, i1 false)
+  switch i8 %sp1, label %suspend [i8 0, label %resume
+                                  i8 1, label %cleanup]
+resume:
+  call void @print(ptr %alias_phi)
+  br label %cleanup
+
+cleanup:
+  %mem = call ptr @llvm.coro.free(token %id, ptr %hdl)
+  call void @free(ptr %mem)
+  br label %suspend
+
+suspend:
+  call i1 @llvm.coro.end(ptr %hdl, i1 0, token none)
+  ret ptr %hdl
+}
+
+; %y and %alias_phi would all go to the frame, but not %x
+; CHECK:       %f.Frame = type { ptr, ptr, i64, ptr, i1 }
+; CHECK-LABEL: @f(
+; CHECK:         %x = alloca i64, align 8, !coro.outside.frame !0
+; CHECK-NOT:     %x.reload.addr = getelementptr inbounds %f.Frame, ptr %hdl, 
i32 0, i32 2
+; CHECK:         %y.reload.addr = getelementptr inbounds %f.Frame, ptr %hdl, 
i32 0, i32 2
+; CHECK:         %alias_phi = phi ptr [ %y.reload.addr, %merge.from.flag_false 
], [ %x, %entry ]
+
+declare ptr @llvm.coro.free(token, ptr)
+declare i32 @llvm.coro.size.i32()
+declare i8  @llvm.coro.suspend(token, i1)
+declare void @llvm.coro.resume(ptr)
+declare void @llvm.coro.destroy(ptr)
+
+declare token @llvm.coro.id(i32, ptr, ptr, ptr)
+declare i1 @llvm.coro.alloc(token)
+declare ptr @llvm.coro.begin(token, ptr)
+declare i1 @llvm.coro.end(ptr, i1, token)
+
+declare void @print(ptr)
+declare noalias ptr @malloc(i32)
+declare void @free(ptr)
\ No newline at end of file

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to