Skip to content

Commit cde82a2

Browse files
committed
[Coroutines] Mark parameter allocas with coro.outside.frame metadata
Parameters to a coroutine get copied (moved) to coroutine-local instances which code inside the coroutine then uses. The original parameters should not be part of the frame. Normally CoroSplit figures that out by itself, but for [[clang::trivial_abi]] parameters which, get destructed at the end of the ramp function, it does not (see bug), causing use-after-free's if the frame is destroyed before the end of the ramp (as happens if it doesn't suspend). Since Clang knows these should never be part of the frame, use metadata to make it so. Fixes #127499
1 parent 6d86a8a commit cde82a2

File tree

2 files changed

+40
-9
lines changed

2 files changed

+40
-9
lines changed

clang/lib/CodeGen/CGCoroutine.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,16 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
855855
// Create parameter copies. We do it before creating a promise, since an
856856
// evolution of coroutine TS may allow promise constructor to observe
857857
// parameter copies.
858+
for (const ParmVarDecl *Parm : FnArgs) {
859+
// If the original param is in an alloca, exclude it from the coroutine
860+
// frame. The parameter copy will be part of the frame.
861+
Address ParmAddr = GetAddrOfLocalVar(Parm);
862+
if (auto *ParmAlloca =
863+
dyn_cast<llvm::AllocaInst>(ParmAddr.getBasePointer())) {
864+
ParmAlloca->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
865+
llvm::MDNode::get(CGM.getLLVMContext(), {}));
866+
}
867+
}
858868
for (auto *PM : S.getParamMoves()) {
859869
EmitStmt(PM);
860870
ParamReplacer.addCopy(cast<DeclStmt>(PM));

clang/test/CodeGenCoroutines/coro-params.cpp

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,46 +59,67 @@ struct MoveAndCopy {
5959
~MoveAndCopy();
6060
};
6161

62-
void consume(int,int,int) noexcept;
62+
struct [[clang::trivial_abi]] TrivialABI {
63+
int val;
64+
TrivialABI(MoveAndCopy&&) noexcept;
65+
~TrivialABI();
66+
};
67+
68+
void consume(int,int,int,int) noexcept;
6369

6470
// TODO: Add support for CopyOnly params
65-
// CHECK: define{{.*}} void @_Z1fi8MoveOnly11MoveAndCopy(i32 noundef %val, ptr noundef %[[MoParam:.+]], ptr noundef %[[McParam:.+]]) #0 personality ptr @__gxx_personality_v0
66-
void f(int val, MoveOnly moParam, MoveAndCopy mcParam) {
71+
// CHECK: define{{.*}} void @_Z1fi8MoveOnly11MoveAndCopy10TrivialABI(i32 noundef %val, ptr noundef %[[MoParam:.+]], ptr noundef %[[McParam:.+]], i32 %[[TrivialParam:.+]]) #0 personality ptr @__gxx_personality_v0
72+
void f(int val, MoveOnly moParam, MoveAndCopy mcParam, TrivialABI trivialParam) {
73+
// CHECK: %[[TrivialAlloca:.+]] = alloca %struct.TrivialABI,
74+
// CHECK-SAME: !coro.outside.frame
6775
// CHECK: %[[MoCopy:.+]] = alloca %struct.MoveOnly,
6876
// CHECK: %[[McCopy:.+]] = alloca %struct.MoveAndCopy,
77+
// CHECK: %[[TrivialCopy:.+]] = alloca %struct.TrivialABI,
6978
// CHECK: store i32 %val, ptr %[[ValAddr:.+]]
7079

7180
// CHECK: call ptr @llvm.coro.begin(
7281
// CHECK: call void @_ZN8MoveOnlyC1EOS_(ptr {{[^,]*}} %[[MoCopy]], ptr noundef nonnull align 4 dereferenceable(4) %[[MoParam]])
7382
// CHECK-NEXT: call void @llvm.lifetime.start.p0(
7483
// CHECK-NEXT: call void @_ZN11MoveAndCopyC1EOS_(ptr {{[^,]*}} %[[McCopy]], ptr noundef nonnull align 4 dereferenceable(4) %[[McParam]]) #
7584
// CHECK-NEXT: call void @llvm.lifetime.start.p0(
76-
// CHECK-NEXT: invoke void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeC1Ev(
85+
// CHECK-NEXT: call void @llvm.memcpy
86+
// CHECK-SAME: %[[TrivialCopy]]
87+
// CHECK-SAME: %[[TrivialAlloca]]
88+
// CHECK-NEXT: call void @llvm.lifetime.start.p0(
89+
// CHECK-NEXT: invoke void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopy10TrivialABIEE12promise_typeC1Ev(
7790

7891
// CHECK: call void @_ZN14suspend_always12await_resumeEv(
7992
// CHECK: %[[IntParam:.+]] = load i32, ptr %{{.*}}
8093
// CHECK: %[[MoGep:.+]] = getelementptr inbounds nuw %struct.MoveOnly, ptr %[[MoCopy]], i32 0, i32 0
8194
// CHECK: %[[MoVal:.+]] = load i32, ptr %[[MoGep]]
82-
// CHECK: %[[McGep:.+]] = getelementptr inbounds nuw %struct.MoveAndCopy, ptr %[[McCopy]], i32 0, i32 0
95+
// CHECK: %[[McGep:.+]] = getelementptr inbounds nuw %struct.MoveAndCopy, ptr %[[McCopy]], i32 0, i32 0
8396
// CHECK: %[[McVal:.+]] = load i32, ptr %[[McGep]]
84-
// CHECK: call void @_Z7consumeiii(i32 noundef %[[IntParam]], i32 noundef %[[MoVal]], i32 noundef %[[McVal]])
97+
// CHECK: %[[TrivialGep:.+]] = getelementptr inbounds nuw %struct.TrivialABI, ptr %[[TrivialCopy]], i32 0, i32 0
98+
// CHECK: %[[TrivialVal:.+]] = load i32, ptr %[[TrivialGep]]
99+
// CHECK: call void @_Z7consumeiiii(i32 noundef %[[IntParam]], i32 noundef %[[MoVal]], i32 noundef %[[McVal]], i32 noundef %[[TrivialVal]])
85100

86-
consume(val, moParam.val, mcParam.val);
101+
consume(val, moParam.val, mcParam.val, trivialParam.val);
87102
co_return;
88103

89104
// Skip to final suspend:
90-
// CHECK: call void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_type13final_suspendEv(
105+
// CHECK: call void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopy10TrivialABIEE12promise_type13final_suspendEv(
91106
// CHECK: call void @_ZN14suspend_always12await_resumeEv(
92107

93108
// Destroy promise, then parameter copies:
94-
// CHECK: call void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeD1Ev(ptr {{[^,]*}} %__promise)
109+
// CHECK: call void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopy10TrivialABIEE12promise_typeD1Ev(ptr {{[^,]*}} %__promise)
110+
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
111+
// CHECK-NEXT: call void @_ZN10TrivialABID1Ev(ptr {{[^,]*}} %[[TrivialCopy]])
95112
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
96113
// CHECK-NEXT: call void @_ZN11MoveAndCopyD1Ev(ptr {{[^,]*}} %[[McCopy]])
97114
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
98115
// CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(ptr {{[^,]*}} %[[MoCopy]]
99116
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
100117
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
101118
// CHECK-NEXT: call ptr @llvm.coro.free(
119+
120+
// The original trivial_abi parameter is destroyed when returning from the ramp.
121+
// CHECK: call i1 @llvm.coro.end
122+
// CHECK: call void @_ZN10TrivialABID1Ev(ptr {{[^,]*}} %[[TrivialAlloca]])
102123
}
103124

104125
// CHECK-LABEL: void @_Z16dependent_paramsI1A1BEvT_T0_S3_(ptr noundef %x, ptr noundef %0, ptr noundef %y)

0 commit comments

Comments
 (0)