Skip to content

Commit c326016

Browse files
gottesmmgottesmm
authored
Merge pull request #27376 from gottesmm/pr-04c94db25674845b859877b727fa7ff6504e4ec5
[ownership] Add verification that parent borrow scopes completely enc…
2 parents acdf2e7 + c195f73 commit c326016

File tree

3 files changed

+176
-12
lines changed

3 files changed

+176
-12
lines changed

‎lib/SIL/SILOwnershipVerifier.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,10 +287,22 @@ bool SILValueOwnershipChecker::gatherUsers(
287287
continue;
288288
}
289289

290-
// If we are guaranteed, but are not a guaranteed forwarding inst,
291-
// just continue. This user is just treated as a normal use.
292-
if (!isGuaranteedForwardingInst(user))
290+
// If we are guaranteed, but are not a guaranteed forwarding inst, we add
291+
// the end scope instructions of any new sub-scopes. This ensures that the
292+
// parent scope completely encloses the child borrow scope.
293+
//
294+
// Example: A guaranteed parameter of a co-routine.
295+
if (!isGuaranteedForwardingInst(user)) {
296+
// First check if we are visiting an operand that introduces a new
297+
// sub-scope. If we do, we need to preserve
298+
if (auto scopedOperand = BorrowScopeOperand::get(op)) {
299+
scopedOperand->visitEndScopeInstructions(
300+
[&](Operand *op) { implicitRegularUsers.push_back(op); });
301+
}
302+
303+
// Then continue.
293304
continue;
305+
}
294306

295307
// At this point, we know that we must have a forwarded subobject. Since the
296308
// base type is guaranteed, we know that the subobject is either guaranteed

‎test/SIL/ownership-verifier/borrow_scope_introducing_operands.sil

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,77 @@ bb3:
100100
%r = tuple ()
101101
return %r : $()
102102
}
103+
104+
// CHECK-LABEL: Function: 'parent_borrow_scope_end_before_end_borrow_coroutine'
105+
// CHECK: Found use after free?!
106+
// CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject
107+
// CHECK: Consuming User: end_borrow %1 : $Builtin.NativeObject
108+
// CHECK: Non Consuming User: end_apply %3
109+
// CHECK: Block: bb0
110+
sil [ossa] @parent_borrow_scope_end_before_end_borrow_coroutine : $@convention(thin) (@owned Builtin.NativeObject) -> () {
111+
bb0(%0 : @owned $Builtin.NativeObject):
112+
%1 = begin_borrow %0 : $Builtin.NativeObject
113+
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
114+
%token = begin_apply %coro(%1) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
115+
end_borrow %1 : $Builtin.NativeObject
116+
end_apply %token
117+
destroy_value %0 : $Builtin.NativeObject
118+
%r = tuple ()
119+
return %r : $()
120+
}
121+
122+
// CHECK-LABEL: Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_2'
123+
// CHECK: Found use after free?!
124+
// CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject
125+
// CHECK: Consuming User: end_borrow %1 : $Builtin.NativeObject
126+
// CHECK: Non Consuming User: abort_apply %3
127+
// CHECK: Block: bb0
128+
sil [ossa] @parent_borrow_scope_end_before_end_borrow_coroutine_2 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
129+
bb0(%0 : @owned $Builtin.NativeObject):
130+
%1 = begin_borrow %0 : $Builtin.NativeObject
131+
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
132+
%token = begin_apply %coro(%1) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
133+
end_borrow %1 : $Builtin.NativeObject
134+
abort_apply %token
135+
destroy_value %0 : $Builtin.NativeObject
136+
%r = tuple ()
137+
return %r : $()
138+
}
139+
140+
// CHECK-LABEL: Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_3'
141+
// CHECK: Found use after free?!
142+
// CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject
143+
// CHECK: Consuming User: end_borrow %1 : $Builtin.NativeObject
144+
// CHECK: Non Consuming User: abort_apply %3
145+
// CHECK: Block: bb1
146+
147+
// CHECK-LABEL: Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_3'
148+
// CHECK: Found use after free due to unvisited non lifetime ending uses?!
149+
// CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject
150+
// CHECK: Remaining Users:
151+
// CHECK: User: abort_apply %3
152+
// CHECK: Block: bb1
153+
154+
sil [ossa] @parent_borrow_scope_end_before_end_borrow_coroutine_3 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
155+
bb0(%0 : @owned $Builtin.NativeObject):
156+
%1 = begin_borrow %0 : $Builtin.NativeObject
157+
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
158+
%token = begin_apply %coro(%1) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
159+
cond_br undef, bb1, bb2
160+
161+
bb1:
162+
end_borrow %1 : $Builtin.NativeObject
163+
abort_apply %token
164+
br bb3
165+
166+
bb2:
167+
end_apply %token
168+
end_borrow %1 : $Builtin.NativeObject
169+
br bb3
170+
171+
bb3:
172+
destroy_value %0 : $Builtin.NativeObject
173+
%r = tuple ()
174+
return %r : $()
175+
}
176+

