[clang] [Clang] Fix ICE when `initial_suspend()`'s `await_resume()` returns a non-trivially destructible type (PR #72935)
Yuxuan Chen via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 20 17:06:20 PST 2023
https://github.com/yuxuanchen1997 created https://github.com/llvm/llvm-project/pull/72935
None
>From 13b54167ea0dfe4f39de7b29c3e61489aa683caa Mon Sep 17 00:00:00 2001
From: Yuxuan Chen <ych at meta.com>
Date: Mon, 20 Nov 2023 16:06:44 -0800
Subject: [PATCH 1/2] Attempt to fix init sus ret destroy
---
clang/lib/CodeGen/CGCoroutine.cpp | 5 ++++-
clang/lib/CodeGen/CGException.cpp | 1 +
clang/lib/CodeGen/CodeGenFunction.cpp | 8 ++++++++
clang/lib/CodeGen/CodeGenFunction.h | 2 ++
4 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..78df459113090e5 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -229,6 +229,8 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
// Emit await_resume expression.
CGF.EmitBlock(ReadyBlock);
+ // Never create an LValue for an initial suspend.
+ assert(Kind != AwaitKind::Init || !forLValue);
// Exception handling requires additional IR. If the 'await_resume' function
// is marked as 'noexcept', we avoid generating this additional IR.
CXXTryStmt *TryStmt = nullptr;
@@ -244,6 +246,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
auto *TryBody = CompoundStmt::Create(CGF.getContext(), S.getResumeExpr(),
FPOptionsOverride(), Loc, Loc);
TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
+ // We circumvent emitting the entire try stmt to get rvalue of the expr.
CGF.EnterCXXTryStmt(*TryStmt);
}
@@ -708,9 +711,9 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
CurCoro.Data->CurrentAwaitKind = AwaitKind::Init;
CurCoro.Data->ExceptionHandler = S.getExceptionHandler();
- EmitStmt(S.getInitSuspendStmt());
CurCoro.Data->FinalJD = getJumpDestInCurrentScope(FinalBB);
+ EmitStmt(S.getInitSuspendStmt());
CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
if (CurCoro.Data->ExceptionHandler) {
diff --git a/clang/lib/CodeGen/CGException.cpp b/clang/lib/CodeGen/CGException.cpp
index bae8babb8efe4a8..99c30a5837ac1bb 100644
--- a/clang/lib/CodeGen/CGException.cpp
+++ b/clang/lib/CodeGen/CGException.cpp
@@ -1202,6 +1202,7 @@ void CodeGenFunction::popCatchScope() {
void CodeGenFunction::ExitCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock) {
unsigned NumHandlers = S.getNumHandlers();
+
EHCatchScope &CatchScope = cast<EHCatchScope>(*EHStack.begin());
assert(CatchScope.getNumHandlers() == NumHandlers);
llvm::BasicBlock *DispatchBlock = CatchScope.getCachedEHDispatchBlock();
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 2199d7b58fb96e6..1a4bca20cc1a112 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2925,3 +2925,11 @@ llvm::Value *CodeGenFunction::emitBoolVecConversion(llvm::Value *SrcVec,
return Builder.CreateShuffleVector(SrcVec, ShuffleMask, Name);
}
+
+void CodeGenFunction::printEHStack() const {
+ llvm::dbgs() << "EHStack: ";
+ for (const auto& scope : EHStack) {
+ llvm::dbgs() << scope.getKind() << " ";
+ }
+ llvm::dbgs() << "\n";
+}
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 618e78809db408b..36a82b63d0b1ff6 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -769,6 +769,8 @@ class CodeGenFunction : public CodeGenTypeCache {
return CurrentFuncletPad && isa<llvm::CleanupPadInst>(CurrentFuncletPad);
}
+ void printEHStack() const;
+
/// pushFullExprCleanup - Push a cleanup to be run at the end of the
/// current full-expression. Safe against the possibility that
/// we're currently inside a conditionally-evaluated expression.
>From 72fcc02a032e28517d87c8c0fe8279b9833bc9a9 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen <ych at meta.com>
Date: Mon, 20 Nov 2023 17:01:05 -0800
Subject: [PATCH 2/2] remove try stmt around initial suspend, merge with main
try
---
clang/lib/CodeGen/CGCoroutine.cpp | 55 +++++--------------------------
1 file changed, 8 insertions(+), 47 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index 78df459113090e5..4c171f36ff2e17c 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -231,24 +231,6 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
// Never create an LValue for an initial suspend.
assert(Kind != AwaitKind::Init || !forLValue);
- // Exception handling requires additional IR. If the 'await_resume' function
- // is marked as 'noexcept', we avoid generating this additional IR.
- CXXTryStmt *TryStmt = nullptr;
- if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
- memberCallExpressionCanThrow(S.getResumeExpr())) {
- Coro.ResumeEHVar =
- CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
- Builder.CreateFlagStore(true, Coro.ResumeEHVar);
-
- auto Loc = S.getResumeExpr()->getExprLoc();
- auto *Catch = new (CGF.getContext())
- CXXCatchStmt(Loc, /*exDecl=*/nullptr, Coro.ExceptionHandler);
- auto *TryBody = CompoundStmt::Create(CGF.getContext(), S.getResumeExpr(),
- FPOptionsOverride(), Loc, Loc);
- TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
- // We circumvent emitting the entire try stmt to get rvalue of the expr.
- CGF.EnterCXXTryStmt(*TryStmt);
- }
LValueOrRValue Res;
if (forLValue)
@@ -256,11 +238,6 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
- Builder.CreateFlagStore(false, Coro.ResumeEHVar);
- CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
@@ -713,39 +690,23 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
CurCoro.Data->ExceptionHandler = S.getExceptionHandler();
CurCoro.Data->FinalJD = getJumpDestInCurrentScope(FinalBB);
- EmitStmt(S.getInitSuspendStmt());
- CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
-
if (CurCoro.Data->ExceptionHandler) {
- // If we generated IR to record whether an exception was thrown from
- // 'await_resume', then use that IR to determine whether the coroutine
- // body should be skipped.
- // If we didn't generate the IR (perhaps because 'await_resume' was marked
- // as 'noexcept'), then we skip this check.
- BasicBlock *ContBB = nullptr;
- if (CurCoro.Data->ResumeEHVar) {
- BasicBlock *BodyBB = createBasicBlock("coro.resumed.body");
- ContBB = createBasicBlock("coro.resumed.cont");
- Value *SkipBody = Builder.CreateFlagLoad(CurCoro.Data->ResumeEHVar,
- "coro.resumed.eh");
- Builder.CreateCondBr(SkipBody, ContBB, BodyBB);
- EmitBlock(BodyBB);
- }
-
auto Loc = S.getBeginLoc();
CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
CurCoro.Data->ExceptionHandler);
+
+ CompoundStmt *BodyStmt = S.getBody();
auto *TryStmt =
- CXXTryStmt::Create(getContext(), Loc, S.getBody(), &Catch);
+ CXXTryStmt::Create(getContext(), Loc, BodyStmt, &Catch);
EnterCXXTryStmt(*TryStmt);
+ EmitStmt(S.getInitSuspendStmt());
+ CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
ExitCXXTryStmt(*TryStmt);
-
- if (ContBB)
- EmitBlock(ContBB);
- }
- else {
+ } else {
+ EmitStmt(S.getInitSuspendStmt());
+ CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
emitBodyAndFallthrough(*this, S, S.getBody());
}
More information about the cfe-commits
mailing list