[clang] [Win32][Coroutine] Fix inalloca arguments lifetime in coroutines (PR #212331)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 29 04:23:43 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: etiennep-chromium
<details>
<summary>Changes</summary>
On Win32 x86, arguments passed by value with non-trivial constructors/destructors require `inalloca`. If a coroutine suspends during the evaluation of these arguments, the stack frame is popped, destroying the already constructed arguments.
This fix detects when a call inside a coroutine requires `inalloca` and contains suspend points in its arguments. In this case, we:
1. Force all arguments of the call to be evaluated into temporaries in the coroutine frame (which survive suspension).
2. Delay the `llvm.stacksave` to the end of argument evaluation (after resume).
3. Allocate the `inalloca` struct on the stack after resume and copy the temporaries into it.
4. Emit lifetime markers (`llvm.lifetime.start`/`llvm.lifetime.end`) for the `inalloca` alloca to prevent it from being moved to the coroutine frame by CoroSplit.
5. Safely clean up the temporaries in the frame.
If an argument is non-copyable/non-movable, we now emit a compilation error.
BUG=LLVM#<!-- -->59382
---
Full diff: https://github.com/llvm/llvm-project/pull/212331.diff
9 Files Affected:
- (modified) clang/include/clang/AST/Expr.h (+4)
- (modified) clang/lib/AST/Expr.cpp (+16)
- (modified) clang/lib/CodeGen/CGCall.cpp (+87-4)
- (modified) clang/lib/CodeGen/CGExpr.cpp (+5)
- (modified) clang/lib/CodeGen/CGExprAgg.cpp (+10)
- (modified) clang/lib/CodeGen/CGExprCXX.cpp (+10-1)
- (modified) clang/lib/CodeGen/CodeGenFunction.h (+12)
- (modified) clang/lib/Sema/SemaExpr.cpp (+40)
- (added) clang/test/CodeGenCoroutines/coro-win32-inalloca.cpp (+93)
``````````diff
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index f95f87cc4e8e0..9a6347d08f790 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -247,6 +247,10 @@ class Expr : public ValueStmt {
return static_cast<bool>(getDependence() & ExprDependence::Error);
}
+ /// Whether this expression contains a coroutine suspend point
+ /// (co_await or co_yield).
+ bool containsCoroutineSuspendPoints() const;
+
/// getExprLoc - Return the preferred location for the arrow when diagnosing
/// a problem with a generic expression.
SourceLocation getExprLoc() const LLVM_READONLY;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9a9a76e265f6a..22a697fe09327 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3998,6 +3998,22 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
return false;
}
+static bool containsCoroutineSuspendPoints(const Stmt *S) {
+ if (!S)
+ return false;
+ if (isa<CoawaitExpr>(S) || isa<CoyieldExpr>(S) ||
+ isa<DependentCoawaitExpr>(S))
+ return true;
+ for (const Stmt *Child : S->children())
+ if (Child && containsCoroutineSuspendPoints(Child))
+ return true;
+ return false;
+}
+
+bool Expr::containsCoroutineSuspendPoints() const {
+ return ::containsCoroutineSuspendPoints(this);
+}
+
FPOptions Expr::getFPFeaturesInEffect(const LangOptions &LO) const {
if (auto Call = dyn_cast<CallExpr>(this))
return Call->getFPFeaturesInEffect(LO);
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index a29f7aab0e58b..17f9c8abd7497 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -28,7 +28,9 @@
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecordLayout.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/CodeGenOptions.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/CodeGen/CGFunctionInfo.h"
@@ -5029,6 +5031,46 @@ static bool isObjCMethodWithTypeParams(const ObjCMethodDecl *method) {
}
#endif
+static const MaterializeTemporaryExpr *getMTEToPreEvaluate(const Expr *E) {
+ while (true) {
+ E = E->IgnoreParens();
+ if (auto *Cleanups = dyn_cast<ExprWithCleanups>(E)) {
+ E = Cleanups->getSubExpr();
+ continue;
+ }
+ if (auto *Bind = dyn_cast<CXXBindTemporaryExpr>(E)) {
+ E = Bind->getSubExpr();
+ continue;
+ }
+ break;
+ }
+
+ if (auto *CE = dyn_cast<CXXConstructExpr>(E)) {
+ if (CE->getNumArgs() > 0) {
+ const Expr *Arg = CE->getArg(0);
+ while (true) {
+ Arg = Arg->IgnoreParens();
+ if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Arg))
+ return MTE;
+ if (auto *Cleanups = dyn_cast<ExprWithCleanups>(Arg)) {
+ Arg = Cleanups->getSubExpr();
+ continue;
+ }
+ if (auto *Bind = dyn_cast<CXXBindTemporaryExpr>(Arg)) {
+ Arg = Bind->getSubExpr();
+ continue;
+ }
+ if (auto *ICE = dyn_cast<ImplicitCastExpr>(Arg)) {
+ Arg = ICE->getSubExpr();
+ continue;
+ }
+ break;
+ }
+ }
+ }
+ return nullptr;
+}
+
/// EmitCallArgs - Emit call arguments for a function.
void CodeGenFunction::EmitCallArgs(
CallArgList &Args, PrototypeWrapper Prototype,
@@ -5127,7 +5169,30 @@ void CodeGenFunction::EmitCallArgs(
std::swap(Args.back(), *(&Args.back() - 1));
};
- // Insert a stack save if we're going to need any inalloca args.
+ SmallVector<const MaterializeTemporaryExpr *, 4> MTEsToPreEvaluate;
+ if (isCoroutine() && hasInAllocaArgs(CGM, ExplicitCC, ArgTypes)) {
+ for (const Expr *A : ArgRange) {
+ if (auto *MTE = getMTEToPreEvaluate(A)) {
+ MTEsToPreEvaluate.push_back(MTE);
+ }
+ }
+ }
+
+ if (!MTEsToPreEvaluate.empty()) {
+ if (LeftToRight) {
+ for (const MaterializeTemporaryExpr *MTE : MTEsToPreEvaluate) {
+ LValue LV = EmitMaterializeTemporaryExpr(MTE);
+ PreEvaluatedMaterializedTemporaries[MTE] = LV;
+ }
+ } else {
+ for (int i = MTEsToPreEvaluate.size() - 1; i >= 0; --i) {
+ const MaterializeTemporaryExpr *MTE = MTEsToPreEvaluate[i];
+ LValue LV = EmitMaterializeTemporaryExpr(MTE);
+ PreEvaluatedMaterializedTemporaries[MTE] = LV;
+ }
+ }
+ }
+
if (hasInAllocaArgs(CGM, ExplicitCC, ArgTypes)) {
assert(getTarget().getTriple().getArch() == llvm::Triple::x86 &&
"inalloca only supported on x86");
@@ -5175,6 +5240,10 @@ void CodeGenFunction::EmitCallArgs(
// Reverse the writebacks to match the MSVC ABI.
Args.reverseWritebacks();
}
+
+ for (const auto *MTE : MTEsToPreEvaluate) {
+ PreEvaluatedMaterializedTemporaries.erase(MTE);
+ }
}
namespace {
@@ -5673,17 +5742,28 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
llvm::Instruction *IP = CallArgs.getStackBase();
llvm::AllocaInst *AI;
if (IP) {
- IP = IP->getNextNode();
- AI = new llvm::AllocaInst(ArgStruct, DL.getAllocaAddrSpace(), "argmem",
- IP->getIterator());
+ if (llvm::Instruction *Next = IP->getNextNode()) {
+ AI = new llvm::AllocaInst(ArgStruct, DL.getAllocaAddrSpace(), "argmem",
+ Next->getIterator());
+ } else {
+ AI = new llvm::AllocaInst(ArgStruct, DL.getAllocaAddrSpace(), "argmem",
+ IP->getParent());
+ }
} else {
AI = CreateTempAlloca(ArgStruct, "argmem");
}
auto Align = CallInfo.getArgStructAlignment();
AI->setAlignment(Align.getAsAlign());
AI->setUsedWithInAlloca(true);
+ if (isCoroutine()) {
+ AI->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
+ llvm::MDNode::get(CGM.getLLVMContext(), {}));
+ }
assert(AI->isUsedWithInAlloca() && !AI->isStaticAlloca());
ArgMemory = RawAddress(AI, ArgStruct, Align);
+ if (isCoroutine()) {
+ EmitLifetimeStart(AI);
+ }
}
ClangToLLVMArgMapping IRFunctionArgs(CGM.getContext(), CallInfo);
@@ -6567,6 +6647,9 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
// The stack cleanup for inalloca arguments has to run out of the normal
// lexical order, so deactivate it and run it manually here.
CallArgs.freeArgumentMemory(*this);
+ if (isCoroutine() && ArgMemory.isValid()) {
+ EmitLifetimeEnd(ArgMemory.getPointer());
+ }
// Extract the return value.
RValue Ret;
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 9201e40bc13a1..2d05121c24f4f 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -521,6 +521,11 @@ static bool isAAPCS(const TargetInfo &TargetInfo) {
LValue CodeGenFunction::
EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
+ auto It = PreEvaluatedMaterializedTemporaries.find(M);
+ if (It != PreEvaluatedMaterializedTemporaries.end()) {
+ return It->second;
+ }
+
const Expr *E = M->getSubExpr();
assert((!M->getExtendingDecl() || !isa<VarDecl>(M->getExtendingDecl()) ||
diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index bc35ffdaad2bd..0c0dde5aa5178 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -777,6 +777,16 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType,
void AggExprEmitter::VisitMaterializeTemporaryExpr(
MaterializeTemporaryExpr *E) {
+ if (CGF.hasPreEvaluatedTemporary(E)) {
+ LValue SrcLV = CGF.getPreEvaluatedTemporary(E);
+ QualType Type = E->getType();
+ assert((!Type->getAsCXXRecordDecl() ||
+ Type->getAsCXXRecordDecl()->isTriviallyCopyable()) &&
+ "Non-trivially copyable MTE should not be visited as aggregate when "
+ "pre-evaluated");
+ EmitFinalDestCopy(Type, SrcLV);
+ return;
+ }
Visit(E->getSubExpr());
}
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index e400a5c5a49c5..7410e2bc18f51 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -637,7 +637,16 @@ void CodeGenFunction::EmitCXXConstructExpr(const CXXConstructExpr *E,
return;
// Elide the constructor if we're constructing from a temporary.
- if (getLangOpts().ElideConstructors && E->isElidable()) {
+ bool DisableElision = false;
+ if (E->getNumArgs() > 0) {
+ if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(
+ E->getArg(0)->IgnoreImpCasts())) {
+ if (hasPreEvaluatedTemporary(MTE))
+ DisableElision = true;
+ }
+ }
+
+ if (getLangOpts().ElideConstructors && E->isElidable() && !DisableElision) {
// FIXME: This only handles the simplest case, where the source object
// is passed directly as the first argument to the constructor.
// This should also handle stepping though implicit casts and
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 3fc9052adb87b..4517b03be8bee 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1774,6 +1774,8 @@ class CodeGenFunction : public CodeGenTypeCache {
/// expressions.
llvm::DenseMap<const OpaqueValueExpr *, LValue> OpaqueLValues;
llvm::DenseMap<const OpaqueValueExpr *, RValue> OpaqueRValues;
+ llvm::DenseMap<const MaterializeTemporaryExpr *, LValue>
+ PreEvaluatedMaterializedTemporaries;
// VLASizeMap - This keeps track of the associated size for each VLA type.
// We track this by the size expression rather than the type itself because
@@ -3106,6 +3108,16 @@ class CodeGenFunction : public CodeGenTypeCache {
/// already been emitted.
bool isOpaqueValueEmitted(const OpaqueValueExpr *E);
+ bool hasPreEvaluatedTemporary(const MaterializeTemporaryExpr *M) const {
+ return PreEvaluatedMaterializedTemporaries.count(M);
+ }
+ LValue getPreEvaluatedTemporary(const MaterializeTemporaryExpr *M) {
+ auto It = PreEvaluatedMaterializedTemporaries.find(M);
+ assert(It != PreEvaluatedMaterializedTemporaries.end() &&
+ "MTE is not pre-evaluated");
+ return It->second;
+ }
+
/// Get the index of the current ArrayInitLoopExpr, if any.
llvm::Value *getArrayInitIndex() { return ArrayInitIndex; }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ed3d27b5adc27..5f71404fb8c5f 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6250,6 +6250,30 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
return false;
}
+static bool isWin32InAllocaRecord(ASTContext &Context, QualType Ty) {
+ const RecordType *RT = Ty->getAs<RecordType>();
+ if (!RT)
+ return false;
+ const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl());
+ if (!RD)
+ return false;
+
+ const llvm::Triple &Triple = Context.getTargetInfo().getTriple();
+ if (Triple.getArch() != llvm::Triple::x86 || !Triple.isOSWindows())
+ return false;
+ if (!Context.getTargetInfo().getCXXABI().isMicrosoft())
+ return false;
+
+ if (RD->canPassInRegisters())
+ return false;
+
+ TypeInfo Info = Context.getTypeInfo(Context.getCanonicalTagType(RD));
+ if (Info.isAlignRequired() && Info.Align > 4)
+ return false; // passed indirectly
+
+ return true;
+}
+
bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,
const FunctionProtoType *Proto,
unsigned FirstParam, ArrayRef<Expr *> Args,
@@ -6257,6 +6281,15 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,
VariadicCallType CallType, bool AllowExplicit,
bool IsListInitialization) {
unsigned NumParams = Proto->getNumParams();
+ bool CallHasSuspend = false;
+ if (getCurFunction() && getCurFunction()->isCoroutine()) {
+ for (const Expr *A : Args) {
+ if (A && A->containsCoroutineSuspendPoints()) {
+ CallHasSuspend = true;
+ break;
+ }
+ }
+ }
bool Invalid = false;
size_t ArgIx = 0;
// Continue to check argument types (even if we have too few/many args).
@@ -6316,6 +6349,13 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,
: diag::warn_obt_discarded_at_function_boundary)
<< Arg->getType() << ProtoArgType;
}
+ if (CallHasSuspend && Arg->isPRValue() &&
+ isWin32InAllocaRecord(Context, ProtoArgType)) {
+ ExprResult Materialized =
+ CreateMaterializeTemporaryExpr(Arg->getType(), Arg, false);
+ if (!Materialized.isInvalid())
+ Arg = Materialized.get();
+ }
ExprResult ArgE = PerformCopyInitialization(
Entity, SourceLocation(), Arg, IsListInitialization, AllowExplicit);
diff --git a/clang/test/CodeGenCoroutines/coro-win32-inalloca.cpp b/clang/test/CodeGenCoroutines/coro-win32-inalloca.cpp
new file mode 100644
index 0000000000000..34d1d0d81d943
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-win32-inalloca.cpp
@@ -0,0 +1,93 @@
+// RUN: %clang_cc1 -std=c++20 -triple=i686-pc-windows-msvc -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s
+
+namespace std {
+template <typename R, typename... Args>
+struct coroutine_traits {
+ using promise_type = typename R::promise_type;
+};
+
+template <class Promise = void> struct coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle<void> {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template <class PromiseType>
+ coroutine_handle(coroutine_handle<PromiseType>) noexcept;
+};
+} // namespace std
+
+struct suspend_always {
+ bool await_ready() noexcept { return false; }
+ void await_suspend(std::coroutine_handle<>) noexcept {}
+ void await_resume() noexcept {}
+};
+
+struct suspend_never {
+ bool await_ready() noexcept { return true; }
+ void await_suspend(std::coroutine_handle<>) noexcept {}
+ void await_resume() noexcept {}
+};
+
+struct task {
+ struct promise_type {
+ task get_return_object() { return {}; }
+ suspend_never initial_suspend() { return {}; }
+ suspend_never final_suspend() noexcept { return {}; }
+ void return_void() {}
+ void unhandled_exception() {}
+ };
+};
+
+struct Noisy {
+ int val;
+ Noisy(int v);
+ Noisy(const Noisy&) = delete;
+ Noisy(Noisy&& o) noexcept;
+ ~Noisy();
+};
+
+struct Awaiter {
+ bool await_ready() noexcept { return false; }
+ void await_suspend(std::coroutine_handle<>) noexcept {}
+ Noisy await_resume() noexcept;
+};
+
+void consume_two(Noisy x, Noisy y);
+
+// CHECK-LABEL: define dso_local void @"?my_task@@YA?AUtask@@XZ"(
+task my_task() {
+ // CHECK: %[[MTE_Y:.+]] = alloca %struct.Noisy,
+ // CHECK: %[[MTE_X:.+]] = alloca %struct.Noisy,
+
+ // Evaluate Noisy(42) before suspend:
+ // CHECK: call x86_thiscallcc noundef ptr @"??0Noisy@@QAE at H@Z"(ptr {{[^,]*}} %[[MTE_Y]], i32 noundef 42)
+
+ // Suspend for co_await Awaiter{}:
+ // CHECK: call void @llvm.coro.await.suspend.void(
+ // CHECK: call i8 @llvm.coro.suspend(
+
+ // After resume:
+ // CHECK: call x86_thiscallcc void @"?await_resume at Awaiter@@QAE?AUNoisy@@XZ"(ptr {{[^,]*}} %{{.*}}, ptr dead_on_unwind writable sret(%struct.Noisy) align 4 %[[MTE_X]])
+
+ // Allocate inalloca:
+ // CHECK: %[[STACKSAVE:.+]] = call ptr @llvm.stacksave.p0()
+ // CHECK: %[[ARGMEM:.+]] = alloca inalloca <{ %struct.Noisy, %struct.Noisy }>, align 4, !coro.outside.frame ![[METADATA_NUM:[0-9]+]]
+
+ // Move y (pre-evaluated Noisy(42)) to inalloca:
+ // CHECK: %[[GEP_Y:.+]] = getelementptr inbounds nuw <{ %struct.Noisy, %struct.Noisy }>, ptr %[[ARGMEM]], i32 0, i32 1
+ // CHECK: call x86_thiscallcc noundef ptr @"??0Noisy@@QAE@$$QAU0@@Z"(ptr {{[^,]*}} %[[GEP_Y]], ptr noundef nonnull align 4 dereferenceable(4) %[[MTE_Y]])
+
+ // Move x (co_await Awaiter{} result) to inalloca:
+ // CHECK: %[[GEP_X:.+]] = getelementptr inbounds nuw <{ %struct.Noisy, %struct.Noisy }>, ptr %[[ARGMEM]], i32 0, i32 0
+ // CHECK: call x86_thiscallcc noundef ptr @"??0Noisy@@QAE@$$QAU0@@Z"(ptr {{[^,]*}} %[[GEP_X]], ptr noundef nonnull align 4 dereferenceable(4) %[[MTE_X]])
+
+ // Lifetime start and call:
+ // CHECK: call void @llvm.lifetime.start.p0(ptr %[[ARGMEM]])
+ // CHECK: call void @"?consume_two@@YAXUNoisy@@0 at Z"(ptr inalloca(<{ %struct.Noisy, %struct.Noisy }>) %[[ARGMEM]])
+
+ consume_two(co_await Awaiter{}, Noisy(42));
+}
+
+// CHECK: ![[METADATA_NUM]] = !{}
``````````
</details>
https://github.com/llvm/llvm-project/pull/212331
More information about the cfe-commits
mailing list