Skip to content

Commit a7a13ea

Browse files
meg-guptameg-gupta
committed
Add memory lifetime verification support for borrow accessors
1 parent 259f9e3 commit a7a13ea

File tree

7 files changed

+157
-38
lines changed

7 files changed

+157
-38
lines changed

‎include/swift/SIL/SILFunctionConventions.h‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -338,15 +338,18 @@ class SILFunctionConventions {
338338
}
339339

340340
bool hasAddressResult() const {
341+
return hasGuaranteedAddressResult() || hasInoutResult();
342+
}
343+
344+
bool hasGuaranteedAddressResult() const {
341345
if (funcTy->getNumResults() != 1) {
342346
return false;
343347
}
344-
auto resultConvention = funcTy->getResults()[0].getConvention();
345-
if (silConv.loweredAddresses) {
346-
return resultConvention == ResultConvention::GuaranteedAddress ||
347-
resultConvention == ResultConvention::Inout;
348+
if (!silConv.loweredAddresses) {
349+
return false;
348350
}
349-
return resultConvention == ResultConvention::Inout;
351+
auto resultConvention = funcTy->getResults()[0].getConvention();
352+
return resultConvention == ResultConvention::GuaranteedAddress;
350353
}
351354

352355
struct SILResultTypeFunc;

‎include/swift/SIL/SILInstruction.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3155,6 +3155,10 @@ class ApplyInst final
31553155
return getSubstCalleeConv().hasGuaranteedResult();
31563156
}
31573157

3158+
bool hasGuaranteedAddressResult() const {
3159+
return getSubstCalleeConv().hasGuaranteedAddressResult();
3160+
}
3161+
31583162
bool hasAddressResult() const {
31593163
return getSubstCalleeConv().hasAddressResult();
31603164
}

‎lib/SIL/Utils/MemoryLocations.cpp‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,15 @@ MemoryLocations::Location::Location(SILValue val, unsigned index, int parentIdx)
6666
representativeValue(val),
6767
parentIdx(parentIdx) {
6868
assert(((parentIdx >= 0) ==
69-
(isa<StructElementAddrInst>(val) || isa<TupleElementAddrInst>(val) ||
70-
isa<InitEnumDataAddrInst>(val) || isa<UncheckedTakeEnumDataAddrInst>(val) ||
71-
isa<InitExistentialAddrInst>(val) || isa<OpenExistentialAddrInst>(val)))
72-
&& "sub-locations can only be introduced with struct/tuple/enum projections");
69+
(isa<StructElementAddrInst>(val) || isa<TupleElementAddrInst>(val) ||
70+
isa<InitEnumDataAddrInst>(val) ||
71+
isa<UncheckedTakeEnumDataAddrInst>(val) ||
72+
isa<InitExistentialAddrInst>(val) ||
73+
isa<OpenExistentialAddrInst>(val) || isa<ApplyInst>(val) ||
74+
isa<StoreBorrowInst>(val))) &&
75+
"sub-locations can only be introduced with "
76+
"struct/tuple/enum/store_borrow/borrow accessor "
77+
"projections");
7378
setBitAndResize(subLocations, index);
7479
setBitAndResize(selfAndParents, index);
7580
}
@@ -349,6 +354,21 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx
349354
if (cast<DebugValueInst>(user)->hasAddrVal())
350355
break;
351356
return false;
357+
case SILInstructionKind::ApplyInst: {
358+
auto *apply = cast<ApplyInst>(user);
359+
if (apply->hasAddressResult()) {
360+
if (!analyzeAddrProjection(apply, locIdx, 0, collectedVals,
361+
subLocationMap))
362+
return false;
363+
}
364+
break;
365+
}
366+
case SILInstructionKind::StoreBorrowInst: {
367+
if (!analyzeAddrProjection(cast<StoreBorrowInst>(user), locIdx, 0,
368+
collectedVals, subLocationMap))
369+
return false;
370+
break;
371+
}
352372
case SILInstructionKind::InjectEnumAddrInst:
353373
case SILInstructionKind::SelectEnumAddrInst:
354374
case SILInstructionKind::ExistentialMetatypeInst:
@@ -357,20 +377,19 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx
357377
case SILInstructionKind::FixLifetimeInst:
358378
case SILInstructionKind::LoadInst:
359379
case SILInstructionKind::StoreInst:
360-
case SILInstructionKind::StoreBorrowInst:
361380
case SILInstructionKind::EndAccessInst:
362381
case SILInstructionKind::DestroyAddrInst:
363382
case SILInstructionKind::CheckedCastAddrBranchInst:
364383
case SILInstructionKind::UncheckedRefCastAddrInst:
365384
case SILInstructionKind::UnconditionalCheckedCastAddrInst:
366-
case SILInstructionKind::ApplyInst:
367385
case SILInstructionKind::TryApplyInst:
368386
case SILInstructionKind::BeginApplyInst:
369387
case SILInstructionKind::CopyAddrInst:
370388
case SILInstructionKind::YieldInst:
371389
case SILInstructionKind::DeallocStackInst:
372390
case SILInstructionKind::SwitchEnumAddrInst:
373391
case SILInstructionKind::WitnessMethodInst:
392+
case SILInstructionKind::EndBorrowInst:
374393
break;
375394
case SILInstructionKind::MarkUnresolvedMoveAddrInst:
376395
// We do not want the memory lifetime verifier to verify move_addr inst

