Skip to content

Commit a246e37

Browse files
committed
[SILGen]: ban forward captures of let temporary allocations
This addresses the same problem as #37935 but handles address-only lets rather than vars.
1 parent aca07fb commit a246e37

File tree

2 files changed

+153
-5
lines changed

2 files changed

+153
-5
lines changed

lib/SILGen/SILGenDecl.cpp

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -827,14 +827,15 @@ class LetValueInitialization : public Initialization {
827827
if (isUninitialized)
828828
address = SGF.B.createMarkUninitializedVar(vd, address);
829829
DestroyCleanup = SGF.enterDormantTemporaryCleanup(address, *lowering);
830-
SGF.VarLocs[vd] = SILGenFunction::VarLoc(address,
831-
SILAccessEnforcement::Unknown);
830+
831+
// N.B. We do not register the address in VarLocs yet so that the capture-
832+
// before-definition machinery will diagnose such cases.
832833
}
833834
// Push a cleanup to destroy the let declaration. This has to be
834-
// inactive until the variable is initialized: if control flow exits the
835+
// inactive until the variable is initialized: if control flow exits
835836
// before the value is bound, we don't want to destroy the value.
836837
//
837-
// Cleanups are required for all lexically scoped variables to delimite
838+
// Cleanups are required for all lexically scoped variables to delimit
838839
// the variable scope, even if the cleanup does nothing.
839840
SGF.Cleanups.pushCleanupInState<DestroyLocalVariable>(
840841
CleanupState::Dormant, vd);
@@ -1006,7 +1007,17 @@ class LetValueInitialization : public Initialization {
10061007
void finishInitialization(SILGenFunction &SGF) override {
10071008
assert(!DidFinish &&
10081009
"called LetValueInit::finishInitialization twice!");
1009-
assert(SGF.VarLocs.count(vd) && "Didn't bind a value to this let!");
1010+
1011+
if (address) {
1012+
// We should have delayed registration to detect forward-declared
1013+
// captures.
1014+
assert(!SGF.VarLocs.count(vd) && "Should not have bound the let yet!");
1015+
SGF.VarLocs[vd] =
1016+
SILGenFunction::VarLoc(address, SILAccessEnforcement::Unknown);
1017+
} else {
1018+
// We should have already registered the variable in the non-address case.
1019+
assert(SGF.VarLocs.count(vd) && "Didn't bind a value to this let!");
1020+
}
10101021

10111022
// Deactivate any cleanups we made when splitting the tuple.
10121023
for (auto cleanup : SplitCleanups)

test/SILGen/capture_order.swift

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,140 @@ func func77933460() {
192192
// expected-note@-3 {{captured value declared here}}
193193
// expected-warning@-4 {{variable 'obj' was never mutated; consider changing to 'let' constant}}
194194
}
195+
196+
// MARK: - Forward Declared Lets
197+
198+
// https://github.com/swiftlang/swift/issues/84909
199+
// Make sure we can't capture an uninitialized 'let' temporary allocation either.
200+
201+
protocol P {}
202+
203+
enum E {
204+
static func static_gen_fwd<T>(_ g: () -> T) -> T { g() }
205+
}
206+
207+
func global_fwd(_ a: () -> Any) -> Any { a() }
208+
func global_gen_fwd<T>(_ g: () -> T) -> T { g() }
209+
func global_fwd_p(_ p: () -> any P) -> any P { p() }
210+
211+
func forward_declared_let_captures() {
212+
do {
213+
let bad: Any = { bad }()
214+
// expected-error@-1 {{closure captures 'bad' before it is declared}}
215+
// expected-note@-2 {{captured here}}
216+
// expected-note@-3 {{captured value declared here}}
217+
}
218+
219+
do {
220+
func fwd(_ i: () -> Any) -> Any { i() }
221+
let bad = fwd { bad }
222+
// expected-error@-1 {{closure captures 'bad' before it is declared}}
223+
// expected-note@-2 {{captured here}}
224+
// expected-note@-3 {{captured value declared here}}
225+
}
226+
227+
do {
228+
func bad_local_f() -> Any { bad }
229+
// expected-error@-1 {{closure captures 'bad' before it is declared}}
230+
// expected-note@-2 {{captured here}}
231+
let bad = bad_local_f()
232+
// expected-note@-1 {{captured value declared here}}
233+
}
234+
235+
do {
236+
let bad = global_fwd { bad }
237+
// expected-error@-1 {{closure captures 'bad' before it is declared}}
238+
// expected-note@-2 {{captured here}}
239+
// expected-note@-3 {{captured value declared here}}
240+
}
241+
242+
do {
243+
let bad: Any = global_gen_fwd { bad }
244+
// expected-error@-1 {{closure captures 'bad' before it is declared}}
245+
// expected-note@-2 {{captured here}}
246+
// expected-note@-3 {{captured value declared here}}
247+
}
248+
249+
do {
250+
let bad: Any = E.static_gen_fwd { bad }
251+
// expected-error@-1 {{closure captures 'bad' before it is declared}}
252+
// expected-note@-2 {{captured here}}
253+
// expected-note@-3 {{captured value declared here}}
254+
}
255+
256+
do {
257+
let badNested: Any = global_fwd { { [badNested] in badNested }() }
258+
// expected-error@-1 {{closure captures 'badNested' before it is declared}}
259+
// expected-note@-2 {{captured here}}
260+
// expected-note@-3 {{captured value declared here}}
261+
}
262+
263+
do {
264+
let badOpt: Any? = { () -> Any? in badOpt }()
265+
// expected-error@-1 {{closure captures 'badOpt' before it is declared}}
266+
// expected-note@-2 {{captured here}}
267+
// expected-note@-3 {{captured value declared here}}
268+
}
269+
270+
do {
271+
let badTup: (Any, Any) = { (badTup.0, badTup.1) }()
272+
// expected-error@-1 {{closure captures 'badTup' before it is declared}}
273+
// expected-note@-2 {{captured here}}
274+
// expected-note@-3 {{captured value declared here}}
275+
}
276+
277+
do {
278+
let badTup: (Int, Any) = { (badTup.0, badTup.1) }()
279+
// expected-error@-1 {{closure captures 'badTup' before it is declared}}
280+
// expected-note@-2 {{captured here}}
281+
// expected-note@-3 {{captured value declared here}}
282+
}
283+
284+
do {
285+
let (badTup3, badTup4): (Any, Any) = { (badTup4, badTup3) }()
286+
// expected-error@-1 {{closure captures 'badTup4' before it is declared}}
287+
// expected-note@-2 {{captured here}}
288+
// expected-note@-3 {{captured value declared here}}
289+
// expected-error@-4 {{closure captures 'badTup3' before it is declared}}
290+
// expected-note@-5 {{captured here}}
291+
// expected-note@-6 {{captured value declared here}}
292+
}
293+
294+
do {
295+
struct S { var p: Any }
296+
let badStruct: S = { S(p: badStruct.p) }()
297+
// expected-error@-1 {{closure captures 'badStruct' before it is declared}}
298+
// expected-note@-2 {{captured here}}
299+
// expected-note@-3 {{captured value declared here}}
300+
}
301+
302+
do {
303+
enum EE {
304+
case boring
305+
case weird(Any)
306+
case strange(Any)
307+
}
308+
309+
let badEnum: EE = { .weird(EE.strange(badEnum)) }()
310+
// expected-error@-1 {{closure captures 'badEnum' before it is declared}}
311+
// expected-note@-2 {{captured here}}
312+
// expected-note@-3 {{captured value declared here}}
313+
}
314+
315+
do {
316+
let badproto: any P = global_fwd_p { badproto }
317+
// expected-error@-1 {{closure captures 'badproto' before it is declared}}
318+
// expected-note@-2 {{captured here}}
319+
// expected-note@-3 {{captured value declared here}}
320+
}
321+
}
322+
323+
// FIXME: should/could these be diagnosed instead of accepted?
324+
func forward_declared_local_lazy_captures() {
325+
// runtime stack overflow, maybe UB?
326+
lazy var infiniteRecurse: Any = { infiniteRecurse }()
327+
328+
// ??? not entirely sure what's going on here...
329+
// this can be called and doesn't seem to trip the sanitizers...
330+
lazy var hmm: () -> Any = { hmm }
331+
}

0 commit comments

Comments
 (0)