Skip to content

Commit d3f0448

Browse files
atrickairspeedswift
atrick
authored and
airspeedswift
committed
Fix EscapeAnalysis losing precision during merge. (#29277)
Array storage was being stack promoted even though it escaped. This happened because multiple locally allocated arrays were merged into the same locally allocated array value box. For this to become a problem, other bizarre merge events need to take place, such as a value node being mapped with a content node. The series of events led to a missing edge in the connection graph. One of the arrays was mapped directly to a project_box instruction which had forgotten it's relationship with the alloc_box. It is a trivial one line fix to simply preserve all value mappings. <rdar://58371330> app crashes (miscompile) (cherry picked from commit fbe38ce)
1 parent f03b92d commit d3f0448

File tree

2 files changed

+222
-3
lines changed

2 files changed

+222
-3
lines changed

‎lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -729,9 +729,13 @@ void EscapeAnalysis::ConnectionGraph::mergeAllScheduledNodes() {
729729
From->isMerged = true;
730730

731731
if (From->mappedValue) {
732-
if (To->mappedValue)
733-
Values2Nodes.erase(From->mappedValue);
734-
else {
732+
// If possible, transfer 'From's mappedValue to 'To' for clarity. Any
733+
// values previously mapped to 'From' but not transferred to 'To's
734+
// mappedValue must remain mapped to 'From'. Lookups on those values will
735+
// find 'To' via the mergeTarget. Dropping a value's mapping is illegal
736+
// because it could cause a node to be recreated without the edges that
737+
// have already been discovered.
738+
if (!To->mappedValue) {
735739
To->mappedValue = From->mappedValue;
736740
Values2Nodes[To->mappedValue] = To;
737741
}

‎test/SILOptimizer/escape_analysis_reduced.sil

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
// RUN: %target-sil-opt %s -escapes-dump -o /dev/null | %FileCheck %s
2+
3+
// REQUIRES: asserts
4+
// REQUIRES: OS=macosx
5+
// REQUIRES: PTRSIZE=64
6+
7+
sil_stage canonical
8+
9+
import Builtin
10+
import Swift
11+
import SwiftShims
12+
13+
// =============================================================================
14+
// Test call to array.uninitialized that has extra release_value uses
15+
16+
class C {
17+
var c: C
18+
}
19+
20+
class DummyArrayStorage<Element> {
21+
@_hasStorage var count: Int { get }
22+
@_hasStorage var capacity: Int { get }
23+
init()
24+
}
25+
26+
// init_any_array_with_buffer
27+
sil [_semantics "array.uninitialized"] @init_any_array_with_buffer : $@convention(thin) (@owned DummyArrayStorage<AnyObject>, Int32, @thin Array<AnyObject>.Type) -> (@owned Array<AnyObject>, UnsafeMutablePointer<AnyObject>)
28+
29+
// CHECK-LABEL: CG of testBadArrayUninit
30+
// CHECK-NEXT: Val %2 Esc: G, Succ: (%2.1)
31+
// CHECK-NEXT: Con %2.1 Esc: G, Succ:
32+
// CHECK-NEXT: Val %5 Esc: G, Succ: (%7)
33+
// CHECK-NEXT: Con %7 Esc: G, Succ: (%7.1)
34+
// CHECK-NEXT: Con %7.1 Esc: G, Succ: %10
35+
// CHECK-NEXT: Val %10 Esc: G, Succ:
36+
// CHECK-LABEL: End
37+
sil hidden @testBadArrayUninit : $@convention(thin) (Builtin.Word, Int32) -> () {
38+
bb0(%0 : $Builtin.Word, %1 : $Int32):
39+
// create an array
40+
%2 = alloc_ref [tail_elems $AnyObject * %0 : $Builtin.Word] $DummyArrayStorage<AnyObject>
41+
%3 = metatype $@thin Array<AnyObject>.Type
42+
%4 = function_ref @init_any_array_with_buffer : $@convention(thin) (@owned DummyArrayStorage<AnyObject>, Int32, @thin Array<AnyObject>.Type) -> (@owned Array<AnyObject>, UnsafeMutablePointer<AnyObject>)
43+
%5 = apply %4(%2, %1, %3) : $@convention(thin) (@owned DummyArrayStorage<AnyObject>, Int32, @thin Array<AnyObject>.Type) -> (@owned Array<AnyObject>, UnsafeMutablePointer<AnyObject>)
44+
%6 = tuple_extract %5 : $(Array<AnyObject>, UnsafeMutablePointer<AnyObject>), 0
45+
%7 = tuple_extract %5 : $(Array<AnyObject>, UnsafeMutablePointer<AnyObject>), 1
46+
%8 = struct_extract %7 : $UnsafeMutablePointer<AnyObject>, #UnsafeMutablePointer._rawValue
47+
%9 = pointer_to_address %8 : $Builtin.RawPointer to [strict] $*AnyObject
48+
49+
// store an elt
50+
%10 = alloc_ref $C
51+
%11 = init_existential_ref %10 : $C : $C, $AnyObject
52+
store %11 to %9 : $*AnyObject
53+
54+
// extra use of the call
55+
release_value %5 : $(Array<AnyObject>, UnsafeMutablePointer<AnyObject>) // id: %228
56+
%13 = tuple ()
57+
return %13 : $()
58+
}
59+
60+
// =============================================================================
61+
// testArrayEscapeToBox: test that an array is marked escaping when
62+
// assigned to a box. When multiple arrays are merged into the same
63+
// box, ensure that a previous mapping from the project_box address to
64+
// the box's content is not lost during the merge.
65+
66+
class ElementClass {
67+
init()
68+
}
69+
70+
class StagedContext {
71+
init()
72+
}
73+
74+
class VFSStagedContext : StagedContext {
75+
override init()
76+
}
77+
78+
// specialized Array.init()
79+
sil @$sS2ayxGycfCSo12ElementClassC_Tg5 : $@convention(method) (@thin Array<ElementClass>.Type) -> @owned Array<ElementClass>
80+
81+
// specialized Array._getCount()
82+
sil @$sSa9_getCountSiyFSo12ElementClassC_Tg5 : $@convention(method) (@guaranteed Array<ElementClass>) -> Int
83+
84+
// specialized static Array._adoptStorage(_:count:)
85+
sil shared [_semantics "array.uninitialized"] @$sSa13_adoptStorage_5countSayxG_SpyxGts016_ContiguousArrayB0CyxGn_SitFZSo12ElementClassC_Tg5 : $@convention(method) (@owned _ContiguousArrayStorage<ElementClass>, Int, @thin Array<ElementClass>.Type) -> (@owned Array<ElementClass>, UnsafeMutablePointer<ElementClass>) {
86+
// %0 // users: %13, %3
87+
// %1 // users: %9, %4
88+
bb0(%0 : $_ContiguousArrayStorage<ElementClass>, %1 : $Int, %2 : $@thin Array<ElementClass>.Type):
89+
%3 = upcast %0 : $_ContiguousArrayStorage<ElementClass> to $__ContiguousArrayStorageBase // users: %17, %11
90+
%4 = struct_extract %1 : $Int, #Int._value // user: %6
91+
%5 = integer_literal $Builtin.Int64, 1 // users: %7, %6
92+
%6 = builtin "shl_Int64"(%4 : $Builtin.Int64, %5 : $Builtin.Int64) : $Builtin.Int64 // user: %7
93+
%7 = builtin "or_Int64"(%6 : $Builtin.Int64, %5 : $Builtin.Int64) : $Builtin.Int64 // user: %8
94+
%8 = struct $UInt (%7 : $Builtin.Int64) // user: %9
95+
%9 = struct $_SwiftArrayBodyStorage (%1 : $Int, %8 : $UInt) // user: %10
96+
%10 = struct $_ArrayBody (%9 : $_SwiftArrayBodyStorage) // user: %12
97+
%11 = ref_element_addr %3 : $__ContiguousArrayStorageBase, #__ContiguousArrayStorageBase.countAndCapacity // user: %12
98+
store %10 to %11 : $*_ArrayBody // id: %12
99+
%13 = unchecked_ref_cast %0 : $_ContiguousArrayStorage<ElementClass> to $Builtin.BridgeObject // user: %14
100+
%14 = struct $_BridgeStorage<__ContiguousArrayStorageBase> (%13 : $Builtin.BridgeObject) // user: %15
101+
%15 = struct $_ArrayBuffer<ElementClass> (%14 : $_BridgeStorage<__ContiguousArrayStorageBase>) // user: %16
102+
%16 = struct $Array<ElementClass> (%15 : $_ArrayBuffer<ElementClass>) // user: %20
103+
%17 = ref_tail_addr %3 : $__ContiguousArrayStorageBase, $ElementClass // user: %18
104+
%18 = address_to_pointer %17 : $*ElementClass to $Builtin.RawPointer // user: %19
105+
%19 = struct $UnsafeMutablePointer<ElementClass> (%18 : $Builtin.RawPointer) // user: %20
106+
%20 = tuple (%16 : $Array<ElementClass>, %19 : $UnsafeMutablePointer<ElementClass>) // user: %21
107+
return %20 : $(Array<ElementClass>, UnsafeMutablePointer<ElementClass>) // id: %21
108+
}
109+
110+
// testArrayUsePointsClosure1
111+
sil @testArrayUsePointsClosure1 : $@convention(thin) (@guaranteed { var Optional<VFSStagedContext> }, @guaranteed @callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> ()) -> ()
112+
113+
// testArrayUsePointsClosure2
114+
sil @testArrayUsePointsClosure2 : $@convention(thin) (@guaranteed @callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> (), @guaranteed Optional<VFSStagedContext>, @guaranteed { var Array<ElementClass> }) -> ()
115+
116+
// Make sure both locally allocated array's are globally escaping.
117+
//
118+
// CHECK-LABEL: CG of testArrayEscapeToBox
119+
// CHECK-NEXT: Arg %0 Esc: A, Succ: (%21)
120+
// CHECK-NEXT: Val %1 Esc: %59, Succ: (%21)
121+
// CHECK-NEXT: Val %4 Esc: , Succ: %0, %1
122+
// CHECK-NEXT: Val %8 Esc: %56,%57,%58, Succ: (%21)
123+
// CHECK-NEXT: Val %12 Esc: G, Succ: (%21)
124+
// CHECK-NEXT: Val %17 Esc: G, Succ: (%39)
125+
// CHECK-NEXT: Val %19 Esc: G, Succ: (%21)
126+
// CHECK-NEXT: Con %21 Esc: G, Succ: %17, %31
127+
// CHECK-NEXT: Val %31 Esc: G, Succ: (%39)
128+
// CHECK-NEXT: Val %33 Esc: G, Succ: (%21)
129+
// CHECK-NEXT: Con %39 Esc: G, Succ: (%21), %12, %19, %33
130+
// CHECK-NEXT: Val %45 Esc: %57, Succ: %0, %8, %39
131+
// CHECK-NEXT: End
132+
sil private @testArrayEscapeToBox : $@convention(thin) (@guaranteed @callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> ()) -> () {
133+
// %0 // users: %54, %51, %47, %45, %5, %4
134+
bb0(%0 : $@callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> ()):
135+
%1 = alloc_box ${ var Optional<VFSStagedContext> }, var, name "context" // users: %59, %6, %4, %2
136+
%2 = project_box %1 : ${ var Optional<VFSStagedContext> }, 0 // user: %44
137+
// function_ref testArrayUsePointsClosure1
138+
%3 = function_ref @testArrayUsePointsClosure1 : $@convention(thin) (@guaranteed { var Optional<VFSStagedContext> }, @guaranteed @callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> ()) -> () // user: %4
139+
%4 = partial_apply [callee_guaranteed] %3(%1, %0) : $@convention(thin) (@guaranteed { var Optional<VFSStagedContext> }, @guaranteed @callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> ()) -> ()
140+
strong_retain %0 : $@callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> () // id: %5
141+
strong_retain %1 : ${ var Optional<VFSStagedContext> } // id: %6
142+
br bb1 // id: %7
143+
144+
bb1: // Preds: bb0
145+
%8 = alloc_box ${ var Array<ElementClass> }, var, name "intents" // users: %58, %56, %52, %48, %45, %9
146+
%9 = project_box %8 : ${ var Array<ElementClass> }, 0 // users: %41, %36, %27, %22, %13
147+
%10 = metatype $@thin Array<ElementClass>.Type // users: %33, %19, %12
148+
// function_ref specialized Array.init()
149+
%11 = function_ref @$sS2ayxGycfCSo12ElementClassC_Tg5 : $@convention(method) (@thin Array<ElementClass>.Type) -> @owned Array<ElementClass> // user: %12
150+
%12 = apply %11(%10) : $@convention(method) (@thin Array<ElementClass>.Type) -> @owned Array<ElementClass> // user: %13
151+
store %12 to %9 : $*Array<ElementClass> // id: %13
152+
cond_br undef, bb2, bb3 // id: %14
153+
154+
bb2: // Preds: bb1
155+
%15 = integer_literal $Builtin.Int64, 1 // user: %16
156+
%16 = struct $Int (%15 : $Builtin.Int64) // user: %19
157+
%17 = alloc_ref [tail_elems $ElementClass * undef : $Builtin.Word] $_ContiguousArrayStorage<ElementClass> // user: %19
158+
// function_ref specialized static Array._adoptStorage(_:count:)
159+
%18 = function_ref @$sSa13_adoptStorage_5countSayxG_SpyxGts016_ContiguousArrayB0CyxGn_SitFZSo12ElementClassC_Tg5 : $@convention(method) (@owned _ContiguousArrayStorage<ElementClass>, Int, @thin Array<ElementClass>.Type) -> (@owned Array<ElementClass>, UnsafeMutablePointer<ElementClass>) // user: %19
160+
%19 = apply %18(%17, %16, %10) : $@convention(method) (@owned _ContiguousArrayStorage<ElementClass>, Int, @thin Array<ElementClass>.Type) -> (@owned Array<ElementClass>, UnsafeMutablePointer<ElementClass>) // users: %21, %20
161+
%20 = tuple_extract %19 : $(Array<ElementClass>, UnsafeMutablePointer<ElementClass>), 0 // user: %27
162+
%21 = tuple_extract %19 : $(Array<ElementClass>, UnsafeMutablePointer<ElementClass>), 1
163+
%22 = struct_element_addr %9 : $*Array<ElementClass>, #Array._buffer // user: %23
164+
%23 = struct_element_addr %22 : $*_ArrayBuffer<ElementClass>, #_ArrayBuffer._storage // user: %24
165+
%24 = struct_element_addr %23 : $*_BridgeStorage<__ContiguousArrayStorageBase>, #_BridgeStorage.rawValue // user: %25
166+
%25 = load %24 : $*Builtin.BridgeObject // user: %26
167+
strong_release %25 : $Builtin.BridgeObject // id: %26
168+
store %20 to %9 : $*Array<ElementClass> // id: %27
169+
br bb4 // id: %28
170+
171+
bb3: // Preds: bb1
172+
%29 = integer_literal $Builtin.Int64, 1 // user: %30
173+
%30 = struct $Int (%29 : $Builtin.Int64) // user: %33
174+
%31 = alloc_ref [tail_elems $ElementClass * undef : $Builtin.Word] $_ContiguousArrayStorage<ElementClass> // user: %33
175+
// function_ref specialized static Array._adoptStorage(_:count:)
176+
%32 = function_ref @$sSa13_adoptStorage_5countSayxG_SpyxGts016_ContiguousArrayB0CyxGn_SitFZSo12ElementClassC_Tg5 : $@convention(method) (@owned _ContiguousArrayStorage<ElementClass>, Int, @thin Array<ElementClass>.Type) -> (@owned Array<ElementClass>, UnsafeMutablePointer<ElementClass>) // user: %33
177+
%33 = apply %32(%31, %30, %10) : $@convention(method) (@owned _ContiguousArrayStorage<ElementClass>, Int, @thin Array<ElementClass>.Type) -> (@owned Array<ElementClass>, UnsafeMutablePointer<ElementClass>) // users: %35, %34
178+
%34 = tuple_extract %33 : $(Array<ElementClass>, UnsafeMutablePointer<ElementClass>), 0 // user: %41
179+
%35 = tuple_extract %33 : $(Array<ElementClass>, UnsafeMutablePointer<ElementClass>), 1
180+
%36 = struct_element_addr %9 : $*Array<ElementClass>, #Array._buffer // user: %37
181+
%37 = struct_element_addr %36 : $*_ArrayBuffer<ElementClass>, #_ArrayBuffer._storage // user: %38
182+
%38 = struct_element_addr %37 : $*_BridgeStorage<__ContiguousArrayStorageBase>, #_BridgeStorage.rawValue // user: %39
183+
%39 = load %38 : $*Builtin.BridgeObject // user: %40
184+
strong_release %39 : $Builtin.BridgeObject // id: %40
185+
store %34 to %9 : $*Array<ElementClass> // id: %41
186+
br bb4 // id: %42
187+
188+
bb4: // Preds: bb3 bb2
189+
// function_ref testArrayUsePointsClosure2
190+
%43 = function_ref @testArrayUsePointsClosure2 : $@convention(thin) (@guaranteed @callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> (), @guaranteed Optional<VFSStagedContext>, @guaranteed { var Array<ElementClass> }) -> () // user: %45
191+
%44 = load %2 : $*Optional<VFSStagedContext> // users: %55, %53, %49, %45
192+
%45 = partial_apply [callee_guaranteed] %43(%0, %44, %8) : $@convention(thin) (@guaranteed @callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> (), @guaranteed Optional<VFSStagedContext>, @guaranteed { var Array<ElementClass> }) -> () // user: %57
193+
// function_ref specialized Array._getCount()
194+
%46 = function_ref @$sSa9_getCountSiyFSo12ElementClassC_Tg5 : $@convention(method) (@guaranteed Array<ElementClass>) -> Int
195+
strong_retain %0 : $@callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> () // id: %47
196+
strong_retain %8 : ${ var Array<ElementClass> } // id: %48
197+
retain_value %44 : $Optional<VFSStagedContext> // id: %49
198+
br bb5 // id: %50
199+
200+
bb5: // Preds: bb4
201+
strong_retain %0 : $@callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> () // id: %51
202+
strong_retain %8 : ${ var Array<ElementClass> } // id: %52
203+
retain_value %44 : $Optional<VFSStagedContext> // id: %53
204+
strong_release %0 : $@callee_guaranteed (@guaranteed Optional<StagedContext>, @guaranteed Optional<Error>) -> () // id: %54
205+
release_value %44 : $Optional<VFSStagedContext> // id: %55
206+
strong_release %8 : ${ var Array<ElementClass> } // id: %56
207+
strong_release %45 : $@callee_guaranteed () -> () // id: %57
208+
strong_release %8 : ${ var Array<ElementClass> } // id: %58
209+
strong_release %1 : ${ var Optional<VFSStagedContext> } // id: %59
210+
br bb6 // id: %60
211+
212+
bb6: // Preds: bb5
213+
%61 = tuple () // user: %62
214+
return %61 : $() // id: %62
215+
}

0 commit comments

Comments
 (0)