Skip to content

Commit ed0d939

Browse files
slavapestovslavapestov
committed
DI: Correctly handle conditional destroy of 'self' in root class
In a designated initializer of a non-root class, 'self' becomes fully initialized after the 'super.init' call, at which point escaping uses of 'self' become valid, and releases of 'self' are lowered to a 'strong_release' instruction, which runs the deinitializer. In a root class, 'self' becomes fully initialized after all stored properties have been initialized, at which point escaping uses of 'self' become valid. However, DI would still lower a conditional destroy of 'self' by destroying any initialized stored properties and freeing the object with 'dealloc_partial_ref'. This is incorrect, because 'self' may have escaped. In the non-conditional destroy case, we correctly lowered the release to a 'strong_release' if all stored properties are known to be initialized. Fix DI to handle the conditional destroy case by first checking if all bits in the control variable are set, and releasing the instance with 'strong_release' if so. The 'dealloc_partial_ref' is only emitted if not at least one stored property was not initialized. This ensures that we don't deallocate an instance that may have escaped. Fixes <https://bugs.swift.org/browse/SR-13439>, <rdar://problem/67746791>, <https://bugs.swift.org/browse/SR-13355>, <rdar://problem/67361228>.
1 parent 89f688f commit ed0d939

File tree

3 files changed

+460
-6
lines changed

3 files changed

+460
-6
lines changed

‎lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

Lines changed: 70 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2221,6 +2221,32 @@ static SILValue testControlVariableBit(SILLocation Loc,
22212221
{}, CondVal);
22222222
}
22232223

