Author: Xun Li Date: 2020-10-12T12:00:20-07:00 New Revision: dce8f2bb25ea1d01533d8e602f2520492fa67259
URL: https://github.com/llvm/llvm-project/commit/dce8f2bb25ea1d01533d8e602f2520492fa67259 DIFF: https://github.com/llvm/llvm-project/commit/dce8f2bb25ea1d01533d8e602f2520492fa67259.diff LOG: [Coroutine][Sema] Only tighten the suspend call temp lifetime for final awaiter In https://reviews.llvm.org/D87470 I added the change to tighten the lifetime of the expression awaiter.await_suspend().address. Howver it was incorrect. ExprWithCleanups will call the dtor and end the lifetime for all the temps created in the current full expr. When this is called on a normal await call, we don't want to do that. We only want to do this for the call on the final_awaiter, to avoid writing into the frame after the frame is destroyed. This change fixes it, by checking IsImplicit. Differential Revision: https://reviews.llvm.org/D89066 Added: Modified: clang/lib/Sema/SemaCoroutine.cpp clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp Removed: ################################################################################ diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp index 565f907e05b2..5582c728aa2d 100644 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -375,7 +375,7 @@ static ExprResult buildMemberCall(Sema &S, Expr *Base, SourceLocation Loc, // returning await_suspend that results in a guaranteed tail call to the target // coroutine. static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E, - SourceLocation Loc) { + SourceLocation Loc, bool IsImplicit) { if (RetType->isReferenceType()) return nullptr; Type const *T = RetType.getTypePtr(); @@ -398,10 +398,17 @@ static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E, diag::warn_coroutine_handle_address_invalid_return_type) << JustAddress->getType(); - // The coroutine handle used to obtain the address is no longer needed - // at this point, clean it up to avoid unnecessarily long lifetime which - // could lead to unnecessary spilling. - JustAddress = S.MaybeCreateExprWithCleanups(JustAddress); + // After the await_suspend call on the awaiter, the coroutine may have + // been destroyed. In that case, we can not store anything to the frame + // from this point on. Hence here we wrap it immediately with a cleanup. This + // could have applied to all await_suspend calls. However doing so causes + // alive objects being destructed for reasons that need further + // investigations. Here we walk-around it temporarily by only doing it after + // the suspend call on the final awaiter (indicated by IsImplicit) where it's + // most common to happen. + // TODO: Properly clean up the temps generated by await_suspend calls. + if (IsImplicit) + JustAddress = S.MaybeCreateExprWithCleanups(JustAddress); return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume, JustAddress); } @@ -409,7 +416,8 @@ static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E, /// Build calls to await_ready, await_suspend, and await_resume for a co_await /// expression. static ReadySuspendResumeResult buildCoawaitCalls(Sema &S, VarDecl *CoroPromise, - SourceLocation Loc, Expr *E) { + SourceLocation Loc, Expr *E, + bool IsImplicit) { OpaqueValueExpr *Operand = new (S.Context) OpaqueValueExpr(Loc, E->getType(), VK_LValue, E->getObjectKind(), E); @@ -458,7 +466,8 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema &S, VarDecl *CoroPromise, QualType RetType = AwaitSuspend->getCallReturnType(S.Context); // Experimental support for coroutine_handle returning await_suspend. - if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc)) + if (Expr *TailCallSuspend = + maybeTailCall(S, RetType, AwaitSuspend, Loc, IsImplicit)) Calls.Results[ACT::ACT_Suspend] = TailCallSuspend; else { // non-class prvalues always have cv-unqualified types @@ -870,8 +879,8 @@ ExprResult Sema::BuildResolvedCoawaitExpr(SourceLocation Loc, Expr *E, SourceLocation CallLoc = E->getExprLoc(); // Build the await_ready, await_suspend, await_resume calls. - ReadySuspendResumeResult RSS = - buildCoawaitCalls(*this, Coroutine->CoroutinePromise, CallLoc, E); + ReadySuspendResumeResult RSS = buildCoawaitCalls( + *this, Coroutine->CoroutinePromise, CallLoc, E, IsImplicit); if (RSS.IsInvalid) return ExprError(); @@ -925,8 +934,8 @@ ExprResult Sema::BuildCoyieldExpr(SourceLocation Loc, Expr *E) { E = CreateMaterializeTemporaryExpr(E->getType(), E, true); // Build the await_ready, await_suspend, await_resume calls. - ReadySuspendResumeResult RSS = - buildCoawaitCalls(*this, Coroutine->CoroutinePromise, Loc, E); + ReadySuspendResumeResult RSS = buildCoawaitCalls( + *this, Coroutine->CoroutinePromise, Loc, E, /*IsImplicit*/ false); if (RSS.IsInvalid) return ExprError(); diff --git a/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp b/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp index 09205799c3f7..9833f14b273d 100644 --- a/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp +++ b/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp @@ -1,4 +1,4 @@ -// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o - +// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o - | FileCheck %s #include "Inputs/coroutine.h" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits