Skip to content

Commit 233eeea

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 df0a8b3 commit 233eeea

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
@@ -2230,6 +2230,32 @@ static SILValue testControlVariableBit(SILLocation Loc,
22302230
{}, CondVal);
22312231
}
22322232

2233+
/// Test if all bits in the control variable are set at the current
2234+
/// insertion point.
2235+
static SILValue testAllControlVariableBits(SILLocation Loc,
2236+
SILValue ControlVariableAddr,
2237+
Identifier &CmpEqFn,
2238+
SILBuilder &B) {
2239+
SILValue ControlVariable =
2240+
B.createLoad(Loc, ControlVariableAddr, LoadOwnershipQualifier::Trivial);
2241+
2242+
SILValue CondVal = ControlVariable;
2243+
CanBuiltinIntegerType IVType = CondVal->getType().castTo<BuiltinIntegerType>();
2244+
2245+
if (IVType->getFixedWidth() == 1)
2246+
return CondVal;
2247+
2248+
SILValue AllBitsSet = B.createIntegerLiteral(Loc, CondVal->getType(), -1);
2249+
if (!CmpEqFn.get())
2250+
CmpEqFn = getBinaryFunction("cmp_eq", CondVal->getType(),
2251+
B.getASTContext());
2252+
SILValue Args[] = { CondVal, AllBitsSet };
2253+
2254+
return B.createBuiltin(Loc, CmpEqFn,
2255+
SILType::getBuiltinIntegerType(1, B.getASTContext()),
2256+
{}, Args);
2257+
}
2258+
22332259
/// handleConditionalInitAssign - This memory object has some stores
22342260
/// into (some element of) it that is either an init or an assign based on the
22352261
/// control flow path through the function, or have a destroy event that happens
@@ -2407,7 +2433,7 @@ SILValue LifetimeChecker::handleConditionalInitAssign() {
24072433
void LifetimeChecker::
24082434
handleConditionalDestroys(SILValue ControlVariableAddr) {
24092435
SILBuilderWithScope B(TheMemory.getUninitializedValue());
2410-
Identifier ShiftRightFn, TruncateFn;
2436+
Identifier ShiftRightFn, TruncateFn, CmpEqFn;
24112437

24122438
unsigned NumMemoryElements = TheMemory.getNumElements();
24132439

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

2540-
// The original strong_release or destroy_addr instruction is
2541-
// always dead at this point.
2542-
deleteDeadRelease(CDElt.ReleaseID);
25432607
continue;
25442608
}
25452609

‎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)