Skip to content

Commit a466ece

Browse files
eecksteineeckstein
authored
Merge pull request #21844 from eeckstein/fix-erase-apply-5.0
[5.0] SILCombine: fix a miscompile caused by dead-apply elimination
2 parents cedfa64 + 1d70bc9 commit a466ece

File tree

5 files changed

+158
-0
lines changed

5 files changed

+158
-0
lines changed

‎include/swift/SILOptimizer/Utils/Local.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,9 @@ class ValueLifetimeAnalysis {
282282
return LiveBlocks.count(BB) && BB != DefValue->getParent();
283283
}
284284

285+
/// Checks if there is a dealloc_ref inside the value's live range.
286+
bool containsDeallocRef(const Frontier &Frontier);
287+
285288
/// For debug dumping.
286289
void dump() const;
287290

‎lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,13 @@ bool SILCombiner::eraseApply(FullApplySite FAS, const UserListTy &Users) {
547547
// Compute the places where we have to insert release-instructions for the
548548
// owned arguments. This must not be done before the result of the
549549
// apply is destroyed. Therefore we compute the lifetime of the apply-result.
550+
551+
// TODO: this is not required anymore when we have ownership SIL. But with
552+
// the current SIL it can happen that the retain of a parameter is moved
553+
// _after_ the apply.
554+
// When we have ownership SIL we can just destroy the parameters at the apply
555+
// location.
556+
550557
ValueLifetimeAnalysis VLA(FAS.getInstruction(), Users);
551558
ValueLifetimeAnalysis::Frontier Frontier;
552559
if (Users.empty()) {
@@ -556,6 +563,12 @@ bool SILCombiner::eraseApply(FullApplySite FAS, const UserListTy &Users) {
556563
} else {
557564
if (!VLA.computeFrontier(Frontier, ValueLifetimeAnalysis::DontModifyCFG))
558565
return false;
566+
// As we are extending the lifetimes of owned parameters, we have to make
567+
// sure that no dealloc_ref instructions are within this extended liferange.
568+
// It could be that the dealloc_ref is deallocating a parameter and then
569+
// we would have a release after the dealloc.
570+
if (VLA.containsDeallocRef(Frontier))
571+
return false;
559572
}
560573

561574
// Release and destroy any owned or in-arguments.

‎lib/SILOptimizer/Utils/Local.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,6 +1371,44 @@ bool ValueLifetimeAnalysis::isWithinLifetime(SILInstruction *Inst) {
13711371
llvm_unreachable("Expected to find use of value in block!");
13721372
}
13731373

1374+
// Searches \p BB backwards from the instruction before \p FrontierInst
1375+
// to the beginning of the list and returns true if we find a dealloc_ref
1376+
// /before/ we find \p DefValue (the instruction that defines our target value).
1377+
static bool blockContainsDeallocRef(SILBasicBlock *BB, SILInstruction *DefValue,
1378+
SILInstruction *FrontierInst) {
1379+
SILBasicBlock::reverse_iterator End = BB->rend();
1380+
SILBasicBlock::reverse_iterator Iter = FrontierInst->getReverseIterator();
1381+
for (++Iter; Iter != End; ++Iter) {
1382+
SILInstruction *I = &*Iter;
1383+
if (isa<DeallocRefInst>(I))
1384+
return true;
1385+
if (I == DefValue)
1386+
return false;
1387+
}
1388+
return false;
1389+
}
1390+
1391+
bool ValueLifetimeAnalysis::containsDeallocRef(const Frontier &Frontier) {
1392+
SmallPtrSet<SILBasicBlock *, 8> FrontierBlocks;
1393+
// Search in live blocks where the value is not alive until the end of the
1394+
// block, i.e. the live range is terminated by a frontier instruction.
1395+
for (SILInstruction *FrontierInst : Frontier) {
1396+
SILBasicBlock *BB = FrontierInst->getParent();
1397+
if (blockContainsDeallocRef(BB, DefValue, FrontierInst))
1398+
return true;
1399+
FrontierBlocks.insert(BB);
1400+
}
1401+
// Search in all other live blocks where the value is alive until the end of
1402+
// the block.
1403+
for (SILBasicBlock *BB : LiveBlocks) {
1404+
if (FrontierBlocks.count(BB) == 0) {
1405+
if (blockContainsDeallocRef(BB, DefValue, BB->getTerminator()))
1406+
return true;
1407+
}
1408+
}
1409+
return false;
1410+
}
1411+
13741412
void ValueLifetimeAnalysis::dump() const {
13751413
llvm::errs() << "lifetime of def: " << *DefValue;
13761414
for (SILInstruction *Use : UserSet) {

‎test/SILOptimizer/sil_combine.sil

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1980,6 +1980,89 @@ bb3:
19801980
return %0 : $B
19811981
}
19821982

1983+
// CHECK-LABEL: sil @dont_delete_readonly_function_dealloc_ref_simple
1984+
// CHECK: apply
1985+
// CHECK-NEXT: dealloc_ref
1986+
// CHECK: return
1987+
sil @dont_delete_readonly_function_dealloc_ref_simple : $@convention(thin) () -> () {
1988+
bb0:
1989+
%3 = alloc_ref [stack] $B
1990+
%f = function_ref @readonly_owned : $@convention(thin) (@owned B) -> @owned B
1991+
%r = apply %f(%3) : $@convention(thin) (@owned B) -> @owned B
1992+
dealloc_ref [stack] %3 : $B
1993+
strong_release %r : $B
1994+
%10 = tuple ()
1995+
return %10 : $()
1996+
}
1997+
1998+
// CHECK-LABEL: sil @dont_delete_readonly_function_dealloc_ref_multibb
1999+
// CHECK: apply
2000+
// CHECK: dealloc_ref
2001+
// CHECK-NEXT: strong_release
2002+
// CHECK: dealloc_ref
2003+
// CHECK-NEXT: strong_release
2004+
// CHECK: return
2005+
sil @dont_delete_readonly_function_dealloc_ref_multibb : $@convention(thin) () -> () {
2006+
bb0:
2007+
%3 = alloc_ref [stack] $B
2008+
%f = function_ref @readonly_owned : $@convention(thin) (@owned B) -> @owned B
2009+
%r = apply %f(%3) : $@convention(thin) (@owned B) -> @owned B
2010+
cond_br undef, bb1, bb2
2011+
2012+
bb1:
2013+
dealloc_ref [stack] %3 : $B
2014+
strong_release %r : $B
2015+
br bb3
2016+
2017+
bb2:
2018+
dealloc_ref [stack] %3 : $B
2019+
strong_release %r : $B
2020+
br bb3
2021+
2022+
bb3:
2023+
%10 = tuple ()
2024+
return %10 : $()
2025+
}
2026+
2027+
// CHECK-LABEL: sil @delete_readonly_function_dealloc_ref_simple
2028+
// CHECK-NOT: apply
2029+
// CHECK: return
2030+
sil @delete_readonly_function_dealloc_ref_simple : $@convention(thin) () -> () {
2031+
bb0:
2032+
%3 = alloc_ref [stack] $B
2033+
%f = function_ref @readonly_owned : $@convention(thin) (@owned B) -> @owned B
2034+
%r = apply %f(%3) : $@convention(thin) (@owned B) -> @owned B
2035+
strong_release %r : $B
2036+
dealloc_ref [stack] %3 : $B
2037+
%10 = tuple ()
2038+
return %10 : $()
2039+
}
2040+
2041+
// CHECK-LABEL: sil @delete_readonly_function_dealloc_ref_multibb
2042+
// CHECK-NOT: apply
2043+
// CHECK: return
2044+
sil @delete_readonly_function_dealloc_ref_multibb : $@convention(thin) () -> () {
2045+
bb0:
2046+
%3 = alloc_ref [stack] $B
2047+
%f = function_ref @readonly_owned : $@convention(thin) (@owned B) -> @owned B
2048+
%r = apply %f(%3) : $@convention(thin) (@owned B) -> @owned B
2049+
cond_br undef, bb1, bb2
2050+
2051+
bb1:
2052+
strong_release %r : $B
2053+
dealloc_ref [stack] %3 : $B
2054+
br bb3
2055+
2056+
bb2:
2057+
strong_release %r : $B
2058+
dealloc_ref [stack] %3 : $B
2059+
br bb3
2060+
2061+
bb3:
2062+
%10 = tuple ()
2063+
return %10 : $()
2064+
}
2065+
19832066
//CHECK-LABEL: sil @test_delete_readonly_try_apply
19842067
//CHECK: bb0(%0 : $ZZZ):
19852068
//CHECK-NEXT: [[IL:%[0-9]+]] = integer_literal $Builtin.Int1, 0

‎test/SILOptimizer/silcombine_runtime_crash.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift -O %s -o %t/a.out
3+
// RUN: %target-run %t/a.out | %FileCheck %s
4+
5+
// REQUIRES: executable_test
6+
7+
// This is an end-to-end test for SR-9627.
8+
9+
@inline(never)
10+
func save(value: Double?) {
11+
var params: [[String : Any]] = [["a": 0]]
12+
params = [[
13+
"b": 0,
14+
"c": value.map({ String.init($0) }) as Any
15+
]]
16+
}
17+
18+
save(value: 0)
19+
20+
// CHECK: ok
21+
print("ok")

0 commit comments

Comments
 (0)