Skip to content

Commit 1897745

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 cf97c2d commit 1897745

File tree

3 files changed

+86
-0
lines changed

3 files changed

+86
-0
lines changed

lib/SILOptimizer/Transforms/DeadCodeElimination.cpp

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

207207
void markValueLive(SILValue V);
208208
void markInstructionLive(SILInstruction *Inst);
209+
void markOwnedDeadValueLive(SILValue v);
209210
void markTerminatorArgsLive(SILBasicBlock *Pred, SILBasicBlock *Succ,
210211
size_t ArgIndex);
211212
void markControllingTerminatorsLive(SILBasicBlock *Block);
@@ -268,6 +269,20 @@ void DCE::markInstructionLive(SILInstruction *Inst) {
268269
Worklist.push_back(Inst);
269270
}
270271

272+
void DCE::markOwnedDeadValueLive(SILValue v) {
273+
if (v->getOwnershipKind() == OwnershipKind::Owned) {
274+
// When an owned value has no lifetime ending uses it means that it is in a
275+
// dead-end region. We must not remove and inserting compensating destroys
276+
// for it because that would potentially destroy the value too early.
277+
// TODO: we can remove this once we have complete OSSA lifetimes
278+
for (Operand *use : v->getUses()) {
279+
if (use->isLifetimeEnding())
280+
return;
281+
}
282+
markValueLive(v);
283+
}
284+
}
285+
271286
/// Gets the producing instruction of a cond_fail condition. Currently these
272287
/// are overflow builtins but may be extended to other instructions in the
273288
/// future.
@@ -334,6 +349,9 @@ void DCE::markLive() {
334349
// to be live in the sense that they are not trivially something we
335350
// can delete by examining only that instruction.
336351
for (auto &BB : *F) {
352+
for (SILArgument *arg : BB.getArguments()) {
353+
markOwnedDeadValueLive(arg);
354+
}
337355
for (auto &I : BB) {
338356
switch (I.getKind()) {
339357
case SILInstructionKind::CondFailInst: {
@@ -414,6 +432,9 @@ void DCE::markLive() {
414432
default:
415433
if (seemsUseful(&I))
416434
markInstructionLive(&I);
435+
for (SILValue result : I.getResults()) {
436+
markOwnedDeadValueLive(result);
437+
}
417438
}
418439
}
419440
}

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

0 commit comments

Comments
 (0)