Skip to content

Commit 040224e

Browse files
committed
InitializeStaticGlobals: fix a SIL verifier crash
When merging stores in a global initializer, it's possible that the merged store is inserted at the wrong location, causing a SIL verifier error. This is hard to reproduce, but can happen. The merged store must be inserted _after_ all other stores. Instead it's inserted after the store of the last property. Now, if properties are _not_ initialized in the order they are declared, this problem can show up. rdar://117189962
1 parent 0d4980e commit 040224e

File tree

2 files changed

+37
-6
lines changed

2 files changed

+37
-6
lines changed

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/InitializeStaticGlobals.swift

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,8 @@ private extension Function {
156156
private func mergeStores(in function: Function, _ context: FunctionPassContext) {
157157
for inst in function.instructions {
158158
if let store = inst as? StoreInst {
159-
if let elementStores = getSequenceOfElementStores(firstStore: store) {
160-
merge(elementStores: elementStores, context)
159+
if let (elementStores, lastStore) = getSequenceOfElementStores(firstStore: store) {
160+
merge(elementStores: elementStores, lastStore: lastStore, context)
161161
}
162162
}
163163
}
@@ -171,7 +171,7 @@ private func mergeStores(in function: Function, _ context: FunctionPassContext)
171171
/// %addr_n = struct_element_addr %structAddr, #field_n
172172
/// store %element_n to %addr_n
173173
///
174-
private func getSequenceOfElementStores(firstStore: StoreInst) -> [StoreInst]? {
174+
private func getSequenceOfElementStores(firstStore: StoreInst) -> ([StoreInst], lastStore: StoreInst)? {
175175
guard let elementAddr = firstStore.destination as? StructElementAddrInst else {
176176
return nil
177177
}
@@ -195,7 +195,7 @@ private func getSequenceOfElementStores(firstStore: StoreInst) -> [StoreInst]? {
195195
numStoresFound += 1
196196
if numStoresFound == numElements {
197197
// If we saw `numElements` distinct stores, it implies that all elements in `elementStores` are not nil.
198-
return elementStores.map { $0! }
198+
return (elementStores.map { $0! }, lastStore: store)
199199
}
200200
default:
201201
if inst.mayReadOrWriteMemory {
@@ -206,8 +206,7 @@ private func getSequenceOfElementStores(firstStore: StoreInst) -> [StoreInst]? {
206206
return nil
207207
}
208208

209-
private func merge(elementStores: [StoreInst], _ context: FunctionPassContext) {
210-
let lastStore = elementStores.last!
209+
private func merge(elementStores: [StoreInst], lastStore: StoreInst, _ context: FunctionPassContext) {
211210
let builder = Builder(after: lastStore, context)
212211

213212
let structAddr = (lastStore.destination as! StructElementAddrInst).struct

test/SILOptimizer/init_static_globals.sil

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ sil_global [let] @g7 : $TwoFields
9898
// CHECK-NEXT: }
9999
sil_global [let] @g8 : $UnsafePointer<Int32>
100100

101+
sil_global [let] @g9 : $TwoFields
102+
103+
sil @unknownfunc : $@convention(thin) () -> ()
104+
101105
// CHECK-LABEL: sil [global_init_once_fn] [ossa] @globalinit_trivialglobal_func :
102106
// CHECK-NOT: alloc_global
103107
// CHECK-NOT: store
@@ -233,6 +237,34 @@ bb0:
233237
return %10 : $()
234238
}
235239

240+
// CHECK-LABEL: sil [global_init_once_fn] [ossa] @merge_stores_but_no_global_init :
241+
// CHECK: [[GA:%.*]] = global_addr @g9 : $*TwoFields
242+
// CHECK: [[L10:%.*]] = integer_literal $Builtin.Int32, 10
243+
// CHECK: [[I10:%.*]] = struct $Int32 ([[L10]] : $Builtin.Int32)
244+
// CHECK: [[L11:%.*]] = integer_literal $Builtin.Int32, 11
245+
// CHECK: [[I11:%.*]] = struct $Int32 ([[L11]] : $Builtin.Int32)
246+
// CHECK: [[TF:%.*]] = struct $TwoFields ([[I11]] : $Int32, [[I10]] : $Int32)
247+
// CHECK: store [[TF]] to [trivial] [[GA]]
248+
// CHECK: function_ref
249+
// CHECK: } // end sil function 'merge_stores_but_no_global_init'
250+
sil [global_init_once_fn] [ossa] @merge_stores_but_no_global_init : $@convention(c) () -> () {
251+
bb0:
252+
alloc_global @g9
253+
%1 = global_addr @g9 : $*TwoFields
254+
%2 = integer_literal $Builtin.Int32, 10
255+
%3 = struct $Int32 (%2 : $Builtin.Int32)
256+
%4 = struct_element_addr %1 : $*TwoFields, #TwoFields.b
257+
store %3 to [trivial] %4 : $*Int32
258+
%6 = integer_literal $Builtin.Int32, 11
259+
%7 = struct $Int32 (%6 : $Builtin.Int32)
260+
%8 = struct_element_addr %1 : $*TwoFields, #TwoFields.a
261+
store %7 to [trivial] %8 : $*Int32
262+
%f = function_ref @unknownfunc : $@convention(thin) () -> ()
263+
%a = apply %f() : $@convention(thin) () -> ()
264+
%10 = tuple ()
265+
return %10 : $()
266+
}
267+
236268
// CHECK-LABEL: sil [global_init_once_fn] [ossa] @globalinit_wrong_separate_stores :
237269
// CHECK: alloc_global
238270
// CHECK: store

0 commit comments

Comments
 (0)