Skip to content

Commit c2a6f9a

Browse files
committed
[ownership] Fix assert to test the right thing semantically in the linear lifetime checker.
I think this assert was just testing the wrong thing. Specifically, it is trying to say that either the given value has a consuming use or it is post-dominated by dead end blocks. Instead of checking that directly by using DeadEndBlocks::isDeadEnd(), it was using a different empty check that caused the assert to actually semantically say that: A value must have a lifetime ending use *or* the dead end blocks analysis must have found at least one block at all in the function that is reachable from a function terminating terminator (e.x.: return). This is clearly not the former and was causing the linear lifetime error to hit this assert in certain cases. <rdar://problem/70690127>
1 parent 4aa1ef3 commit c2a6f9a

File tree

2 files changed

+30
-2
lines changed

2 files changed

+30
-2
lines changed

lib/SIL/Verifier/LinearLifetimeChecker.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "swift/SIL/OwnershipUtils.h"
3030
#include "swift/SIL/SILBasicBlock.h"
3131
#include "swift/SIL/SILFunction.h"
32+
#include "swift/SIL/SILUndef.h"
3233
#include "llvm/ADT/DenseMap.h"
3334
#include "llvm/ADT/MapVector.h"
3435
#include "llvm/ADT/STLExtras.h"
@@ -549,7 +550,8 @@ LinearLifetimeChecker::Error LinearLifetimeChecker::checkValueImpl(
549550
Optional<function_ref<void(SILBasicBlock *)>> leakingBlockCallback,
550551
Optional<function_ref<void(Operand *)>>
551552
nonConsumingUseOutsideLifetimeCallback) {
552-
assert((!consumingUses.empty() || !deadEndBlocks.empty()) &&
553+
assert((!consumingUses.empty()
554+
|| deadEndBlocks.isDeadEnd(value->getParentBlock())) &&
553555
"Must have at least one consuming user?!");
554556

555557
State state(value, visitedBlocks, errorBuilder, leakingBlockCallback,

test/SILOptimizer/semantic-arc-opts.sil

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ protocol MyFakeAnyObject : Klass {
4141

4242
final class Klass {
4343
var base: Klass
44+
let baseLet: Klass
4445
}
4546

4647
extension Klass : MyFakeAnyObject {
@@ -2805,4 +2806,29 @@ bb2(%2 : @owned ${ var Builtin.NativeObject }):
28052806
apply %user(%4) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
28062807
destroy_value %2 : ${ var Builtin.NativeObject }
28072808
unreachable
2808-
}
2809+
}
2810+
2811+
// Just make sure that we do not crash on this code and convert the 2nd load
2812+
// [copy] to a load_borrow.
2813+
//
2814+
// CHECK-LABEL: sil [ossa] @inproper_dead_end_block_crasher_test : $@convention(thin) (Builtin.RawPointer) -> () {
2815+
// CHECK: load_borrow
2816+
// CHECK: load_borrow
2817+
// CHECK: } // end sil function 'inproper_dead_end_block_crasher_test'
2818+
sil [ossa] @inproper_dead_end_block_crasher_test : $@convention(thin) (Builtin.RawPointer) -> () {
2819+
bb0(%0 : $Builtin.RawPointer):
2820+
%1 = pointer_to_address %0 : $Builtin.RawPointer to [strict] $*Klass
2821+
%2 = load_borrow %1 : $*Klass
2822+
%3 = ref_element_addr %2 : $Klass, #Klass.baseLet
2823+
%4 = load [copy] %3 : $*Klass
2824+
%f = function_ref @guaranteed_klass_user : $@convention(thin) (@guaranteed Klass) -> ()
2825+
apply %f(%4) : $@convention(thin) (@guaranteed Klass) -> ()
2826+
destroy_value %4 : $Klass
2827+
cond_br undef, bb1, bb2
2828+
2829+
bb1:
2830+
unreachable
2831+
2832+
bb2:
2833+
unreachable
2834+
}

0 commit comments

Comments
 (0)