Skip to content

Commit 61546ab

Browse files
committed
DeadCodeElimination: don't insert destroys for removed values in dead-end blocks
When an owned value has no lifetime ending uses it means that it is in a dead-end region. We must not remove and inserting compensating destroys for it because that would potentially destroy the value too early. Initialization of an object might be cut off and removed after a dead-end loop or an `unreachable`. In this case a class destructor would see uninitialized fields. Fixes a mis-compile #85851 rdar://165876726
1 parent 7e590fd commit 61546ab

File tree

3 files changed

+82
-0
lines changed

3 files changed

+82
-0
lines changed

lib/SILOptimizer/Transforms/DeadCodeElimination.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ class DCE {
203203

204204
void markValueLive(SILValue V);
205205
void markInstructionLive(SILInstruction *Inst);
206+
void markOwnedDeadValueLive(SILValue v);
206207
void markTerminatorArgsLive(SILBasicBlock *Pred, SILBasicBlock *Succ,
207208
size_t ArgIndex);
208209
void markControllingTerminatorsLive(SILBasicBlock *Block);
@@ -265,6 +266,20 @@ void DCE::markInstructionLive(SILInstruction *Inst) {
265266
Worklist.push_back(Inst);
266267
}
267268

269+
void DCE::markOwnedDeadValueLive(SILValue v) {
270+
if (v->getOwnershipKind() == OwnershipKind::Owned) {
271+
// When an owned value has no lifetime ending uses it means that it is in a
272+
// dead-end region. We must not remove and inserting compensating destroys
273+
// for it because that would potentially destroy the value too early.
274+
// TODO: we can remove this once we have complete OSSA lifetimes
275+
for (Operand *use : v->getUses()) {
276+
if (use->isLifetimeEnding())
277+
return;
278+
}
279+
markValueLive(v);
280+
}
281+
}
282+
268283
/// Gets the producing instruction of a cond_fail condition. Currently these
269284
/// are overflow builtins but may be extended to other instructions in the
270285
/// future.
@@ -331,6 +346,9 @@ void DCE::markLive() {
331346
// to be live in the sense that they are not trivially something we
332347
// can delete by examining only that instruction.
333348
for (auto &BB : *F) {
349+
for (SILArgument *arg : BB.getArguments()) {
350+
markOwnedDeadValueLive(arg);
351+
}
334352
for (auto &I : BB) {
335353
switch (I.getKind()) {
336354
case SILInstructionKind::CondFailInst: {
@@ -395,6 +413,9 @@ void DCE::markLive() {
395413
default:
396414
if (seemsUseful(&I))
397415
markInstructionLive(&I);
416+
for (SILValue result : I.getResults()) {
417+
markOwnedDeadValueLive(result);
418+
}
398419
}
399420
}
400421
}

test/SILOptimizer/dead_code_elimination_ossa.sil

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ public struct S {
1212
typealias Int1 = Builtin.Int1
1313

1414
class C {}
15+
class D: C {}
1516

1617
sil @getC : $@convention(thin) () -> @owned C
1718
sil @barrier : $@convention(thin) () -> ()
@@ -534,3 +535,32 @@ bb2:
534535
%17 = tuple ()
535536
return %17
536537
}
538+
539+
// CHECK-LABEL: sil [ossa] @dont_insert_destroy_for_uninitialized_object :
540+
// CHECK-NOT: destroy_value
541+
// CHECK-LABEL: } // end sil function 'dont_insert_destroy_for_uninitialized_object'
542+
sil [ossa] @dont_insert_destroy_for_uninitialized_object : $@convention(thin) () -> () {
543+
bb0:
544+
%0 = alloc_ref $D // D might not be initialized due to dead-end loop -> will lead to crash if it is destroyed here
545+
%1 = upcast %0 to $C
546+
br bb1
547+
548+
bb1:
549+
br bb1
550+
}
551+
552+
// CHECK-LABEL: sil [ossa] @dont_insert_arg_destroy_for_uninitialized_object :
553+
// CHECK-NOT: destroy_value
554+
// CHECK-LABEL: } // end sil function 'dont_insert_arg_destroy_for_uninitialized_object'
555+
sil [ossa] @dont_insert_arg_destroy_for_uninitialized_object : $@convention(thin) () -> () {
556+
bb0:
557+
%0 = alloc_ref $D // D might not be initialized due to dead-end loop -> will lead to crash if it is destroyed here
558+
br bb1(%0)
559+
560+
bb1(%2 : @owned $D):
561+
br bb2
562+
563+
bb2:
564+
br bb2
565+
}
566+
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %target-run-simple-swift(-O -Xllvm -sil-disable-pass=deadobject-elim) | %FileCheck %s
2+
3+
// REQUIRES: executable_test
4+
5+
#if os(macOS)
6+
import Darwin
7+
#elseif canImport(Glibc)
8+
import Glibc
9+
#elseif canImport(Musl)
10+
import Musl
11+
#elseif canImport(Android)
12+
import Android
13+
#endif
14+
15+
@inline(never)
16+
public func hiddenExit() {
17+
// CHECK: okay
18+
print("okay")
19+
exit(0)
20+
}
21+
22+
func foo() -> Never {
23+
while true {
24+
hiddenExit()
25+
}
26+
}
27+
28+
func bar(_ x: Any...) {
29+
}
30+
31+
bar(foo())

0 commit comments

Comments
 (0)