Skip to content

Commit 8d5c73a

Browse files
atrickatrick
authored
Merge pull request swiftlang#30009 from atrick/fix-escape-miscompile
Fix EscapeAnalysis connection graph for existential values.
2 parents c6183ee + 601a51a commit 8d5c73a

File tree

5 files changed

+91
-15
lines changed

5 files changed

+91
-15
lines changed

‎lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ SILValue EscapeAnalysis::getPointerBase(SILValue value) {
122122
case ValueKind::StructElementAddrInst:
123123
case ValueKind::StructExtractInst:
124124
case ValueKind::TupleElementAddrInst:
125+
case ValueKind::InitExistentialAddrInst:
126+
case ValueKind::OpenExistentialAddrInst:
125127
case ValueKind::BeginAccessInst:
126128
case ValueKind::UncheckedTakeEnumDataAddrInst:
127129
case ValueKind::UncheckedEnumDataInst:
@@ -2165,9 +2167,7 @@ void EscapeAnalysis::analyzeInstruction(SILInstruction *I,
21652167
}
21662168
case SILInstructionKind::RefElementAddrInst:
21672169
case SILInstructionKind::RefTailAddrInst:
2168-
case SILInstructionKind::ProjectBoxInst:
2169-
case SILInstructionKind::InitExistentialAddrInst:
2170-
case SILInstructionKind::OpenExistentialAddrInst: {
2170+
case SILInstructionKind::ProjectBoxInst: {
21712171
// For projections into objects, get the non-address reference operand and
21722172
// return an interior content node that the reference points to.
21732173
auto SVI = cast<SingleValueInstruction>(I);

‎test/SILOptimizer/escape_analysis.sil

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,9 +1019,8 @@ bb0(%0 : $Builtin.Int64, %1 : $X, %2 : $X, %3 : $X):
10191019

10201020
// CHECK-LABEL: CG of test_existential_addr
10211021
// CHECK-NEXT: Arg [ref] %0 Esc: A, Succ:
1022-
// CHECK-NEXT: Val %1 Esc: , Succ: (%2)
1023-
// CHECK-NEXT: Con %2 Esc: , Succ: (%2.1)
1024-
// CHECK-NEXT: Con [ref] %2.1 Esc: , Succ: %0
1022+
// CHECK-NEXT: Val %1 Esc: , Succ: (%1.1)
1023+
// CHECK-NEXT: Con [ref] %1.1 Esc: , Succ: %0
10251024
// CHECK-NEXT: End
10261025
sil @test_existential_addr : $@convention(thin) (@owned Pointer) -> () {
10271026
bb0(%0 : $Pointer):

‎test/SILOptimizer/escape_analysis_invalidate.sil

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
1-
// RUN: %target-sil-opt %s -temp-rvalue-opt -enable-sil-verify-all -escapes-internal-verify | %FileCheck %s
1+
// RUN: %target-sil-opt %s -temp-rvalue-opt -enable-sil-verify-all -escapes-internal-verify -wmo | %FileCheck %s
22
//
33
// TempRValue iteratively uses EscapeAnalysis and deletes
44
// instructions. Make sure that the connection graph remains valid
55
// <rdar://57290845>.
6+
//
7+
// This test requires -wmo so EscapeAnalysis can find all
8+
// implementations of SomeProtocol.foo. Otherwise the existential
9+
// address appears to escape. As an alternative, we could be more
10+
// aggressive about considering address-type argument not to escape,
11+
// but that would require some limiting address_to_pointer to never
12+
// occur on an exclusive address argument.
613

714
import Swift
815

@@ -82,3 +89,24 @@ bb0(%0 : $*SomeProtocol, %1 : $SomeClass):
8289
%v = tuple ()
8390
return %v : $()
8491
}
92+
93+
sil hidden @$s26escape_analysis_invalidate12SomeInstanceV3fooyyF : $@convention(method) (SomeInstance) -> () {
94+
bb0(%0 : $SomeInstance):
95+
debug_value %0 : $SomeInstance, let, name "self", argno 1 // id: %1
96+
%2 = tuple () // user: %3
97+
return %2 : $() // id: %3
98+
}
99+
100+
sil private [transparent] [thunk] @$s26escape_analysis_invalidate12SomeInstanceVAA0A8ProtocolA2aDP3fooyyFTW : $@convention(witness_method: SomeProtocol) (@in_guaranteed SomeInstance) -> () {
101+
bb0(%0 : $*SomeInstance):
102+
%1 = load %0 : $*SomeInstance
103+
// function_ref SomeInstance.foo()
104+
%2 = function_ref @$s26escape_analysis_invalidate12SomeInstanceV3fooyyF : $@convention(method) (SomeInstance) -> ()
105+
%3 = apply %2(%1) : $@convention(method) (SomeInstance) -> ()
106+
%4 = tuple ()
107+
return %4 : $()
108+
}
109+
110+
sil_witness_table hidden SomeInstance: SomeProtocol module t {
111+
method #SomeProtocol.foo!1: <Self where Self : SomeProtocol> (Self) -> () -> () : @$s26escape_analysis_invalidate12SomeInstanceVAA0A8ProtocolA2aDP3fooyyFTW // protocol witness for SomeProtocol.foo() in conformance SomeInstance
112+
}

‎test/SILOptimizer/escape_analysis_reduced.sil

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -499,17 +499,18 @@ bb2:
499499
}
500500

501501
// CHECK-LABEL: CG of testPendingMerge
502-
// CHECK-NEXT: Arg %0 Esc: A, Succ: (%22)
503-
// CHECK-NEXT: Arg [ref] %1 Esc: G, Succ: (%9.1)
502+
// CHECK-NEXT: Arg %0 Esc: A, Succ: (%0.1)
503+
// CHECK-NEXT: Con %0.1 Esc: A, Succ: %1, %4.1
504+
// CHECK-NEXT: Arg [ref] %1 Esc: A, Succ: (%9.1)
504505
// CHECK-NEXT: Val %2 Esc: , Succ: (%9)
505506
// CHECK-NEXT: Val %4 Esc: , Succ: (%4.1)
506-
// CHECK-NEXT: Con %4.1 Esc: A, Succ: (%9.1), %7, %9
507-
// CHECK-NEXT: Val %5 Esc: , Succ: (%7)
508-
// CHECK-NEXT: Con %7 Esc: A, Succ: (%9.1)
507+
// CHECK-NEXT: Con %4.1 Esc: A, Succ: (%9.1), %5.1, %9
508+
// CHECK-NEXT: Val %5 Esc: , Succ: (%5.1)
509+
// CHECK-NEXT: Con %5.1 Esc: A, Succ: %1
509510
// CHECK-NEXT: Con [ref] %9 Esc: A, Succ: (%9.1)
510-
// CHECK-NEXT: Con [int] %9.1 Esc: G, Succ: (%9.1), %1
511-
// CHECK-NEXT: Con %22 Esc: A, Succ: (%22.1)
512-
// CHECK-NEXT: Con %22.1 Esc: A, Succ: %1, %4.1
511+
// CHECK-NEXT: Con [int] %9.1 Esc: G, Succ: (%9.2)
512+
// CHECK-NEXT: Con %9.2 Esc: G, Succ: (%9.3)
513+
// CHECK-NEXT: Con %9.3 Esc: G, Succ:
513514
// CHECK-LABEL: End
514515
sil private @testPendingMerge : $@convention(thin) (@owned VariableNode) -> (@out ASTNode, @error Error) {
515516
bb0(%0 : $*ASTNode, %1 : $VariableNode):

‎test/SILOptimizer/escape_analysis_release_hoisting.sil

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all -release-hoisting %s | %FileCheck %s
2+
// REQUIRES: CPU=x86_64
3+
// REQUIRES: OS=macosx
4+
5+
sil_stage canonical
6+
7+
import Builtin
8+
import Swift
9+
import SwiftShims
10+
11+
// Test <rdar://59559805> miscompile; use-after-free
12+
// Avoid hoisting strong_release of an existential
13+
// over a store to the value.
14+
protocol ArrayElementProtocol {}
15+
16+
struct ArrayElementStruct : ArrayElementProtocol {
17+
@_hasStorage @_hasInitialValue var dummy: Int { get set }
18+
}
19+
20+
// CHECK-LABEL: sil @testArrayStorageInitAfterFree : $@convention(thin) () -> () {
21+
// CHECK: [[ARRAY:%.*]] = alloc_ref [stack] [tail_elems $ArrayElementProtocol * undef : $Builtin.Word] $_ContiguousArrayStorage<ArrayElementProtocol>
22+
// CHECK: [[CAST:%.*]] = upcast [[ARRAY]] : $_ContiguousArrayStorage<ArrayElementProtocol> to $__ContiguousArrayStorageBase
23+
// CHECK: [[TAIL:%.*]] = ref_tail_addr [[CAST]] : $__ContiguousArrayStorageBase, $ArrayElementProtocol
24+
// CHECK: [[ADR:%.*]] = init_existential_addr [[TAIL]] : $*ArrayElementProtocol, $ArrayElementStruct
25+
// CHECK: store %{{.*}} to [[ADR]] : $*ArrayElementStruct
26+
// CHECK: strong_release [[ARRAY]] : $_ContiguousArrayStorage<ArrayElementProtocol>
27+
// CHECK-LABEL: } // end sil function 'testArrayStorageInitAfterFree'
28+
sil @testArrayStorageInitAfterFree : $@convention(thin) () -> () {
29+
bb0:
30+
%0 = alloc_ref [stack] [tail_elems $ArrayElementProtocol * undef : $Builtin.Word] $_ContiguousArrayStorage<ArrayElementProtocol>
31+
%1 = upcast %0 : $_ContiguousArrayStorage<ArrayElementProtocol> to $__ContiguousArrayStorageBase
32+
%2 = struct $_SwiftArrayBodyStorage (undef : $Int, undef : $UInt)
33+
%3 = struct $_ArrayBody (%2 : $_SwiftArrayBodyStorage)
34+
%4 = ref_element_addr %1 : $__ContiguousArrayStorageBase, #__ContiguousArrayStorageBase.countAndCapacity
35+
store %3 to %4 : $*_ArrayBody
36+
%6 = ref_tail_addr %1 : $__ContiguousArrayStorageBase, $ArrayElementProtocol
37+
%7 = value_to_bridge_object undef : $Builtin.Int64
38+
%8 = struct $_StringObject (undef : $UInt64, %7 : $Builtin.BridgeObject)
39+
%9 = struct $_StringGuts (%8 : $_StringObject)
40+
%10 = struct $String (%9 : $_StringGuts)
41+
%11 = struct $ArrayElementStruct (undef : $Int)
42+
%12 = init_existential_addr %6 : $*ArrayElementProtocol, $ArrayElementStruct
43+
store %11 to %12 : $*ArrayElementStruct
44+
strong_release %0 : $_ContiguousArrayStorage<ArrayElementProtocol>
45+
dealloc_ref [stack] %0 : $_ContiguousArrayStorage<ArrayElementProtocol>
46+
%16 = tuple ()
47+
return %16 : $()
48+
}

0 commit comments

Comments
 (0)