Skip to content

Commit a3f6c3e

Browse files
gottesmmgottesmm
authored
Merge pull request #26273 from gottesmm/swift-5.1-branch-rdar53376468
[5.1] Use GetValueAtEndOfBlock instead of GetValueInMiddleOfBlock for getti…
2 parents 945b925 + fdf69f0 commit a3f6c3e

File tree

2 files changed

+158
-1
lines changed

2 files changed

+158
-1
lines changed

‎lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ static bool fixupCopyBlockWithoutEscaping(CopyBlockWithoutEscapingInst *CB,
758758
auto *SafeClosureDestructionPt =
759759
getDeinitSafeClosureDestructionPoint(Term);
760760
SILBuilderWithScope B(SafeClosureDestructionPt);
761-
B.createDestroyValue(generatedLoc, Updater.GetValueInMiddleOfBlock(Exit));
761+
B.createDestroyValue(generatedLoc, Updater.GetValueAtEndOfBlock(Exit));
762762
}
763763
}
764764

‎test/SILOptimizer/closure-lifetime-fixup.sil

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all -closure-lifetime-fixup %s | %FileCheck %s
2+
3+
sil_stage raw
4+
5+
import Swift
6+
import Builtin
7+
import SwiftShims
8+
9+
class FakeNSString {}
10+
class Klass {}
11+
12+
sil @$sSSSgIgg_AAIegg_TR : $@convention(thin) (@guaranteed Optional<String>, @noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()) -> ()
13+
sil @noescapeBlock3 : $@convention(c) (Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<FakeNSString>) -> ()
14+
sil @$sSS10FoundationE19_bridgeToObjectiveCSo8FakeNSStringCyF : $@convention(method) (@guaranteed String) -> @owned FakeNSString
15+
sil @$sSS21_builtinStringLiteral17utf8CodeUnitCount7isASCIISSBp_BwBi1_tcfC : $@convention(method) (Builtin.RawPointer, Builtin.Word, Builtin.Int1, @thin String.Type) -> @owned String
16+
sil @$sSSSgIegg_So8FakeNSStringCSgIyBy_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), Optional<FakeNSString>) -> ()
17+
18+
// Just make sure that we perform the optimization and do not trigger the ownership verifier.
19+
//
20+
// CHECK-LABEL: sil [ossa] @test1 : $@convention(thin) (@guaranteed Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>, @guaranteed Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>) -> () {
21+
// CHECK-NOT: convert_escape_to_noescape [not_guaranteed]
22+
// CHECK: } // end sil function 'test1'
23+
sil [ossa] @test1 : $@convention(thin) (@guaranteed Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>, @guaranteed Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>) -> () {
24+
bb0(%0 : @guaranteed $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>, %1 : @guaranteed $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>):
25+
%2 = copy_value %0 : $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>
26+
switch_enum %2 : $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>, case #Optional.some!enumelt.1: bb1, case #Optional.none!enumelt: bb12
27+
28+
bb1(%4 : @owned $@callee_guaranteed (@guaranteed Optional<String>) -> ()):
29+
%5 = convert_escape_to_noescape [not_guaranteed] %4 : $@callee_guaranteed (@guaranteed Optional<String>) -> () to $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
30+
%6 = enum $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, #Optional.some!enumelt.1, %5 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
31+
destroy_value %4 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
32+
br bb2(%6 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>)
33+
34+
bb2(%9 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>):
35+
switch_enum %9 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, case #Optional.some!enumelt.1: bb3, case #Optional.none!enumelt: bb4
36+
37+
bb3(%11 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()):
38+
%12 = function_ref @$sSSSgIgg_AAIegg_TR : $@convention(thin) (@guaranteed Optional<String>, @noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()) -> ()
39+
%13 = partial_apply [callee_guaranteed] %12(%11) : $@convention(thin) (@guaranteed Optional<String>, @noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()) -> ()
40+
%14 = mark_dependence %13 : $@callee_guaranteed (@guaranteed Optional<String>) -> () on %11 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
41+
%15 = copy_value %14 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
42+
%16 = alloc_stack $@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
43+
%17 = project_block_storage %16 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
44+
store %15 to [init] %17 : $*@callee_guaranteed (@guaranteed Optional<String>) -> ()
45+
%19 = function_ref @$sSSSgIegg_So8FakeNSStringCSgIyBy_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), Optional<FakeNSString>) -> ()
46+
%20 = init_block_storage_header %16 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), invoke %19 : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), Optional<FakeNSString>) -> (), type $@convention(block) @noescape (Optional<FakeNSString>) -> ()
47+
%21 = copy_block_without_escaping %20 : $@convention(block) @noescape (Optional<FakeNSString>) -> () withoutEscaping %14 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
48+
%22 = enum $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, #Optional.some!enumelt.1, %21 : $@convention(block) @noescape (Optional<FakeNSString>) -> ()
49+
destroy_addr %17 : $*@callee_guaranteed (@guaranteed Optional<String>) -> ()
50+
dealloc_stack %16 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
51+
br bb5(%22 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>)
52+
53+
bb4:
54+
%26 = enum $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, #Optional.none!enumelt
55+
br bb5(%26 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>)
56+
57+
bb5(%28 : @owned $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>):
58+
%29 = copy_value %1 : $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>
59+
switch_enum %29 : $Optional<@callee_guaranteed (@guaranteed Optional<String>) -> ()>, case #Optional.some!enumelt.1: bb6, case #Optional.none!enumelt: bb11
60+
61+
bb6(%31 : @owned $@callee_guaranteed (@guaranteed Optional<String>) -> ()):
62+
%32 = convert_escape_to_noescape [not_guaranteed] %31 : $@callee_guaranteed (@guaranteed Optional<String>) -> () to $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
63+
%33 = enum $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, #Optional.some!enumelt.1, %32 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
64+
destroy_value %31 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
65+
br bb7(%33 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>)
66+
67+
bb7(%36 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>):
68+
switch_enum %36 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, case #Optional.some!enumelt.1: bb8, case #Optional.none!enumelt: bb9
69+
70+
bb8(%38 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()):
71+
%39 = function_ref @$sSSSgIgg_AAIegg_TR : $@convention(thin) (@guaranteed Optional<String>, @noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()) -> ()
72+
%40 = partial_apply [callee_guaranteed] %39(%38) : $@convention(thin) (@guaranteed Optional<String>, @noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()) -> ()
73+
%41 = mark_dependence %40 : $@callee_guaranteed (@guaranteed Optional<String>) -> () on %38 : $@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()
74+
%42 = copy_value %41 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
75+
%43 = alloc_stack $@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
76+
%44 = project_block_storage %43 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
77+
store %42 to [init] %44 : $*@callee_guaranteed (@guaranteed Optional<String>) -> ()
78+
%46 = function_ref @$sSSSgIegg_So8FakeNSStringCSgIyBy_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), Optional<FakeNSString>) -> ()
79+
%47 = init_block_storage_header %43 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), invoke %46 : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed Optional<String>) -> (), Optional<FakeNSString>) -> (), type $@convention(block) @noescape (Optional<FakeNSString>) -> ()
80+
%48 = copy_block_without_escaping %47 : $@convention(block) @noescape (Optional<FakeNSString>) -> () withoutEscaping %41 : $@callee_guaranteed (@guaranteed Optional<String>) -> ()
81+
%49 = enum $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, #Optional.some!enumelt.1, %48 : $@convention(block) @noescape (Optional<FakeNSString>) -> ()
82+
destroy_addr %44 : $*@callee_guaranteed (@guaranteed Optional<String>) -> ()
83+
dealloc_stack %43 : $*@block_storage @callee_guaranteed (@guaranteed Optional<String>) -> ()
84+
br bb10(%49 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>)
85+
86+
bb9:
87+
%53 = enum $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, #Optional.none!enumelt
88+
br bb10(%53 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>)
89+
90+
bb10(%55 : @owned $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>):
91+
%56 = string_literal utf8 "Foobar"
92+
%57 = integer_literal $Builtin.Word, 6
93+
%58 = integer_literal $Builtin.Int1, -1
94+
%59 = metatype $@thin String.Type
95+
%60 = function_ref @$sSS21_builtinStringLiteral17utf8CodeUnitCount7isASCIISSBp_BwBi1_tcfC : $@convention(method) (Builtin.RawPointer, Builtin.Word, Builtin.Int1, @thin String.Type) -> @owned String
96+
%61 = apply %60(%56, %57, %58, %59) : $@convention(method) (Builtin.RawPointer, Builtin.Word, Builtin.Int1, @thin String.Type) -> @owned String
97+
%62 = function_ref @$sSS10FoundationE19_bridgeToObjectiveCSo8FakeNSStringCyF : $@convention(method) (@guaranteed String) -> @owned FakeNSString
98+
%63 = begin_borrow %61 : $String
99+
%64 = apply %62(%63) : $@convention(method) (@guaranteed String) -> @owned FakeNSString
100+
end_borrow %63 : $String
101+
%66 = enum $Optional<FakeNSString>, #Optional.some!enumelt.1, %64 : $FakeNSString
102+
destroy_value %61 : $String
103+
%68 = function_ref @noescapeBlock3 : $@convention(c) (Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<FakeNSString>) -> ()
104+
%69 = apply %68(%28, %55, %66) : $@convention(c) (Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>, Optional<FakeNSString>) -> ()
105+
destroy_value %66 : $Optional<FakeNSString>
106+
destroy_value %55 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>
107+
destroy_value %28 : $Optional<@convention(block) @noescape (Optional<FakeNSString>) -> ()>
108+
%73 = tuple ()
109+
return %73 : $()
110+
111+
bb11:
112+
%75 = enum $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, #Optional.none!enumelt
113+
br bb7(%75 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>)
114+
115+
bb12:
116+
%77 = enum $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>, #Optional.none!enumelt
117+
br bb2(%77 : $Optional<@noescape @callee_guaranteed (@guaranteed Optional<String>) -> ()>)
118+
}
119+
120+
sil @originalClosure : $@convention(thin) () -> ()
121+
sil @noEscapeThunk : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> ()
122+
sil @blockThunk : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed () -> ()) -> ()
123+
124+
// Just make sure we apply the optimization. The ownership verifier will verify
125+
// that we do not catch the leak.
126+
//
127+
// CHECK-LABEL: sil [ossa] @ssaupdater_no_single_destroy_some_in_exit_block : $@convention(thin) (@guaranteed Klass, @guaranteed @callee_guaranteed () -> (), @guaranteed Klass, @guaranteed @callee_guaranteed () -> ()) -> () {
128+
// CHECK-NOT: convert_escape_to_noescape [not_guaranteed]
129+
// CHECK-NOT: copy_block_without_escaping
130+
// CHECK: } // end sil function 'ssaupdater_no_single_destroy_some_in_exit_block'
131+
sil [ossa] @ssaupdater_no_single_destroy_some_in_exit_block : $@convention(thin) (@guaranteed Klass, @guaranteed @callee_guaranteed () -> (), @guaranteed Klass, @guaranteed @callee_guaranteed () -> ()) -> () {
132+
bb0(%0 : @guaranteed $Klass, %1 : @guaranteed $@callee_guaranteed () -> (), %2 : @guaranteed $Klass, %3 : @guaranteed $@callee_guaranteed () -> ()):
133+
// This basic block is needed to trigger the bug.
134+
br bb1
135+
136+
bb1:
137+
%39 = function_ref @originalClosure : $@convention(thin) () -> ()
138+
%43 = partial_apply [callee_guaranteed] %39() : $@convention(thin) () -> ()
139+
%44 = convert_escape_to_noescape [not_guaranteed] %43 : $@callee_guaranteed () -> () to $@noescape @callee_guaranteed () -> ()
140+
%45 = function_ref @noEscapeThunk : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> ()
141+
%46 = partial_apply [callee_guaranteed] %45(%44) : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> ()
142+
%47 = mark_dependence %46 : $@callee_guaranteed () -> () on %44 : $@noescape @callee_guaranteed () -> ()
143+
%48 = copy_value %47 : $@callee_guaranteed () -> ()
144+
%49 = alloc_stack $@block_storage @callee_guaranteed () -> ()
145+
%50 = project_block_storage %49 : $*@block_storage @callee_guaranteed () -> ()
146+
store %48 to [init] %50 : $*@callee_guaranteed () -> ()
147+
%52 = function_ref @blockThunk : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed () -> ()) -> ()
148+
%53 = init_block_storage_header %49 : $*@block_storage @callee_guaranteed () -> (), invoke %52 : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed () -> ()) -> (), type $@convention(block) @noescape () -> ()
149+
%54 = copy_block_without_escaping %53 : $@convention(block) @noescape () -> () withoutEscaping %47 : $@callee_guaranteed () -> ()
150+
%55 = enum $Optional<@convention(block) @noescape () -> ()>, #Optional.some!enumelt.1, %54 : $@convention(block) @noescape () -> ()
151+
destroy_addr %50 : $*@callee_guaranteed () -> ()
152+
dealloc_stack %49 : $*@block_storage @callee_guaranteed () -> ()
153+
destroy_value %43 : $@callee_guaranteed () -> ()
154+
destroy_value %55 : $Optional<@convention(block) @noescape () -> ()>
155+
%86 = tuple ()
156+
return %86 : $()
157+
}

0 commit comments

Comments
 (0)