‎lib/SIL/Verifier/MemoryLifetimeVerifier.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ bool MemoryLifetimeVerifier::applyMayRead(Operand *argOp, SILValue addr) {
335335

336336
void MemoryLifetimeVerifier::requireNoGuaranteedAddressLocation(
337337
SILValue addr, SILInstruction *where) {
338-
if (isa<StoreBorrowInst>(addr)) {
338+
if (isStoreBorrowLocation(addr)) {
339339
reportError("store-borrow location cannot be written",
340340
locations.getLocationIdx(addr), where);
341341
}

‎test/SIL/OwnershipVerifier/borrow_accessor.sil‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,26 +8,14 @@ public struct Wrapper {
88
var k: Klass
99
}
1010

11-
public struct GenWrapper<T> {
12-
@_hasStorage var _prop: T { get set }
13-
public var prop: T
14-
}
15-
1611
sil [ossa] @borrow_loadable_prop : $@convention(method) (@guaranteed Wrapper) -> @guaranteed Klass {
1712
bb0(%0 : @guaranteed $Wrapper):
1813
%2 = struct_extract %0, #Wrapper._k
1914
return %2
2015
}
2116

22-
sil [ossa] @borrow_addressonly_prop : $@convention(method) <T> (@in_guaranteed GenWrapper<T>) -> @guaranteed_address T {
23-
bb0(%0 : $*GenWrapper<T>):
24-
%2 = struct_element_addr %0, #GenWrapper._prop
25-
return %2
26-
}
27-
2817
sil @get_wrapper : $@convention(thin) () -> @owned Klass
2918
sil @use_klass : $@convention(thin) (@guaranteed Klass) -> ()
30-
sil @use_T : $@convention(thin) <T> (@in_guaranteed T) -> ()
3119

3220
// CHECK-LABEL: Error#: 0. Begin Error in Function: 'test_end_borrow_on_guaranteed_return_value'
3321
// CHECK: Invalid End Borrow!
@@ -151,15 +139,3 @@ bb0(%0 : @owned $Wrapper):
151139
return %6
152140
}
153141

154-
// TODO: Add verification support in MemoryLifetimeVerifier
155-
sil [ossa] @test_use_after_free_address_only : $@convention(thin) <T> (@in GenWrapper<T>) -> () {
156-
bb0(%0 : $*GenWrapper<T>):
157-
%1 = function_ref @borrow_addressonly_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0
158-
%2 = apply %1<T>(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0
159-
%3 = function_ref @use_T : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
160-
destroy_addr %0
161-
%5 = apply %3<T>(%2) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
162-
%6 = tuple ()
163-
return %6
164-
}
165-

‎test/SIL/memory_lifetime.sil‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,17 @@ bb0(%0 : @guaranteed $T):
532532
return %res : $()
533533
}
534534

535+
sil [ossa] @test_store_borrow_nested : $@convention(thin) (@guaranteed Inner) -> () {
536+
bb0(%0 : @guaranteed $Inner):
537+
%s = alloc_stack $Inner
538+
%sb = store_borrow %0 to %s
539+
%elem = struct_element_addr %sb, #Inner.a
540+
end_borrow %sb
541+
dealloc_stack %s
542+
%res = tuple ()
543+
return %res : $()
544+
}
545+
535546
sil [ossa] @test_cast_br_take_always : $@convention(thin) <U, V> (@in U) -> () {
536547
bb0(%0 : $*U):
537548
%s = alloc_stack $V

‎test/SIL/memory_lifetime_failures.sil‎

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,24 @@ struct Mixed {
2929
var i: Int
3030
}
3131

32+
public struct InnerWrapper<T> {
33+
@_hasStorage var _prop: T { get set }
34+
public var prop: T
35+
}
36+
37+
public struct GenWrapper<T> {
38+
@_hasStorage var _prop: T { get set }
39+
public var prop: T
40+
41+
@_hasStorage var _nestedProp: InnerWrapper<T> { get set }
42+
public var nestedProp: InnerWrapper<T>
43+
}
44+
3245
sil @use_owned : $@convention(thin) (@owned T) -> ()
3346
sil @use_guaranteed : $@convention(thin) (@guaranteed T) -> ()
3447
sil @get_owned : $@convention(thin) () -> @owned T
48+
sil @use_T : $@convention(thin) <T> (@in_guaranteed T) -> ()
49+
sil @mutate_T : $@convention(thin) <T> (@inout T) -> ()
3550

3651
// CHECK: SIL memory lifetime failure in @test_simple: indirect argument is not alive at function return
3752
sil [ossa] @test_simple : $@convention(thin) (@inout T) -> @owned T {
@@ -276,8 +291,8 @@ bb0(%0 : $*T):
276291
return %res : $()
277292
}
278293

279-
// CHECK: SIL memory lifetime failure in @test_store_borrow_destroy: store-borrow location cannot be written
280-
sil [ossa] @test_store_borrow_destroy : $@convention(thin) (@guaranteed T) -> () {
294+
// CHECK: SIL memory lifetime failure in @test_store_borrow_destroy1: store-borrow location cannot be written
295+
sil [ossa] @test_store_borrow_destroy1 : $@convention(thin) (@guaranteed T) -> () {
281296
bb0(%0 : @guaranteed $T):
282297
%s = alloc_stack $T
283298
%sb = store_borrow %0 to %s : $*T
@@ -288,6 +303,19 @@ bb0(%0 : @guaranteed $T):
288303
return %res : $()
289304
}
290305

306+
// CHECK: SIL memory lifetime failure in @test_store_borrow_destroy2: store-borrow location cannot be written
307+
sil [ossa] @test_store_borrow_destroy2 : $@convention(thin) (@guaranteed Inner) -> () {
308+
bb0(%0 : @guaranteed $Inner):
309+
%s = alloc_stack $Inner
310+
%sb = store_borrow %0 to %s
311+
%elem = struct_element_addr %sb, #Inner.a
312+
destroy_addr %elem
313+
end_borrow %sb
314+
dealloc_stack %s
315+
%res = tuple ()
316+
return %res : $()
317+
}
318+
291319
sil [ossa] @func_with_inout_param : $@convention(thin) (@inout T) -> ()
292320

293321
// T-CHECK: SIL memory lifetime failure in @test_store_borrow_inout: store-borrow location cannot be written
@@ -842,3 +870,81 @@ bb0(%0 : @owned $T, %1 : @owned $Inner):
842870
dealloc_stack %2
843871
return %8
844872
}
873+
874+
sil [ossa] @borrow_addressonly_prop : $@convention(method) <T> (@in_guaranteed GenWrapper<T>) -> @guaranteed_address T {
875+
bb0(%0 : $*GenWrapper<T>):
876+
%2 = struct_element_addr %0, #GenWrapper._prop
877+
return %2
878+
}
879+
880+
sil [ossa] @mutate_addressonly_prop : $@convention(method) <T> (@inout GenWrapper<T>) -> @inout T {
881+
bb0(%0 : $*GenWrapper<T>):
882+
%2 = struct_element_addr %0, #GenWrapper._prop
883+
return %2
884+
}
885+
886+
sil [ossa] @borrow_addressonly_nested_prop : $@convention(method) <T> (@in_guaranteed GenWrapper<T>) -> @guaranteed_address InnerWrapper<T> {
887+
bb0(%0 : $*GenWrapper<T>):
888+
%2 = struct_element_addr %0, #GenWrapper._nestedProp
889+
return %2
890+
}
891+
892+
sil [ossa] @mutate_addressonly_nested_prop : $@convention(method) <T> (@inout GenWrapper<T>) -> @inout InnerWrapper<T> {
893+
bb0(%0 : $*GenWrapper<T>):
894+
%2 = struct_element_addr %0, #GenWrapper._nestedProp
895+
return %2
896+
}
897+
898+
// CHECK: SIL memory lifetime failure in @test_use_after_free_address_only1: memory is not initialized, but should be
899+
// CHECK: memory location: %2 = apply %1<T>(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0
900+
// CHECK: at instruction: %5 = apply %3<T>(%2) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
901+
sil [ossa] @test_use_after_free_address_only1 : $@convention(thin) <T> (@in GenWrapper<T>) -> () {
902+
bb0(%0 : $*GenWrapper<T>):
903+
%1 = function_ref @borrow_addressonly_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0
904+
%2 = apply %1<T>(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0
905+
%3 = function_ref @use_T : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
906+
destroy_addr %0
907+
%5 = apply %3<T>(%2) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
908+
%6 = tuple ()
909+
return %6
910+
}
911+
912+
// CHECK: SIL memory lifetime failure in @test_use_after_free_address_only2: memory is not initialized, but should be
913+
// CHECK: memory location: %2 = apply %1<T>(%0) : $@convention(method) <τ_0_0> (@inout GenWrapper<τ_0_0>) -> @inout τ_0_0
914+
// CHECK: at instruction: %5 = apply %3<T>(%2) : $@convention(thin) <τ_0_0> (@inout τ_0_0) -> ()
915+
sil [ossa] @test_use_after_free_address_only2 : $@convention(thin) <T> (@in GenWrapper<T>) -> () {
916+
bb0(%0 : $*GenWrapper<T>):
917+
%1 = function_ref @mutate_addressonly_prop : $@convention(method) <τ_0_0> (@inout GenWrapper<τ_0_0>) -> @inout τ_0_0
918+
%2 = apply %1<T>(%0) : $@convention(method) <τ_0_0> (@inout GenWrapper<τ_0_0>) -> @inout τ_0_0
919+
%3 = function_ref @mutate_T : $@convention(thin) <τ_0_0> (@inout τ_0_0) -> ()
920+
destroy_addr %0
921+
%5 = apply %3<T>(%2) : $@convention(thin) <τ_0_0> (@inout τ_0_0) -> ()
922+
%6 = tuple ()
923+
return %6
924+
}
925+
926+
// TODO-CHECK: SIL memory lifetime failure in @test_guaranteed_address_consume: store-borrow location cannot be written
927+
// TODO-CHECK: memory location: %2 = apply %1<T>(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0
928+
// TODO-CHECK: at instruction: destroy_addr %2 : $*T
929+
sil [ossa] @test_guaranteed_address_consume : $@convention(thin) <T> (@in GenWrapper<T>) -> () {
930+
bb0(%0 : $*GenWrapper<T>):
931+
%1 = function_ref @borrow_addressonly_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0
932+
%2 = apply %1<T>(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address τ_0_0
933+
destroy_addr %2
934+
%4 = tuple ()
935+
return %4
936+
}
937+
938+
// TODO-CHECK: SIL memory lifetime failure in @test_guaranteed_nested_address_consume: store-borrow location cannot be written
939+
// TODO-CHECK: memory location: %3 = struct_element_addr %2 : $*InnerWrapper<T>, #InnerWrapper._prop
940+
// TODO-CHECK: at instruction: destroy_addr %3 : $*T
941+
sil [ossa] @test_guaranteed_nested_address_consume : $@convention(thin) <T> (@in GenWrapper<T>) -> () {
942+
bb0(%0 : $*GenWrapper<T>):
943+
%1 = function_ref @borrow_addressonly_nested_prop : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address InnerWrapper<τ_0_0>
944+
%2 = apply %1<T>(%0) : $@convention(method) <τ_0_0> (@in_guaranteed GenWrapper<τ_0_0>) -> @guaranteed_address InnerWrapper<τ_0_0>
945+
%3 = struct_element_addr %2, #InnerWrapper._prop
946+
destroy_addr %3
947+
%4 = tuple ()
948+
return %4
949+
}
950+

0 commit comments

Comments
 (0)