Skip to content

Commit 5e8ba1d

Browse files
gottesmmgottesmm
authored
Merge pull request #27337 from gottesmm/pr-e029383157166756e653b260323559ce3a8eab16
[ownership] Add a higher level construct called BorrowScopeIntroducingOperand
2 parents 87d1338 + f9f3f53 commit 5e8ba1d

File tree

5 files changed

+314
-40
lines changed

5 files changed

+314
-40
lines changed

‎include/swift/SIL/OwnershipUtils.h

Lines changed: 98 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,86 @@ bool isOwnershipForwardingInst(SILInstruction *i);
200200

201201
bool isGuaranteedForwardingInst(SILInstruction *i);
202202

203-
struct BorrowScopeIntroducerKind {
203+
struct BorrowScopeOperandKind {
204+
using UnderlyingKindTy = std::underlying_type<SILInstructionKind>::type;
205+
206+
enum Kind : UnderlyingKindTy {
207+
BeginBorrow = UnderlyingKindTy(SILInstructionKind::BeginBorrowInst),
208+
BeginApply = UnderlyingKindTy(SILInstructionKind::BeginApplyInst),
209+
};
210+
211+
Kind value;
212+
213+
BorrowScopeOperandKind(Kind newValue) : value(newValue) {}
214+
BorrowScopeOperandKind(const BorrowScopeOperandKind &other)
215+
: value(other.value) {}
216+
operator Kind() const { return value; }
217+
218+
static Optional<BorrowScopeOperandKind> get(SILInstructionKind kind) {
219+
switch (kind) {
220+
default:
221+
return None;
222+
case SILInstructionKind::BeginBorrowInst:
223+
return BorrowScopeOperandKind(BeginBorrow);
224+
case SILInstructionKind::BeginApplyInst:
225+
return BorrowScopeOperandKind(BeginApply);
226+
}
227+
}
228+
229+
void print(llvm::raw_ostream &os) const;
230+
LLVM_ATTRIBUTE_DEPRECATED(void dump() const, "only for use in the debugger");
231+
};
232+
233+
/// An operand whose user instruction introduces a new borrow scope for the
234+
/// operand's value. The value of the operand must be considered as implicitly
235+
/// borrowed until the user's corresponding end scope instruction.
236+
struct BorrowScopeOperand {
237+
BorrowScopeOperandKind kind;
238+
Operand *op;
239+
240+
BorrowScopeOperand(Operand *op)
241+
: kind(*BorrowScopeOperandKind::get(op->getUser()->getKind())), op(op) {}
242+
243+
/// If value is a borrow introducer return it after doing some checks.
244+
static Optional<BorrowScopeOperand> get(Operand *op) {
245+
auto *user = op->getUser();
246+
auto kind = BorrowScopeOperandKind::get(user->getKind());
247+
if (!kind)
248+
return None;
249+
return BorrowScopeOperand(*kind, op);
250+
}
251+
252+
void visitEndScopeInstructions(function_ref<void(Operand *)> func) const {
253+
switch (kind) {
254+
case BorrowScopeOperandKind::BeginBorrow:
255+
for (auto *use : cast<BeginBorrowInst>(op->getUser())->getUses()) {
256+
if (isa<EndBorrowInst>(use->getUser())) {
257+
func(use);
258+
}
259+
}
260+
return;
261+
case BorrowScopeOperandKind::BeginApply: {
262+
auto *user = cast<BeginApplyInst>(op->getUser());
263+
for (auto *use : user->getTokenResult()->getUses()) {
264+
func(use);
265+
}
266+
return;
267+
}
268+
}
269+
llvm_unreachable("Covered switch isn't covered");
270+
}
271+
272+
private:
273+
/// Internal constructor for failable static constructor. Please do not expand
274+
/// its usage since it assumes the code passed in is well formed.
275+
BorrowScopeOperand(BorrowScopeOperandKind kind, Operand *op)
276+
: kind(kind), op(op) {}
277+
};
278+
279+
llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
280+
BorrowScopeOperandKind kind);
281+
282+
struct BorrowScopeIntroducingValueKind {
204283
using UnderlyingKindTy = std::underlying_type<ValueKind>::type;
205284

206285
/// Enum we use for exhaustive pattern matching over borrow scope introducers.
@@ -210,23 +289,23 @@ struct BorrowScopeIntroducerKind {
210289
SILFunctionArgument = UnderlyingKindTy(ValueKind::SILFunctionArgument),
211290
};
212291

213-
static Optional<BorrowScopeIntroducerKind> get(ValueKind kind) {
292+
static Optional<BorrowScopeIntroducingValueKind> get(ValueKind kind) {
214293
switch (kind) {
215294
default:
216295
return None;
217296
case ValueKind::LoadBorrowInst:
218-
return BorrowScopeIntroducerKind(LoadBorrow);
297+
return BorrowScopeIntroducingValueKind(LoadBorrow);
219298
case ValueKind::BeginBorrowInst:
220-
return BorrowScopeIntroducerKind(BeginBorrow);
299+
return BorrowScopeIntroducingValueKind(BeginBorrow);
221300
case ValueKind::SILFunctionArgument:
222-
return BorrowScopeIntroducerKind(SILFunctionArgument);
301+
return BorrowScopeIntroducingValueKind(SILFunctionArgument);
223302
}
224303
}
225304

226305
Kind value;
227306

228-
BorrowScopeIntroducerKind(Kind newValue) : value(newValue) {}
229-
BorrowScopeIntroducerKind(const BorrowScopeIntroducerKind &other)
307+
BorrowScopeIntroducingValueKind(Kind newValue) : value(newValue) {}
308+
BorrowScopeIntroducingValueKind(const BorrowScopeIntroducingValueKind &other)
230309
: value(other.value) {}
231310
operator Kind() const { return value; }
232311

@@ -238,10 +317,10 @@ struct BorrowScopeIntroducerKind {
238317
/// of the scope.
239318
bool isLocalScope() const {
240319
switch (value) {
241-
case BorrowScopeIntroducerKind::BeginBorrow:
242-
case BorrowScopeIntroducerKind::LoadBorrow:
320+
case BorrowScopeIntroducingValueKind::BeginBorrow:
321+
case BorrowScopeIntroducingValueKind::LoadBorrow:
243322
return true;
244-
case BorrowScopeIntroducerKind::SILFunctionArgument:
323+
case BorrowScopeIntroducingValueKind::SILFunctionArgument:
245324
return false;
246325
}
247326
llvm_unreachable("Covered switch isnt covered?!");
@@ -252,7 +331,7 @@ struct BorrowScopeIntroducerKind {
252331
};
253332

254333
llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
255-
BorrowScopeIntroducerKind kind);
334+
BorrowScopeIntroducingValueKind kind);
256335

257336
/// A higher level construct for working with values that represent the
258337
/// introduction of a new borrow scope.
@@ -271,26 +350,26 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
271350
/// borrow introducers can not have guaranteed results that are not creating a
272351
/// new borrow scope. No such instructions exist today.
273352
struct BorrowScopeIntroducingValue {
274-
BorrowScopeIntroducerKind kind;
353+
BorrowScopeIntroducingValueKind kind;
275354
SILValue value;
276355

277356
BorrowScopeIntroducingValue(LoadBorrowInst *lbi)
278-
: kind(BorrowScopeIntroducerKind::LoadBorrow), value(lbi) {}
357+
: kind(BorrowScopeIntroducingValueKind::LoadBorrow), value(lbi) {}
279358
BorrowScopeIntroducingValue(BeginBorrowInst *bbi)
280-
: kind(BorrowScopeIntroducerKind::BeginBorrow), value(bbi) {}
359+
: kind(BorrowScopeIntroducingValueKind::BeginBorrow), value(bbi) {}
281360
BorrowScopeIntroducingValue(SILFunctionArgument *arg)
282-
: kind(BorrowScopeIntroducerKind::SILFunctionArgument), value(arg) {
361+
: kind(BorrowScopeIntroducingValueKind::SILFunctionArgument), value(arg) {
283362
assert(arg->getOwnershipKind() == ValueOwnershipKind::Guaranteed);
284363
}
285364

286365
BorrowScopeIntroducingValue(SILValue v)
287-
: kind(*BorrowScopeIntroducerKind::get(v->getKind())), value(v) {
366+
: kind(*BorrowScopeIntroducingValueKind::get(v->getKind())), value(v) {
288367
assert(v.getOwnershipKind() == ValueOwnershipKind::Guaranteed);
289368
}
290369

291370
/// If value is a borrow introducer return it after doing some checks.
292371
static Optional<BorrowScopeIntroducingValue> get(SILValue value) {
293-
auto kind = BorrowScopeIntroducerKind::get(value->getKind());
372+
auto kind = BorrowScopeIntroducingValueKind::get(value->getKind());
294373
if (!kind || value.getOwnershipKind() != ValueOwnershipKind::Guaranteed)
295374
return None;
296375
return BorrowScopeIntroducingValue(*kind, value);
@@ -334,7 +413,8 @@ struct BorrowScopeIntroducingValue {
334413
private:
335414
/// Internal constructor for failable static constructor. Please do not expand
336415
/// its usage since it assumes the code passed in is well formed.
337-
BorrowScopeIntroducingValue(BorrowScopeIntroducerKind kind, SILValue value)
416+
BorrowScopeIntroducingValue(BorrowScopeIntroducingValueKind kind,
417+
SILValue value)
338418
: kind(kind), value(value) {}
339419
};
340420

‎lib/SIL/OwnershipUtils.cpp

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -81,22 +81,22 @@ bool swift::isOwnershipForwardingInst(SILInstruction *i) {
8181
// Borrow Introducers
8282
//===----------------------------------------------------------------------===//
8383

84-
void BorrowScopeIntroducerKind::print(llvm::raw_ostream &os) const {
84+
void BorrowScopeIntroducingValueKind::print(llvm::raw_ostream &os) const {
8585
switch (value) {
86-
case BorrowScopeIntroducerKind::SILFunctionArgument:
86+
case BorrowScopeIntroducingValueKind::SILFunctionArgument:
8787
os << "SILFunctionArgument";
8888
return;
89-
case BorrowScopeIntroducerKind::BeginBorrow:
89+
case BorrowScopeIntroducingValueKind::BeginBorrow:
9090
os << "BeginBorrowInst";
9191
return;
92-
case BorrowScopeIntroducerKind::LoadBorrow:
92+
case BorrowScopeIntroducingValueKind::LoadBorrow:
9393
os << "LoadBorrowInst";
9494
return;
9595
}
9696
llvm_unreachable("Covered switch isn't covered?!");
9797
}
9898

99-
void BorrowScopeIntroducerKind::dump() const {
99+
void BorrowScopeIntroducingValueKind::dump() const {
100100
#ifndef NDEBUG
101101
print(llvm::dbgs());
102102
#endif
@@ -107,13 +107,13 @@ void BorrowScopeIntroducingValue::getLocalScopeEndingInstructions(
107107
assert(isLocalScope() && "Should only call this given a local scope");
108108

109109
switch (kind) {
110-
case BorrowScopeIntroducerKind::SILFunctionArgument:
110+
case BorrowScopeIntroducingValueKind::SILFunctionArgument:
111111
llvm_unreachable("Should only call this with a local scope");
112-
case BorrowScopeIntroducerKind::BeginBorrow:
112+
case BorrowScopeIntroducingValueKind::BeginBorrow:
113113
llvm::copy(cast<BeginBorrowInst>(value)->getEndBorrows(),
114114
std::back_inserter(scopeEndingInsts));
115115
return;
116-
case BorrowScopeIntroducerKind::LoadBorrow:
116+
case BorrowScopeIntroducingValueKind::LoadBorrow:
117117
llvm::copy(cast<LoadBorrowInst>(value)->getEndBorrows(),
118118
std::back_inserter(scopeEndingInsts));
119119
return;
@@ -125,17 +125,17 @@ void BorrowScopeIntroducingValue::visitLocalScopeEndingUses(
125125
function_ref<void(Operand *)> visitor) const {
126126
assert(isLocalScope() && "Should only call this given a local scope");
127127
switch (kind) {
128-
case BorrowScopeIntroducerKind::SILFunctionArgument:
128+
case BorrowScopeIntroducingValueKind::SILFunctionArgument:
129129
llvm_unreachable("Should only call this with a local scope");
130-
case BorrowScopeIntroducerKind::BeginBorrow:
131-
for (auto *use : cast<BeginBorrowInst>(value)->getUses()) {
130+
case BorrowScopeIntroducingValueKind::BeginBorrow:
131+
for (auto *use : value->getUses()) {
132132
if (isa<EndBorrowInst>(use->getUser())) {
133133
visitor(use);
134134
}
135135
}
136136
return;
137-
case BorrowScopeIntroducerKind::LoadBorrow:
138-
for (auto *use : cast<LoadBorrowInst>(value)->getUses()) {
137+
case BorrowScopeIntroducingValueKind::LoadBorrow:
138+
for (auto *use : value->getUses()) {
139139
if (isa<EndBorrowInst>(use->getUser())) {
140140
visitor(use);
141141
}
@@ -182,7 +182,7 @@ bool swift::getUnderlyingBorrowIntroducingValues(
182182
}
183183

184184
llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &os,
185-
BorrowScopeIntroducerKind kind) {
185+
BorrowScopeIntroducingValueKind kind) {
186186
kind.print(os);
187187
return os;
188188
}

‎lib/SIL/SILOwnershipVerifier.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -278,14 +278,10 @@ bool SILValueOwnershipChecker::gatherUsers(
278278
// For correctness reasons we use indices to make sure that we can
279279
// append to NonLifetimeEndingUsers without needing to deal with
280280
// iterator invalidation.
281-
for (unsigned i : indices(nonLifetimeEndingUsers)) {
282-
if (auto *bbi = dyn_cast<BeginBorrowInst>(
283-
nonLifetimeEndingUsers[i]->getUser())) {
284-
for (auto *use : bbi->getUses()) {
285-
if (isa<EndBorrowInst>(use->getUser())) {
286-
implicitRegularUsers.push_back(use);
287-
}
288-
}
281+
for (auto *op : nonLifetimeEndingUsers) {
282+
if (auto scopedOperand = BorrowScopeOperand::get(op)) {
283+
scopedOperand->visitEndScopeInstructions(
284+
[&](Operand *op) { implicitRegularUsers.push_back(op); });
289285
}
290286
}
291287
}

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

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -enable-sil-verify-all=0 -o /dev/null 2>&1 %s | %FileCheck %s
2+
// REQUIRES: asserts
3+
4+
// This file tests that when we emit an error, it is a true negative. This is
5+
// done by parsing the emitted output from the ownership verifier.
6+
7+
import Builtin
8+
9+
10+
sil [ossa] @coroutine_callee : $@yield_once (@guaranteed Builtin.NativeObject) -> () {
11+
bb0(%0 : @guaranteed $Builtin.NativeObject):
12+
yield (), resume bb1, unwind bb2
13+
14+
bb1:
15+
%r = tuple ()
16+
return %r : $()
17+
18+
bb2:
19+
unwind
20+
}
21+
22+
// CHECK-LABEL: Function: 'destroy_value_before_end_borrow'
23+
// CHECK: Found use after free?!
24+
// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
25+
// CHECK: Consuming User: destroy_value %0 : $Builtin.NativeObject
26+
// CHECK: Non Consuming User: end_borrow %1 : $Builtin.NativeObject
27+
// CHECK: Block: bb0
28+
sil [ossa] @destroy_value_before_end_borrow : $@convention(thin) (@owned Builtin.NativeObject) -> () {
29+
bb0(%0 : @owned $Builtin.NativeObject):
30+
%1 = begin_borrow %0 : $Builtin.NativeObject
31+
destroy_value %0 : $Builtin.NativeObject
32+
end_borrow %1 : $Builtin.NativeObject
33+
%9999 = tuple()
34+
return %9999 : $()
35+
}
36+
37+
// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine'
38+
// CHECK: Found use after free?!
39+
// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
40+
// CHECK: Consuming User: destroy_value %0 : $Builtin.NativeObject
41+
// CHECK: Non Consuming User: end_apply %2
42+
// CHECK: Block: bb0
43+
sil [ossa] @destroy_value_before_end_borrow_coroutine : $@convention(thin) (@owned Builtin.NativeObject) -> () {
44+
bb0(%0 : @owned $Builtin.NativeObject):
45+
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
46+
%token = begin_apply %coro(%0) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
47+
destroy_value %0 : $Builtin.NativeObject
48+
end_apply %token
49+
%r = tuple ()
50+
return %r : $()
51+
}
52+
53+
// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine_2'
54+
// CHECK: Found use after free?!
55+
// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
56+
// CHECK: Consuming User: destroy_value %0 : $Builtin.NativeObject
57+
// CHECK: Non Consuming User: abort_apply %2
58+
// CHECK: Block: bb0
59+
sil [ossa] @destroy_value_before_end_borrow_coroutine_2 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
60+
bb0(%0 : @owned $Builtin.NativeObject):
61+
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
62+
%token = begin_apply %coro(%0) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
63+
destroy_value %0 : $Builtin.NativeObject
64+
abort_apply %token
65+
%r = tuple ()
66+
return %r : $()
67+
}
68+
69+
// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine_3'
70+
// CHECK: Found use after free?!
71+
// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
72+
// CHECK: Consuming User: destroy_value %0 : $Builtin.NativeObject
73+
// CHECK: Non Consuming User: abort_apply %2
74+
// CHECK: Block: bb1
75+
76+
// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine_3'
77+
// CHECK: Found use after free due to unvisited non lifetime ending uses?!
78+
// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
79+
// CHECK: Remaining Users:
80+
// CHECK: User: abort_apply %2
81+
// CHECK: Block: bb1
82+
83+
sil [ossa] @destroy_value_before_end_borrow_coroutine_3 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
84+
bb0(%0 : @owned $Builtin.NativeObject):
85+
%coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
86+
%token = begin_apply %coro(%0) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
87+
cond_br undef, bb1, bb2
88+
89+
bb1:
90+
destroy_value %0 : $Builtin.NativeObject
91+
abort_apply %token
92+
br bb3
93+
94+
bb2:
95+
end_apply %token
96+
destroy_value %0 : $Builtin.NativeObject
97+
br bb3
98+
99+
bb3:
100+
%r = tuple ()
101+
return %r : $()
102+
}

0 commit comments

Comments
 (0)