‎test/SIL/ownership-verifier/borrow_scope_introducing_operands_positive.sil

Lines changed: 87 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,6 @@
77

88
import Builtin
99

10-
sil [ossa] @destroy_value_before_end_borrow : $@convention(thin) (@owned Builtin.NativeObject) -> () {
11-
bb0(%0 : @owned $Builtin.NativeObject):
12-
%1 = begin_borrow %0 : $Builtin.NativeObject
13-
end_borrow %1 : $Builtin.NativeObject
14-
destroy_value %0 : $Builtin.NativeObject
15-
%9999 = tuple()
16-
return %9999 : $()
17-
}
18-
1910
sil [ossa] @coroutine_callee : $@yield_once (@guaranteed Builtin.NativeObject) -> () {
2011
bb0(%0 : @guaranteed $Builtin.NativeObject):
2112
yield (), resume bb1, unwind bb2
@@ -28,6 +19,15 @@ bb2:
2819
unwind
2920
}
3021

22+
sil [ossa] @destroy_value_before_end_borrow : $@convention(thin) (@owned Builtin.NativeObject) -> () {
23+
bb0(%0 : @owned $Builtin.NativeObject):
24+
%1 = begin_borrow %0 : $Builtin.NativeObject
25+
end_borrow %1 : $Builtin.NativeObject
26+
destroy_value %0 : $Builtin.NativeObject
27+
%9999 = tuple()
28+
return %9999 : $()
29+
}
30+
3131
sil [ossa] @destroy_value_before_end_borrow_coroutine : $@convention(thin) (@owned Builtin.NativeObject) -> () {
3232
bb0(%0 : @owned $Builtin.NativeObject):
3333
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
@@ -94,3 +94,81 @@ bb3:
9494
%r = tuple ()
9595
return %r : $()
9696
}
97+
98+
sil [ossa] @end_parent_scope_before_end_borrow_coroutine : $@convention(thin) (@owned Builtin.NativeObject) -> () {
99+
bb0(%0 : @owned $Builtin.NativeObject):
100+
%1 = begin_borrow %0 : $Builtin.NativeObject
101+
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
102+
%token = begin_apply %coro(%1) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
103+
end_apply %token
104+
end_borrow %1 : $Builtin.NativeObject
105+
destroy_value %0 : $Builtin.NativeObject
106+
%r = tuple ()
107+
return %r : $()
108+
}
109+
110+
sil [ossa] @end_parent_scope_before_end_borrow_coroutine_1a : $@convention(thin) (@owned Builtin.NativeObject) -> () {
111+
bb0(%0 : @owned $Builtin.NativeObject):
112+
%1 = begin_borrow %0 : $Builtin.NativeObject
113+
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
114+
%token = begin_apply %coro(%1) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
115+
end_apply %token
116+
br bb1
117+
118+
bb1:
119+
end_borrow %1 : $Builtin.NativeObject
120+
destroy_value %0 : $Builtin.NativeObject
121+
%r = tuple ()
122+
return %r : $()
123+
}
124+
125+
sil [ossa] @end_parent_scope_before_end_borrow_coroutine_2 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
126+
bb0(%0 : @owned $Builtin.NativeObject):
127+
%1 = begin_borrow %0 : $Builtin.NativeObject
128+
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
129+
%token = begin_apply %coro(%1) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
130+
abort_apply %token
131+
end_borrow %1 : $Builtin.NativeObject
132+
destroy_value %0 : $Builtin.NativeObject
133+
%r = tuple ()
134+
return %r : $()
135+
}
136+
137+
sil [ossa] @end_parent_scope_before_end_borrow_coroutine_2b : $@convention(thin) (@owned Builtin.NativeObject) -> () {
138+
bb0(%0 : @owned $Builtin.NativeObject):
139+
%1 = begin_borrow %0 : $Builtin.NativeObject
140+
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
141+
%token = begin_apply %coro(%1) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
142+
abort_apply %token
143+
br bb1
144+
145+
bb1:
146+
end_borrow %1 : $Builtin.NativeObject
147+
destroy_value %0 : $Builtin.NativeObject
148+
%r = tuple ()
149+
return %r : $()
150+
}
151+
152+
sil [ossa] @positive_end_parent_scope_before_end_borrow_coroutine_3 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
153+
bb0(%0 : @owned $Builtin.NativeObject):
154+
%1 = begin_borrow %0 : $Builtin.NativeObject
155+
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
156+
%token = begin_apply %coro(%1) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
157+
cond_br undef, bb1, bb2
158+
159+
bb1:
160+
abort_apply %token
161+
end_borrow %1 : $Builtin.NativeObject
162+
destroy_value %0 : $Builtin.NativeObject
163+
br bb3
164+
165+
bb2:
166+
end_apply %token
167+
end_borrow %1 : $Builtin.NativeObject
168+
destroy_value %0 : $Builtin.NativeObject
169+
br bb3
170+
171+
bb3:
172+
%r = tuple ()
173+
return %r : $()
174+
}

0 commit comments

Comments
 (0)