[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    [PATCH] D144680: [Coroutines] Avoid creating conditional cleanup markers in suspend block
From:       Wei Wang via Phabricator via cfe-commits <cfe-commits () lists ! llvm ! org>
Date:       2023-02-28 23:31:47
Message-ID: soeVEOC4QLOe8eRswNBl5A () geopod-ismtpd-11
[Download RAW message or body]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGce7eb2e05544: [Coroutines] Avoid creating conditional cl=
eanup markers in suspend block (authored by weiwang).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144680/new/

https://reviews.llvm.org/D144680

Files:
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCoroutines/pr59181.cpp


["D144680.501311.patch" (D144680.501311.patch)]

Index: clang/test/CodeGenCoroutines/pr59181.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCoroutines/pr59181.cpp
@@ -0,0 +1,60 @@
+// Test for PR59181. Tests that no conditional cleanup is created around \
await_suspend. +//
+// REQUIRES: x86-registered-target
+//
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - -std=c++20 \
-disable-llvm-passes -fsanitize-address-use-after-scope | FileCheck %s +
+#include "Inputs/coroutine.h"
+
+struct Task {
+  int value_;
+  struct promise_type {
+    Task get_return_object() {
+      return Task{0};
+    }
+
+    std::suspend_never initial_suspend() noexcept {
+      return {};
+    }
+
+    std::suspend_never final_suspend() noexcept {
+      return {};
+    }
+
+    void return_value(Task t) noexcept {}
+    void unhandled_exception() noexcept {}
+
+    auto await_transform(Task t) {
+      struct Suspension {
+        auto await_ready() noexcept { return false;}
+        auto await_suspend(std::coroutine_handle<> coro) {
+          coro.destroy();
+        }
+
+        auto await_resume() noexcept {
+          return 0;
+        }
+      };
+      return Suspension{};
+    }
+  };
+};
+
+Task bar(bool cond) {
+  co_return cond ? Task{ co_await Task{}}: Task{};
+}
+
+void foo() {
+  bar(true);
+}
+
+// CHECK: cleanup.cont:{{.*}}
+// CHECK-NEXT: load i8
+// CHECK-NEXT: trunc
+// CHECK-NEXT: store i1 false
+// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 8, ptr [[REF:%ref.tmp[0-9]+]])
+
+// CHECK: await.suspend:{{.*}}
+// CHECK-NOT: call void @llvm.lifetime.start.p0(i64 8, ptr [[REF]])
+// CHECK: call void \
@_ZZN4Task12promise_type15await_transformES_EN10Suspension13await_suspendESt16coroutine_handleIvE
 +// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 8, ptr [[REF]])
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -333,6 +333,7 @@
   // in this header.
   struct CGCoroInfo {
     std::unique_ptr<CGCoroData> Data;
+    bool InSuspendBlock = false;
     CGCoroInfo();
     ~CGCoroInfo();
   };
@@ -342,6 +343,10 @@
     return CurCoro.Data != nullptr;
   }
 
+  bool inSuspendBlock() const {
+    return isCoroutine() && CurCoro.InSuspendBlock;
+  }
+
   /// CurGD - The GlobalDecl for the current function being compiled.
   GlobalDecl CurGD;
 
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -541,13 +541,17 @@
       // Avoid creating a conditional cleanup just to hold an llvm.lifetime.end
       // marker. Instead, start the lifetime of a conditional temporary earlier
       // so that it's unconditional. Don't do this with sanitizers which need
-      // more precise lifetime marks.
+      // more precise lifetime marks. However when inside an "await.suspend"
+      // block, we should always avoid conditional cleanup because it creates
+      // boolean marker that lives across await_suspend, which can destroy coro
+      // frame.
       ConditionalEvaluation *OldConditional = nullptr;
       CGBuilderTy::InsertPoint OldIP;
       if (isInConditionalBranch() && !E->getType().isDestructedType() &&
-          !SanOpts.has(SanitizerKind::HWAddress) &&
-          !SanOpts.has(SanitizerKind::Memory) &&
-          !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) {
+          ((!SanOpts.has(SanitizerKind::HWAddress) &&
+            !SanOpts.has(SanitizerKind::Memory) &&
+            !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) ||
+           inSuspendBlock())) {
         OldConditional = OutermostConditional;
         OutermostConditional = nullptr;
 
Index: clang/lib/CodeGen/CGCoroutine.cpp
===================================================================
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -198,7 +198,9 @@
   auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy);
   auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
 
+  CGF.CurCoro.InSuspendBlock = true;
   auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
+  CGF.CurCoro.InSuspendBlock = false;
   if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
     // Veto suspension if requested by bool returning await_suspend.
     BasicBlock *RealSuspendBlock =


[Attachment #4 (text/plain)]

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


[prev in list] [next in list] [prev in thread] [next in thread]