2224+
/// Test if all bits in the control variable are set at the current
2225+
/// insertion point.
2226+
static SILValue testAllControlVariableBits(SILLocation Loc,
2227+
SILValue ControlVariableAddr,
2228+
Identifier &CmpEqFn,
2229+
SILBuilder &B) {
2230+
SILValue ControlVariable =
2231+
B.createLoad(Loc, ControlVariableAddr, LoadOwnershipQualifier::Trivial);
2232+
2233+
SILValue CondVal = ControlVariable;
2234+
CanBuiltinIntegerType IVType = CondVal->getType().castTo<BuiltinIntegerType>();
2235+
2236+
if (IVType->getFixedWidth() == 1)
2237+
return CondVal;
2238+
2239+
SILValue AllBitsSet = B.createIntegerLiteral(Loc, CondVal->getType(), -1);
2240+
if (!CmpEqFn.get())
2241+
CmpEqFn = getBinaryFunction("cmp_eq", CondVal->getType(),
2242+
B.getASTContext());
2243+
SILValue Args[] = { CondVal, AllBitsSet };
2244+
2245+
return B.createBuiltin(Loc, CmpEqFn,
2246+
SILType::getBuiltinIntegerType(1, B.getASTContext()),
2247+
{}, Args);
2248+
}
2249+
22242250
/// handleConditionalInitAssign - This memory object has some stores
22252251
/// into (some element of) it that is either an init or an assign based on the
22262252
/// control flow path through the function, or have a destroy event that happens
@@ -2398,7 +2424,7 @@ SILValue LifetimeChecker::handleConditionalInitAssign() {
23982424
void LifetimeChecker::
23992425
handleConditionalDestroys(SILValue ControlVariableAddr) {
24002426
SILBuilderWithScope B(TheMemory.getUninitializedValue());
2401-
Identifier ShiftRightFn, TruncateFn;
2427+
Identifier ShiftRightFn, TruncateFn, CmpEqFn;
24022428

24032429
unsigned NumMemoryElements = TheMemory.getNumElements();
24042430

@@ -2525,12 +2551,50 @@ handleConditionalDestroys(SILValue ControlVariableAddr) {
25252551
// Just conditionally destroy each memory element, and for classes,
25262552
// also free the partially initialized object.
25272553
if (!TheMemory.isNonRootClassSelf()) {
2528-
destroyMemoryElements(Loc, Availability);
2529-
processUninitializedRelease(Release, false, B.getInsertionPoint());
2554+
assert(!Availability.isAllYes() &&
2555+
"Should not end up here if fully initialized");
2556+
2557+
// For root class initializers, we check if all proeprties were
2558+
// dynamically initialized, and if so, treat this as a release of
2559+
// an initialized 'self', instead of tearing down the fields
2560+
// one by one and deallocating memory.
2561+
//
2562+
// This is required for correctness, since the condition that
2563+
// allows 'self' to escape is that all stored properties were
2564+
// initialized. So we cannot deallocate the memory if 'self' may
2565+
// have escaped.
2566+
//
2567+
// This also means the deinitializer will run if all stored
2568+
// properties were initialized.
2569+
if (TheMemory.isClassInitSelf() &&
2570+
Availability.hasAny(DIKind::Partial)) {
2571+
auto CondVal = testAllControlVariableBits(Loc, ControlVariableAddr,
2572+
CmpEqFn, B);
2573+
2574+
SILBasicBlock *ReleaseBlock, *DeallocBlock, *ContBlock;
2575+
2576+
InsertCFGDiamond(CondVal, Loc, B,
2577+
ReleaseBlock, DeallocBlock, ContBlock);
2578+
2579+
// If true, self was fully initialized and must be released.
2580+
B.setInsertionPoint(ReleaseBlock->begin());
2581+
B.setCurrentDebugScope(ReleaseBlock->begin()->getDebugScope());
2582+
Release->moveBefore(&*B.getInsertionPoint());
2583+
2584+
// If false, self is uninitialized and must be freed.
2585+
B.setInsertionPoint(DeallocBlock->begin());
2586+
B.setCurrentDebugScope(DeallocBlock->begin()->getDebugScope());
2587+
destroyMemoryElements(Loc, Availability);
2588+
processUninitializedRelease(Release, false, B.getInsertionPoint());
2589+
} else {
2590+
destroyMemoryElements(Loc, Availability);
2591+
processUninitializedRelease(Release, false, B.getInsertionPoint());
2592+
2593+
// The original strong_release or destroy_addr instruction is
2594+
// always dead at this point.
2595+
deleteDeadRelease(CDElt.ReleaseID);
2596+
}
25302597

2531-
// The original strong_release or destroy_addr instruction is
2532-
// always dead at this point.
2533-
deleteDeadRelease(CDElt.ReleaseID);
25342598
continue;
25352599
}
25362600

‎test/Interpreter/failable_initializers_root_class.swift

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
// RUN: %target-run-simple-swift
2+
3+
// REQUIRES: executable_test
4+
5+
import StdlibUnittest
6+
7+
8+
var FailableInitTestSuite = TestSuite("FailableInit")
9+
10+
var deinitCalled = 0
11+
12+
func mustFail<T>(f: () -> T?) {
13+
if f() != nil {
14+
preconditionFailure("Didn't fail")
15+
}
16+
}
17+
18+
func mustSucceed<T>(f: () -> T?) {
19+
if f() == nil {
20+
preconditionFailure("Didn't succeed")
21+
}
22+
}
23+
24+
class FirstClass {
25+
var x: LifetimeTracked
26+
27+
init?(n: Int) {
28+
if n == 0 {
29+
return nil
30+
}
31+
32+
x = LifetimeTracked(0)
33+
34+
if n == 1 {
35+
return nil
36+
}
37+
}
38+
39+
deinit {
40+
deinitCalled += 1
41+
}
42+
}
43+
44+
FailableInitTestSuite.test("FirstClass") {
45+
deinitCalled = 0
46+
47+
mustFail { FirstClass(n: 0) }
48+
expectEqual(0, deinitCalled)
49+
50+
mustFail { FirstClass(n: 1) }
51+
expectEqual(1, deinitCalled)
52+
53+
mustSucceed { FirstClass(n: 2) }
54+
expectEqual(2, deinitCalled)
55+
}
56+
57+
class FirstClassTrivial {
58+
var x: Int
59+
60+
init?(n: Int) {
61+
if n == 0 {
62+
return nil
63+
}
64+
65+
x = 0
66+
67+
if n == 1 {
68+
return nil
69+
}
70+
}
71+
72+
deinit {
73+
deinitCalled += 1
74+
}
75+
}
76+
77+
FailableInitTestSuite.test("FirstClassTrivial") {
78+
deinitCalled = 0
79+
80+
mustFail { FirstClassTrivial(n: 0) }
81+
expectEqual(0, deinitCalled)
82+
83+
mustFail { FirstClassTrivial(n: 1) }
84+
expectEqual(1, deinitCalled)
85+
86+
mustSucceed { FirstClassTrivial(n: 2) }
87+
expectEqual(2, deinitCalled)
88+
}
89+
90+
class SecondClass {
91+
var x: LifetimeTracked
92+
var y: LifetimeTracked
93+
94+
init?(n: Int) {
95+
if n == 0 {
96+
return nil
97+
}
98+
99+
x = LifetimeTracked(0)
100+
101+
if n == 1 {
102+
return nil
103+
}
104+
105+
y = LifetimeTracked(0)
106+
107+
if n == 2 {
108+
return nil
109+
}
110+
}
111+
112+
deinit {
113+
deinitCalled += 1
114+
}
115+
}
116+
117+
FailableInitTestSuite.test("SecondClass") {
118+
deinitCalled = 0
119+
120+
mustFail { SecondClass(n: 0) }
121+
expectEqual(0, deinitCalled)
122+
123+
mustFail { SecondClass(n: 1) }
124+
expectEqual(0, deinitCalled)
125+
126+
mustFail { SecondClass(n: 2) }
127+
expectEqual(1, deinitCalled)
128+
129+
mustSucceed { SecondClass(n: 3) }
130+
expectEqual(2, deinitCalled)
131+
}
132+
133+
class SecondClassTrivial {
134+
var x: Int
135+
var y: Int
136+
137+
init?(n: Int) {
138+
if n == 0 {
139+
return nil
140+
}
141+
142+
x = 0
143+
144+
if n == 1 {
145+
return nil
146+
}
147+
148+
y = 0
149+
150+
if n == 2 {
151+
return nil
152+
}
153+
}
154+
155+
deinit {
156+
deinitCalled += 1
157+
}
158+
}
159+
160+
FailableInitTestSuite.test("SecondClassTrivial") {
161+
deinitCalled = 0
162+
163+
mustFail { SecondClassTrivial(n: 0) }
164+
expectEqual(0, deinitCalled)
165+
166+
mustFail { SecondClassTrivial(n: 1) }
167+
expectEqual(0, deinitCalled)
168+
169+
mustFail { SecondClassTrivial(n: 2) }
170+
expectEqual(1, deinitCalled)
171+
172+
mustSucceed { SecondClassTrivial(n: 3) }
173+
expectEqual(2, deinitCalled)
174+
}
175+
176+
runAllTests()

0 commit comments

Comments
 (0)