Skip to content

Commit d6736e9

Browse files
eecksteineeckstein
committed
MemoryLifetime: support partial_apply arguments in memory lifetime verification.
1 parent fa11bdf commit d6736e9

File tree

8 files changed

+99
-12
lines changed

8 files changed

+99
-12
lines changed

‎include/swift/SIL/ApplySite.h

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,37 @@ class ApplySite {
348348
return getSubstCalleeConv().getSILArgumentConvention(calleeArgIdx);
349349
}
350350

351+
/// Return the SILArgumentConvention for the given applied argument operand at
352+
/// the apply instruction.
353+
///
354+
/// For full applies, this is equivalent to `getArgumentConvention`. But for
355+
/// a partial_apply, the argument ownership convention at the partial_apply
356+
/// instruction itself is different from the argument convention of the callee.
357+
/// For details see the partial_apply documentation in SIL.rst.
358+
SILArgumentConvention getArgumentOperandConvention(const Operand &oper) const {
359+
SILArgumentConvention conv = getArgumentConvention(oper);
360+
auto *pai = dyn_cast<PartialApplyInst>(Inst);
361+
if (!pai)
362+
return conv;
363+
switch (conv) {
364+
case SILArgumentConvention::Indirect_Inout:
365+
case SILArgumentConvention::Indirect_InoutAliasable:
366+
return conv;
367+
case SILArgumentConvention::Direct_Owned:
368+
case SILArgumentConvention::Direct_Unowned:
369+
case SILArgumentConvention::Direct_Guaranteed:
370+
return pai->isOnStack() ? SILArgumentConvention::Direct_Guaranteed
371+
: SILArgumentConvention::Direct_Owned;
372+
case SILArgumentConvention::Indirect_In:
373+
case SILArgumentConvention::Indirect_In_Constant:
374+
case SILArgumentConvention::Indirect_In_Guaranteed:
375+
return pai->isOnStack() ? SILArgumentConvention::Indirect_In_Guaranteed
376+
: SILArgumentConvention::Indirect_In;
377+
case SILArgumentConvention::Indirect_Out:
378+
llvm_unreachable("partial_apply cannot have an @out operand");
379+
}
380+
}
381+
351382
/// Return true if 'self' is an applied argument.
352383
bool hasSelfArgument() const {
353384
switch (ApplySiteKind(Inst->getKind())) {

‎lib/SIL/Verifier/MemoryLifetime.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx
350350
case SILInstructionKind::EndAccessInst:
351351
case SILInstructionKind::LoadBorrowInst:
352352
case SILInstructionKind::DestroyAddrInst:
353+
case SILInstructionKind::PartialApplyInst:
353354
case SILInstructionKind::ApplyInst:
354355
case SILInstructionKind::TryApplyInst:
355356
case SILInstructionKind::BeginApplyInst:
@@ -870,12 +871,13 @@ void MemoryLifetimeVerifier::initDataflowInBlock(SILBasicBlock *block,
870871
case SILInstructionKind::DeallocStackInst:
871872
state.killBits(I.getOperand(0), locations);
872873
break;
874+
case SILInstructionKind::PartialApplyInst:
873875
case SILInstructionKind::ApplyInst:
874876
case SILInstructionKind::TryApplyInst: {
875-
FullApplySite FAS(&I);
877+
ApplySite AS(&I);
876878
for (Operand &op : I.getAllOperands()) {
877-
if (FAS.isArgumentOperand(op)) {
878-
setFuncOperandBits(state, op, FAS.getArgumentConvention(op),
879+
if (AS.isArgumentOperand(op)) {
880+
setFuncOperandBits(state, op, AS.getArgumentOperandConvention(op),
879881
isa<TryApplyInst>(&I));
880882
}
881883
}
@@ -1092,12 +1094,13 @@ void MemoryLifetimeVerifier::checkBlock(SILBasicBlock *block, Bits &bits) {
10921094
requireBitsSet(bits, orig, &I);
10931095
break;
10941096
}
1097+
case SILInstructionKind::PartialApplyInst:
10951098
case SILInstructionKind::ApplyInst:
10961099
case SILInstructionKind::TryApplyInst: {
1097-
FullApplySite FAS(&I);
1100+
ApplySite AS(&I);
10981101
for (Operand &op : I.getAllOperands()) {
1099-
if (FAS.isArgumentOperand(op))
1100-
checkFuncArgument(bits, op, FAS.getArgumentConvention(op), &I);
1102+
if (AS.isArgumentOperand(op))
1103+
checkFuncArgument(bits, op, AS.getArgumentOperandConvention(op), &I);
11011104
}
11021105
break;
11031106
}

‎lib/SILOptimizer/Transforms/DestroyHoisting.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ void DestroyHoisting::getUsedLocationsOfInst(Bits &bits, SILInstruction *I) {
352352
case SILInstructionKind::StoreInst:
353353
case SILInstructionKind::CopyAddrInst:
354354
case SILInstructionKind::InjectEnumAddrInst:
355+
case SILInstructionKind::PartialApplyInst:
355356
case SILInstructionKind::ApplyInst:
356357
case SILInstructionKind::TryApplyInst:
357358
case SILInstructionKind::YieldInst:

‎test/Reflection/capture_descriptors.sil

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,11 @@ bb0(%t : $*T):
111111
return %12 : $()
112112
}
113113

114-
sil [ossa] @generic_caller3 : $@convention(thin) <A, B, C> () -> @owned @callee_guaranteed () -> () {
115-
bb0:
114+
sil [ossa] @generic_caller3 : $@convention(thin) <A, B, C> (@owned Optional<@callee_guaranteed @substituted <X, Y> (@in_guaranteed X) -> @out Y for <A, B>>) -> @owned @callee_guaranteed () -> () {
115+
bb0(%0 : @owned $Optional<@callee_guaranteed @substituted <X, Y> (@in_guaranteed X) -> @out Y for <A, B>>):
116116
%f = function_ref @generic_callee3 : $@convention(thin) <T, U> (@in_guaranteed T) -> ()
117117
%t = alloc_stack $Optional<@callee_guaranteed @substituted <X, Y> (@in_guaranteed X) -> @out Y for <A, B>>
118+
store %0 to [init] %t : $*Optional<@callee_guaranteed @substituted <X, Y> (@in_guaranteed X) -> @out Y for <A, B>>
118119
%c = partial_apply [callee_guaranteed] %f<Optional<(A) -> B>, (B, (C) -> Int)>(%t) : $@convention(thin) <T, U> (@in_guaranteed T) -> ()
119120
dealloc_stack %t : $*Optional<@callee_guaranteed @substituted <X, Y> (@in_guaranteed X) -> @out Y for <A, B>>
120121
return %c : $@callee_guaranteed () -> ()

‎test/SIL/memory_lifetime.sil

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,3 +464,23 @@ bb0(%0 : @owned $T):
464464
return %r : $()
465465
}
466466

467+
sil [ossa] @closure : $@convention(thin) (@in_guaranteed T) -> ()
468+
469+
sil [ossa] @test_non_escaping_closure : $@convention(thin) (@in_guaranteed T) -> () {
470+
bb0(%0 : $*T):
471+
%func = function_ref @closure : $@convention(thin) (@in_guaranteed T) -> ()
472+
%pa = partial_apply [callee_guaranteed] [on_stack] %func(%0) : $@convention(thin) (@in_guaranteed T) -> ()
473+
dealloc_stack %pa : $@noescape @callee_guaranteed () -> ()
474+
%res = tuple ()
475+
return %res : $()
476+
}
477+
478+
sil [ossa] @test_escaping_closure : $@convention(thin) (@in T) -> () {
479+
bb0(%0 : $*T):
480+
%func = function_ref @closure : $@convention(thin) (@in_guaranteed T) -> ()
481+
%pa = partial_apply %func(%0) : $@convention(thin) (@in_guaranteed T) -> ()
482+
destroy_value %pa : $@callee_owned () -> ()
483+
%res = tuple ()
484+
return %res : $()
485+
}
486+

‎test/SIL/memory_lifetime_failures.sil

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,3 +238,25 @@ bb0(%0 : @owned $T):
238238
return %r : $()
239239
}
240240

241+
sil [ossa] @closure : $@convention(thin) (@in_guaranteed T) -> ()
242+
243+
// CHECK: SIL memory lifetime failure in @test_non_escaping_closure: memory is initialized at function return but shouldn't
244+
sil [ossa] @test_non_escaping_closure : $@convention(thin) (@in T) -> () {
245+
bb0(%0 : $*T):
246+
%func = function_ref @closure : $@convention(thin) (@in_guaranteed T) -> ()
247+
%pa = partial_apply [callee_guaranteed] [on_stack] %func(%0) : $@convention(thin) (@in_guaranteed T) -> ()
248+
dealloc_stack %pa : $@noescape @callee_guaranteed () -> ()
249+
%res = tuple ()
250+
return %res : $()
251+
}
252+
253+
// CHECK: SIL memory lifetime failure in @test_escaping_closure: indirect argument is not alive at function return
254+
sil [ossa] @test_escaping_closure : $@convention(thin) (@in_guaranteed T) -> () {
255+
bb0(%0 : $*T):
256+
%func = function_ref @closure : $@convention(thin) (@in_guaranteed T) -> ()
257+
%pa = partial_apply %func(%0) : $@convention(thin) (@in_guaranteed T) -> ()
258+
destroy_value %pa : $@callee_owned () -> ()
259+
%res = tuple ()
260+
return %res : $()
261+
}
262+

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-sil-opt -enable-sil-verify-all -inline %s -verify-continue-on-failure=true -o /dev/null 2>&1 | %FileCheck %s
1+
// RUN: %target-sil-opt -enable-sil-verify-all -inline %s -verify-continue-on-failure=true -dont-abort-on-memory-lifetime-errors -o /dev/null 2>&1 | %FileCheck %s
22
// REQUIRES: asserts
33

44
// Tests here are patterns we should not consider as broken

‎test/SILOptimizer/specialize_ossa.sil

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -702,7 +702,10 @@ bb1:
702702
%val = integer_literal $Builtin.Int64, 4
703703
store %val to [trivial] %val_storage : $*Builtin.Int64
704704
%fn = function_ref @selfReferringGenericClosure : $@convention(thin) <U, V> (@in_guaranteed U, @in_guaranteed V, Builtin.Int64) -> ()
705-
%7 = partial_apply %fn<R, Builtin.Int64>(%0, %val_storage, %4) : $@convention(thin) <U, V> (@in_guaranteed U, @in_guaranteed V, Builtin.Int64) -> ()
705+
%s = alloc_stack $R
706+
copy_addr %0 to [initialization] %s : $*R
707+
%7 = partial_apply %fn<R, Builtin.Int64>(%s, %val_storage, %4) : $@convention(thin) <U, V> (@in_guaranteed U, @in_guaranteed V, Builtin.Int64) -> ()
708+
dealloc_stack %s : $*R
706709
dealloc_stack %val_storage : $*Builtin.Int64
707710
destroy_value %7 : $@callee_owned () -> ()
708711
br bb3
@@ -725,7 +728,10 @@ bb1:
725728
%val2 = copy_value %2 : $Builtin.NativeObject
726729
store %val to [init] %val_storage : $*Builtin.NativeObject
727730
%fn = function_ref @selfReferringGenericClosure_nontrivial_guaranteed : $@convention(thin) <U, V> (@in_guaranteed U, @in_guaranteed V, @guaranteed Builtin.NativeObject) -> ()
728-
%7 = partial_apply %fn<R, Builtin.NativeObject>(%0, %val_storage, %val2) : $@convention(thin) <U, V> (@in_guaranteed U, @in_guaranteed V, @guaranteed Builtin.NativeObject) -> ()
731+
%s = alloc_stack $R
732+
copy_addr %0 to [initialization] %s : $*R
733+
%7 = partial_apply %fn<R, Builtin.NativeObject>(%s, %val_storage, %val2) : $@convention(thin) <U, V> (@in_guaranteed U, @in_guaranteed V, @guaranteed Builtin.NativeObject) -> ()
734+
dealloc_stack %s : $*R
729735
dealloc_stack %val_storage : $*Builtin.NativeObject
730736
destroy_value %7 : $@callee_owned () -> ()
731737
br bb3
@@ -748,7 +754,10 @@ bb1:
748754
%val2 = copy_value %2 : $Builtin.NativeObject
749755
store %val to [init] %val_storage : $*Builtin.NativeObject
750756
%fn = function_ref @selfReferringGenericClosure_nontrivial_guaranteed_applied : $@convention(thin) <U, V> (@in_guaranteed U, @in_guaranteed V, @guaranteed Builtin.NativeObject) -> ()
751-
%7 = partial_apply %fn<R, Builtin.NativeObject>(%0, %val_storage, %val2) : $@convention(thin) <U, V> (@in_guaranteed U, @in_guaranteed V, @guaranteed Builtin.NativeObject) -> ()
757+
%s = alloc_stack $R
758+
copy_addr %0 to [initialization] %s : $*R
759+
%7 = partial_apply %fn<R, Builtin.NativeObject>(%s, %val_storage, %val2) : $@convention(thin) <U, V> (@in_guaranteed U, @in_guaranteed V, @guaranteed Builtin.NativeObject) -> ()
760+
dealloc_stack %s : $*R
752761
apply %7() :$@callee_owned () -> ()
753762
dealloc_stack %val_storage : $*Builtin.NativeObject
754763
br bb3

0 commit comments

Comments
 (0)