Skip to content

Commit e2cb057

Browse files
committed
SILOptimizer: Re-implement NPCR diagnostics in SIL pass
This fixes a test involving transitive captures of local functions, as well as an infinite recursion possible with the old code. Fixes <rdar://problem/34496304>.
1 parent 6a34ed0 commit e2cb057

File tree

3 files changed

+115
-0
lines changed

3 files changed

+115
-0
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,12 @@ NOTE(value_captured_here,none,"captured here", ())
137137
NOTE(value_captured_transitively,none,
138138
"captured indirectly by this call", ())
139139

140+
ERROR(err_noescape_param_call,none,
141+
"passing a %select{|closure which captures a }1non-escaping function "
142+
"parameter %0 to a call to a non-escaping function parameter can allow "
143+
"re-entrant modification of a variable",
144+
(DeclName, unsigned))
145+
140146
// Definite initialization diagnostics.
141147
NOTE(variable_defined_here,none,
142148
"%select{variable|constant}0 defined here", (bool))

lib/SILOptimizer/Mandatory/DiagnoseInvalidEscapingCaptures.cpp

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,11 +329,72 @@ static void checkPartialApply(ASTContext &Context, DeclContext *DC,
329329
}
330330
}
331331

332+
static void checkApply(ASTContext &Context, FullApplySite site) {
333+
auto isNoEscapeParam = [&](SILValue value) -> const ParamDecl * {
334+
// If the value is an escaping, do not enforce any restrictions.
335+
if (!value->getType().getASTType()->isNoEscape())
336+
return nullptr;
337+
338+
// If the value is not a function parameter, do not enforce any restrictions.
339+
return getParamDeclFromOperand(value);
340+
};
341+
342+
// If the callee is not a no-escape parameter, there is nothing to check.
343+
auto callee = site.getCalleeOrigin();
344+
if (!isNoEscapeParam(callee))
345+
return;
346+
347+
// See if any of our arguments are noescape parameters, or closures capturing
348+
// noescape parameters.
349+
SmallVector<std::pair<SILValue, bool>, 4> args;
350+
llvm::SmallDenseSet<SILValue, 4> visited;
351+
auto arglistInsert = [&](SILValue arg, bool capture) {
352+
if (visited.insert(arg).second)
353+
args.emplace_back(arg, capture);
354+
};
355+
356+
for (auto arg : site.getArguments())
357+
arglistInsert(arg, /*capture=*/false);
358+
359+
while (!args.empty()) {
360+
auto pair = args.pop_back_val();
361+
auto arg = pair.first;
362+
bool capture = pair.second;
363+
364+
if (auto *CI = dyn_cast<ConversionInst>(arg)) {
365+
arglistInsert(CI->getConverted(), /*capture=*/false);
366+
continue;
367+
}
368+
369+
// If one of our call arguments is a noescape parameter, diagnose the
370+
// violation.
371+
if (auto *param = isNoEscapeParam(arg)) {
372+
diagnose(Context, site.getLoc(), diag::err_noescape_param_call,
373+
param->getName(), capture);
374+
return;
375+
}
376+
377+
// If one of our call arguments is a closure, recursively visit all of
378+
// the closure's captures.
379+
if (auto *PAI = dyn_cast<PartialApplyInst>(arg)) {
380+
ApplySite site(PAI);
381+
for (auto arg : site.getArguments())
382+
arglistInsert(arg, /*capture=*/true);
383+
continue;
384+
}
385+
}
386+
}
387+
332388
static void checkForViolationsAtInstruction(ASTContext &Context,
333389
DeclContext *DC,
334390
SILInstruction *I) {
335391
if (auto *PAI = dyn_cast<PartialApplyInst>(I))
336392
checkPartialApply(Context, DC, PAI);
393+
394+
if (isa<ApplyInst>(I) || isa<TryApplyInst>(I)) {
395+
FullApplySite site(I);
396+
checkApply(Context, site);
397+
}
337398
}
338399

339400
static void checkEscapingCaptures(SILFunction *F) {
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// RUN: %target-swift-frontend -emit-sil %s -verify
2+
3+
func test0(a: (() -> ()) -> (), b: () -> ()) {
4+
a(b) // expected-error {{passing a non-escaping function parameter 'b' to a call to a non-escaping function parameter can allow re-entrant modification of a variable}}
5+
}
6+
7+
func test0(fn: (() -> ()) -> ()) {
8+
fn { fn {} } // expected-error {{passing a closure which captures a non-escaping function parameter 'fn' to a call to a non-escaping function parameter can allow re-entrant modification of a variable}}
9+
}
10+
11+
func test1(fn: (() -> ()) -> ()) {
12+
func foo() {
13+
fn { fn {} } // expected-error {{passing a closure which captures a non-escaping function parameter 'fn' to a call to a non-escaping function parameter can allow re-entrant modification of a variable}}
14+
}
15+
}
16+
17+
func test2(x: inout Int, fn: (() -> ()) -> ()) {
18+
func foo(myfn: () -> ()) {
19+
x += 1
20+
myfn()
21+
}
22+
23+
// Make sure we only complain about calls to noescape parameters.
24+
foo { fn {} }
25+
}
26+
27+
func test3(fn: (() -> ()) -> ()) {
28+
{ myfn in myfn { fn {} } }(fn) // expected-error {{passing a closure which captures a non-escaping function parameter 'fn' to a call to a non-escaping function parameter can allow re-entrant modification of a variable}}
29+
}
30+
31+
func test4(fn: (() -> ()) -> ()) {
32+
func foo() {
33+
fn {}
34+
}
35+
36+
fn(foo) // expected-error {{passing a closure which captures a non-escaping function parameter 'fn' to a call to a non-escaping function parameter can allow re-entrant modification of a variable}}
37+
}
38+
39+
// rdar://problem/34496304
40+
func test5(outer: (() throws -> Int) throws -> Int) throws -> Int {
41+
func descend(_ inner: (() throws -> Int) throws -> Int) throws -> Int {
42+
return try inner { // expected-error {{passing a closure which captures a non-escaping function parameter 'inner' to a call to a non-escaping function parameter can allow re-entrant modification of a variable}}
43+
try descend(inner)
44+
}
45+
}
46+
47+
return try descend(outer)
48+
}

0 commit comments

Comments
 (0)