From eb9f5b2a92727cc7b9ac7c44b19396d7b2d094f3 Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Mon, 25 Aug 2025 16:45:46 -0700 Subject: [PATCH 1/2] [AllocBoxToStack] Don't destroy in dead-ends. It is valid to leak a value on paths into dead-end regions. Specifically, it is valid to leak an `alloc_box`. Thus, "final releases" in dead-end regions may not destroy the box and consequently may not release its contents. Therefore it's invalid to lower such final releases to `dealloc_stack`s, let alone `destroy_addr`s. The in-general invalidity of that transformation results in miscompiling whenever a box is leaked and its projected address is used after such final releases. Fix this by not treating final releases as boundary markers of the `alloc_box` and not lowering them to `destroy_addr`s and `dealloc_stack`s. rdar://158149082 --- .../Transforms/AllocBoxToStack.cpp | 41 ++- test/SILOptimizer/allocbox_to_stack.sil | 203 ++++++++++++++- .../allocbox_to_stack_ownership.sil | 239 +++++++++++++++++- 3 files changed, 473 insertions(+), 10 deletions(-) diff --git a/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp b/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp index ae08e252c3104..976ce1329d4a8 100644 --- a/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp +++ b/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp @@ -25,6 +25,8 @@ #include "swift/SIL/SILArgument.h" #include "swift/SIL/SILBuilder.h" #include "swift/SIL/SILCloner.h" +#include "swift/SILOptimizer/Analysis/DeadEndBlocksAnalysis.h" +#include "swift/SILOptimizer/Analysis/LoopAnalysis.h" #include "swift/SILOptimizer/PassManager/Passes.h" #include "swift/SILOptimizer/PassManager/Transforms.h" #include "swift/SILOptimizer/Utils/InstOptUtils.h" @@ -601,7 +603,9 @@ static void hoistMarkUnresolvedNonCopyableValueInsts( /// rewriteAllocBoxAsAllocStack - Replace uses of the alloc_box with a /// new alloc_stack, but do not delete the alloc_box yet. -static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI) { +static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI, + DeadEndBlocksAnalysis &deba, + SILLoopAnalysis &la) { LLVM_DEBUG(llvm::dbgs() << "*** Promoting alloc_box to stack: " << *ABI); SILValue HeapBox = ABI; @@ -693,9 +697,31 @@ static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI) { ABI->getBoxType(), ABI->getModule().Types, 0)); auto Loc = CleanupLocation(ABI->getLoc()); + auto *deb = deba.get(ABI->getFunction()); for (auto LastRelease : FinalReleases) { + auto *dbi = dyn_cast(LastRelease); + if (!dbi && deb->isDeadEnd(LastRelease->getParent()) && + !la.get(ABI->getFunction())->getLoopFor(LastRelease->getParent())) { + // "Last" releases in dead-end regions may not actually destroy the box + // and consequently may not actually release the stored value. That's + // because values (including boxes) may be leaked along paths into + // dead-end regions. Thus it is invalid to lower such final releases of + // the box to destroy_addr's/dealloc_box's of the stack-promoted storage. + // + // There is one exception: if the alloc_box is in a dead-end loop. In + // that case SIL invariants require that the final releases actually + // destroy the box; otherwise, a box would leak once per loop. To check + // for this, it is sufficient check that the LastRelease is in a dead-end + // loop: if the alloc_box is not in that loop, then the entire loop is in + // the live range, so no release within the loop would be a "final + // release". + // + // None of this applies to dealloc_box instructions which always destroy + // the box. + continue; + } SILBuilderWithScope Builder(LastRelease); - if (!isa(LastRelease)&& !Lowering.isTrivial()) { + if (!dbi && !Lowering.isTrivial()) { // If we have a mark_unresolved_non_copyable_value use of our stack box, // we want to destroy that. SILValue valueToDestroy = StackBox; @@ -709,7 +735,6 @@ static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI) { // instruction we found that isn't an explicit dealloc_box. Builder.emitDestroyAddrAndFold(Loc, valueToDestroy); } - auto *dbi = dyn_cast(LastRelease); if (dbi && dbi->isDeadEnd()) { // Don't bother to create dealloc_stack instructions in dead-ends. continue; @@ -1265,7 +1290,9 @@ static void rewriteApplySites(AllocBoxToStackState &pass) { /// Clone closure bodies and rewrite partial applies. Returns the number of /// alloc_box allocations promoted. -static unsigned rewritePromotedBoxes(AllocBoxToStackState &pass) { +static unsigned rewritePromotedBoxes(AllocBoxToStackState &pass, + DeadEndBlocksAnalysis &deba, + SILLoopAnalysis &la) { // First we'll rewrite any ApplySite that we can to remove // the box container pointer from the operands. rewriteApplySites(pass); @@ -1274,7 +1301,7 @@ static unsigned rewritePromotedBoxes(AllocBoxToStackState &pass) { auto rend = pass.Promotable.rend(); for (auto I = pass.Promotable.rbegin(); I != rend; ++I) { auto *ABI = *I; - if (rewriteAllocBoxAsAllocStack(ABI)) { + if (rewriteAllocBoxAsAllocStack(ABI, deba, la)) { ++Count; ABI->eraseFromParent(); } @@ -1299,7 +1326,9 @@ class AllocBoxToStack : public SILFunctionTransform { } if (!pass.Promotable.empty()) { - auto Count = rewritePromotedBoxes(pass); + auto *deba = getAnalysis(); + auto *la = getAnalysis(); + auto Count = rewritePromotedBoxes(pass, *deba, *la); NumStackPromoted += Count; if (Count) { if (StackNesting::fixNesting(getFunction()) == StackNesting::Changes::CFG) diff --git a/test/SILOptimizer/allocbox_to_stack.sil b/test/SILOptimizer/allocbox_to_stack.sil index 5be2d84f663a5..f35046e99a698 100644 --- a/test/SILOptimizer/allocbox_to_stack.sil +++ b/test/SILOptimizer/allocbox_to_stack.sil @@ -14,6 +14,8 @@ struct Bool { protocol Error {} +class C {} + // CHECK-LABEL: sil @simple_promotion sil @simple_promotion : $(Int) -> Int { bb0(%0 : $Int): @@ -901,7 +903,6 @@ bb2: // CHECK: bb1: // CHECK-NEXT: [[STACK2:%[0-9]+]] = alloc_stack $Bool // CHECK-NEXT: dealloc_stack [[STACK2]] -// CHECK-NEXT: dealloc_stack [[BOX]] // CHECK-NEXT: unreachable // CHECK: bb2: // CHECK: store {{%[0-9]+}} @@ -1059,10 +1060,8 @@ bb3: // CHECK-NEXT: [[AI:%[0-9]+]] = alloc_stack $Int // CHECK-NEXT: cond_br // CHECK: bb1: -// CHECK-NEXT: dealloc_stack [[AI]] // CHECK-NEXT: br bb3 // CHECK: bb2: -// CHECK-NEXT: dealloc_stack [[AI]] // CHECK-NEXT: br bb3 // CHECK: bb3: // CHECK-NEXT: store {{%[0-9]+}} @@ -1226,3 +1225,201 @@ bb0: destroy_value %box : ${ var Builtin.Int32 } return %value : $Builtin.Int32 } + +sil @getC : $@convention(thin) () -> (@owned C) + +// CHECK-LABEL: sil @leak_to_inf_loop_1 : {{.*}} { +// CHECK-NOT: destroy_addr +// CHECK-NOT: dealloc_stack +// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_1' +sil @leak_to_inf_loop_1 : $@convention(thin) (@owned C) -> () { +entry(%c : $C): + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + store %c to %c_addr + strong_retain %box + strong_release %box + br loop + +loop: + %getC = function_ref @getC : $@convention(thin) () -> (@owned C) + %c2 = apply %getC() : $@convention(thin) () -> (@owned C) + %c_old = load %c_addr + store %c2 to %c_addr + release_value %c_old + br loop +} + +// CHECK-LABEL: sil @leak_to_inf_loop_2 : {{.*}} { +// CHECK: cond_br undef, [[EXIT:bb[0-9]+]], [[TO_LOOP:bb[0-9]+]] +// CHECK: [[EXIT]] +// CHECK: destroy_addr +// CHECK: dealloc_stack +// CHECK: [[TO_LOOP]] +// CHECK-NOT: destroy_addr +// CHECK-NOT: dealloc_stack +// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_2' +sil @leak_to_inf_loop_2 : $@convention(thin) (@owned C) -> () { +entry(%c : $C): + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + store %c to %c_addr + strong_retain %box + strong_release %box + cond_br undef, exit, to_loop + +exit: + strong_release %box + return undef : $() + +to_loop: + strong_retain %box + strong_release %box + br loop + +loop: + %getC = function_ref @getC : $@convention(thin) () -> (@owned C) + %c2 = apply %getC() : $@convention(thin) () -> (@owned C) + %c_old = load %c_addr + store %c2 to %c_addr + release_value %c_old + br loop +} + +// CHECK-LABEL: sil @leak_to_inf_loop_3 : {{.*}} { +// CHECK-NOT: destroy_addr +// CHECK-NOT: dealloc_stack +// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_3' +sil @leak_to_inf_loop_3 : $@convention(thin) (@owned C) -> () { +entry(%c : $C): + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + store %c to %c_addr + br to_loop + +to_loop: + strong_retain %box + strong_release %box + br loop + +loop: + %getC = function_ref @getC : $@convention(thin) () -> (@owned C) + %c2 = apply %getC() : $@convention(thin) () -> (@owned C) + %c_old = load %c_addr + store %c2 to %c_addr + release_value %c_old + br loop +} + +// CHECK-LABEL: sil @dealloc_box_before_loop +// CHECK: dealloc_stack +// CHECK-LABEL: } // end sil function 'dealloc_box_before_loop' +sil @dealloc_box_before_loop : $@convention(thin) (@owned C) -> () { +entry(%c : $C): + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + store %c to %c_addr + cond_br undef, exit, to_loop + +exit: + strong_release %box + return undef : $() + +to_loop: + dealloc_box %box + br loop + +loop: + br loop +} + +// CHECK-LABEL: sil @alloc_box_in_deadend_loop +// CHECK: alloc_stack +// CHECK: dealloc_stack +// CHECK-LABEL: } // end sil function 'alloc_box_in_deadend_loop' +sil @alloc_box_in_deadend_loop : $@convention(thin) () -> () { +entry: + br loop + +loop: + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + %getC = function_ref @getC : $@convention(thin) () -> (@owned C) + %c = apply %getC() : $@convention(thin) () -> (@owned C) + store %c to %c_addr + strong_release %box + br loop +} + +// CHECK-LABEL: sil @alloc_box_in_exiting_loop +// CHECK: br [[EXITING_LOOP:bb[0-9]+]] +// CHECK: [[EXITING_LOOP]]: +// CHECK: alloc_stack +// CHECK: cond_br undef, [[EXIT:bb[0-9]+]], [[LATCH:bb[0-9]+]] +// CHECK: [[LATCH]]: +// CHECK: cond_br undef, [[BACKEDGE:bb[0-9]+]], [[INFINITE_LOOP:bb[0-9]+]] +// CHECK: [[BACKEDGE]]: +// CHECK: dealloc_stack +// CHECK: br [[EXITING_LOOP]] +// CHECK: [[EXIT]]: +// CHECK: dealloc_stack +// CHECK: [[INFINITE_LOOP]]: +// CHECK-NOT: dealloc_stack +// CHECK-LABEL: } // end sil function 'alloc_box_in_exiting_loop' +sil @alloc_box_in_exiting_loop : $@convention(thin) () -> () { +entry: + br exiting_loop + +exiting_loop: + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + cond_br undef, exit, latch + +latch: + cond_br undef, backedge, infinite_loop + +backedge: + strong_release %box + br exiting_loop + +exit: + strong_release %box + return undef : $() + +infinite_loop: + %getC = function_ref @getC : $@convention(thin) () -> (@owned C) + %c = apply %getC() : $@convention(thin) () -> (@owned C) + store %c to %c_addr + strong_retain %box + strong_release %box + br infinite_loop +} + +// CHECK-LABEL: sil @alloc_box_in_deadend_loop_with_another_deadend +// CHECK: br [[LOOP:bb[0-9]+]] +// CHECK: [[LOOP]]: +// CHECK: alloc_stack +// CHECK: cond_br undef, [[BACKEDGE:bb[0-9]+]], [[DIE:bb[0-9]+]] +// CHECK: [[BACKEDGE]]: +// CHECK: dealloc_stack +// CHECK: br [[LOOP]] +// CHECK: [[DIE]]: +// CHECK-NOT: dealloc_stack +// CHECK-LABEL: } // end sil function 'alloc_box_in_deadend_loop_with_another_deadend' +sil @alloc_box_in_deadend_loop_with_another_deadend : $@convention(thin) () -> () { +entry: + br loop + +loop: + %box = alloc_box ${ var C } + cond_br undef, backedge, die + +backedge: + strong_release %box + br loop + +die: + strong_retain %box + strong_release %box + unreachable +} diff --git a/test/SILOptimizer/allocbox_to_stack_ownership.sil b/test/SILOptimizer/allocbox_to_stack_ownership.sil index a50f11903468b..d8ae775d7cb5a 100644 --- a/test/SILOptimizer/allocbox_to_stack_ownership.sil +++ b/test/SILOptimizer/allocbox_to_stack_ownership.sil @@ -14,6 +14,8 @@ struct Bool { protocol Error {} +class C {} + // CHECK-LABEL: sil [ossa] @simple_promotion sil [ossa] @simple_promotion : $@convention(thin) (Int) -> Int { bb0(%0 : $Int): @@ -1006,7 +1008,6 @@ bb2: // CHECK: bb1: // CHECK-NEXT: [[STACK2:%[0-9]+]] = alloc_stack $Bool // CHECK-NEXT: dealloc_stack [[STACK2]] -// CHECK-NEXT: dealloc_stack [[BOX]] // CHECK-NEXT: unreachable // CHECK: bb2: // CHECK: store @@ -1203,3 +1204,239 @@ bb0(%0 : $*T, %1 : $*T, %2 : $*T): %15 = tuple () return %15 : $() } + +// CHECK-LABEL: sil [ossa] @alloc_box_in_specialized_callee : +// CHECK-NOT: alloc_box +// CHECK-LABEL: } // end sil function 'alloc_box_in_specialized_callee' +sil [ossa] @alloc_box_in_specialized_callee : $@convention(thin) (Int) -> () { +bb0(%0 : $Int): + %1 = alloc_box ${ var Int } + %2 = project_box %1, 0 + store %0 to [trivial] %2 + %4 = function_ref @callee_with_allocbox : $@convention(thin) (@guaranteed { var Int }) -> () + %5 = apply %4(%1) : $@convention(thin) (@guaranteed { var Int }) -> () + destroy_value %1 + %r = tuple () + return %r +} + +// CHECK-LABEL: sil shared [ossa] @$s20callee_with_allocboxTf0s_n : +// CHECK-NOT: alloc_box +// CHECK-LABEL: } // end sil function '$s20callee_with_allocboxTf0s_n' + +// CHECK-LABEL: sil [ossa] @callee_with_allocbox : +// CHECK-NOT: alloc_box +// CHECK-LABEL: } // end sil function 'callee_with_allocbox' +sil [ossa] @callee_with_allocbox : $@convention(thin) (@guaranteed { var Int }) -> () { +bb0(%0 : @guaranteed ${ var Int }): + %1 = alloc_box ${ var Int } + %2 = project_box %1 : ${ var Int }, 0 + %3 = project_box %0 : ${ var Int }, 0 + copy_addr %3 to %2 + destroy_value %1 + %r = tuple () + return %r +} + +sil @getC : $@convention(thin) () -> (@owned C) + +// CHECK-LABEL: sil [ossa] @leak_to_inf_loop_1 : {{.*}} { +// CHECK-NOT: destroy_addr +// CHECK-NOT: dealloc_stack +// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_1' +sil [ossa] @leak_to_inf_loop_1 : $@convention(thin) (@owned C) -> () { +entry(%c : @owned $C): + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + store %c to [init] %c_addr + %copy = copy_value %box + destroy_value %box + br loop + +loop: + %getC = function_ref @getC : $@convention(thin) () -> (@owned C) + %c2 = apply %getC() : $@convention(thin) () -> (@owned C) + %c_old = load [take] %c_addr + store %c2 to [init] %c_addr + destroy_value %c_old + br loop +} + +// CHECK-LABEL: sil [ossa] @leak_to_inf_loop_2 : {{.*}} { +// CHECK: cond_br undef, [[EXIT:bb[0-9]+]], [[TO_LOOP:bb[0-9]+]] +// CHECK: [[EXIT]] +// CHECK: destroy_addr +// CHECK: dealloc_stack +// CHECK: [[TO_LOOP]] +// CHECK-NOT: destroy_addr +// CHECK-NOT: dealloc_stack +// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_2' +sil [ossa] @leak_to_inf_loop_2 : $@convention(thin) (@owned C) -> () { +entry(%c : @owned $C): + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + store %c to [init] %c_addr + %copy = copy_value %box + destroy_value %box + cond_br undef, exit, to_loop + +exit: + destroy_value %copy + return undef : $() + +to_loop: + %copy2 = copy_value %copy + destroy_value %copy + br loop + +loop: + %getC = function_ref @getC : $@convention(thin) () -> (@owned C) + %c2 = apply %getC() : $@convention(thin) () -> (@owned C) + %c_old = load [take] %c_addr + store %c2 to [init] %c_addr + destroy_value %c_old + br loop +} + +// CHECK-LABEL: sil [ossa] @leak_to_inf_loop_3 : {{.*}} { +// CHECK-NOT: destroy_addr +// CHECK-NOT: dealloc_stack +// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_3' +sil [ossa] @leak_to_inf_loop_3 : $@convention(thin) (@owned C) -> () { +entry(%c : @owned $C): + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + store %c to [init] %c_addr + br to_loop + +to_loop: + %copy = copy_value %box + destroy_value %box + br loop + +loop: + %getC = function_ref @getC : $@convention(thin) () -> (@owned C) + %c2 = apply %getC() : $@convention(thin) () -> (@owned C) + %c_old = load [take] %c_addr + store %c2 to [init] %c_addr + destroy_value %c_old + br loop +} + +// CHECK-LABEL: sil [ossa] @dealloc_box_before_loop +// CHECK: dealloc_stack +// CHECK-LABEL: } // end sil function 'dealloc_box_before_loop' +sil [ossa] @dealloc_box_before_loop : $@convention(thin) (@owned C) -> () { +entry(%c : @owned $C): + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + store %c to [init] %c_addr + cond_br undef, exit, to_loop + +exit: + destroy_value %box + return undef : $() + +to_loop: + dealloc_box %box + br loop + +loop: + br loop +} + +// CHECK-LABEL: sil [ossa] @alloc_box_in_deadend_loop +// CHECK: alloc_stack +// CHECK: dealloc_stack +// CHECK-LABEL: } // end sil function 'alloc_box_in_deadend_loop' +sil [ossa] @alloc_box_in_deadend_loop : $@convention(thin) () -> () { +entry: + br loop + +loop: + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + %getC = function_ref @getC : $@convention(thin) () -> (@owned C) + %c = apply %getC() : $@convention(thin) () -> (@owned C) + store %c to [init] %c_addr + destroy_value %box + br loop +} + +// CHECK-LABEL: sil [ossa] @alloc_box_in_exiting_loop +// CHECK: br [[EXITING_LOOP:bb[0-9]+]] +// CHECK: [[EXITING_LOOP]]: +// CHECK: alloc_stack +// CHECK: cond_br undef, [[EXIT:bb[0-9]+]], [[LATCH:bb[0-9]+]] +// CHECK: [[LATCH]]: +// CHECK: cond_br undef, [[BACKEDGE:bb[0-9]+]], [[INFINITE_LOOP:bb[0-9]+]] +// CHECK: [[BACKEDGE]]: +// CHECK: dealloc_stack +// CHECK: br [[EXITING_LOOP]] +// CHECK: [[EXIT]]: +// CHECK: dealloc_stack +// CHECK: [[INFINITE_LOOP]]: +// CHECK-NOT: dealloc_stack +// CHECK-LABEL: } // end sil function 'alloc_box_in_exiting_loop' +sil [ossa] @alloc_box_in_exiting_loop : $@convention(thin) () -> () { +entry: + br exiting_loop + +exiting_loop: + %box = alloc_box ${ var C } + %c_addr = project_box %box, 0 + %getC = function_ref @getC : $@convention(thin) () -> (@owned C) + %c = apply %getC() : $@convention(thin) () -> (@owned C) + store %c to [init] %c_addr + cond_br undef, exit, latch + +latch: + cond_br undef, backedge, to_infinite_loop + +backedge: + destroy_value %box + br exiting_loop + +exit: + destroy_value %box + return undef : $() + +to_infinite_loop: + br infinite_loop + +infinite_loop: + %c2 = apply %getC() : $@convention(thin) () -> (@owned C) + store %c2 to [assign] %c_addr + %copy = copy_value %box + destroy_value %copy + br infinite_loop +} + +// CHECK-LABEL: sil [ossa] @alloc_box_in_deadend_loop_with_another_deadend +// CHECK: br [[LOOP:bb[0-9]+]] +// CHECK: [[LOOP]]: +// CHECK: alloc_stack +// CHECK: cond_br undef, [[BACKEDGE:bb[0-9]+]], [[DIE:bb[0-9]+]] +// CHECK: [[BACKEDGE]]: +// CHECK: dealloc_stack +// CHECK: br [[LOOP]] +// CHECK: [[DIE]]: +// CHECK-NOT: dealloc_stack +// CHECK-LABEL: } // end sil function 'alloc_box_in_deadend_loop_with_another_deadend' +sil [ossa] @alloc_box_in_deadend_loop_with_another_deadend : $@convention(thin) () -> () { +entry: + br loop + +loop: + %box = alloc_box ${ var C } + cond_br undef, backedge, die + +backedge: + destroy_value %box + br loop + +die: + %copy = copy_value %box + destroy_value %copy + unreachable +} From 5851dcb971f233819bef2170f3bfc927a6c0f2a0 Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Wed, 27 Aug 2025 18:03:53 -0700 Subject: [PATCH 2/2] [MemoryLifetimeVerifier] Permit leaks in dead-ends --- include/swift/SIL/SILFunction.h | 3 +- lib/SIL/Verifier/MemoryLifetimeVerifier.cpp | 31 ++++++++++------- lib/SIL/Verifier/SILVerifier.cpp | 2 +- test/SIL/memory_lifetime.sil | 8 +++++ .../allocbox_to_stack_ownership.sil | 33 ------------------- 5 files changed, 31 insertions(+), 46 deletions(-) diff --git a/include/swift/SIL/SILFunction.h b/include/swift/SIL/SILFunction.h index 9f9098b707756..dae5ed085588c 100644 --- a/include/swift/SIL/SILFunction.h +++ b/include/swift/SIL/SILFunction.h @@ -1676,7 +1676,8 @@ class SILFunction } /// Verifies the lifetime of memory locations in the function. - void verifyMemoryLifetime(CalleeCache *calleeCache); + void verifyMemoryLifetime(CalleeCache *calleeCache, + DeadEndBlocks *deadEndBlocks); /// Verifies ownership of the function. /// Since we don't have complete lifetimes everywhere, computes DeadEndBlocks diff --git a/lib/SIL/Verifier/MemoryLifetimeVerifier.cpp b/lib/SIL/Verifier/MemoryLifetimeVerifier.cpp index d83fc493224d9..73ef672fc1fe2 100644 --- a/lib/SIL/Verifier/MemoryLifetimeVerifier.cpp +++ b/lib/SIL/Verifier/MemoryLifetimeVerifier.cpp @@ -12,13 +12,14 @@ #define DEBUG_TYPE "sil-memory-lifetime-verifier" #include "swift/Basic/Assertions.h" -#include "swift/SIL/MemoryLocations.h" +#include "swift/SIL/ApplySite.h" +#include "swift/SIL/BasicBlockDatastructures.h" +#include "swift/SIL/BasicBlockUtils.h" #include "swift/SIL/BitDataflow.h" #include "swift/SIL/CalleeCache.h" +#include "swift/SIL/MemoryLocations.h" #include "swift/SIL/SILBasicBlock.h" #include "swift/SIL/SILFunction.h" -#include "swift/SIL/ApplySite.h" -#include "swift/SIL/BasicBlockDatastructures.h" #include "llvm/Support/CommandLine.h" using namespace swift; @@ -43,6 +44,7 @@ class MemoryLifetimeVerifier { SILFunction *function; CalleeCache *calleeCache; + DeadEndBlocks *deadEndBlocks; MemoryLocations locations; /// alloc_stack memory locations which are used for store_borrow. @@ -140,11 +142,12 @@ class MemoryLifetimeVerifier { } public: - MemoryLifetimeVerifier(SILFunction *function, CalleeCache *calleeCache) : - function(function), - calleeCache(calleeCache), - locations(/*handleNonTrivialProjections*/ true, - /*handleTrivialLocations*/ true) {} + MemoryLifetimeVerifier(SILFunction *function, CalleeCache *calleeCache, + DeadEndBlocks *deadEndBlocks) + : function(function), calleeCache(calleeCache), + deadEndBlocks(deadEndBlocks), + locations(/*handleNonTrivialProjections*/ true, + /*handleTrivialLocations*/ true) {} /// The main entry point to verify the lifetime of all memory locations in /// the function. @@ -883,7 +886,12 @@ void MemoryLifetimeVerifier::checkBlock(SILBasicBlock *block, Bits &bits) { } case SILInstructionKind::DeallocStackInst: { SILValue opVal = cast(&I)->getOperand(); - requireBitsClear(bits & nonTrivialLocations, opVal, &I); + if (!deadEndBlocks->isDeadEnd(I.getParent())) { + // TODO: rdar://159311784: Maybe at some point the invariant will be + // enforced that values stored into addresses + // don't leak in dead-ends. + requireBitsClear(bits & nonTrivialLocations, opVal, &I); + } // Needed to clear any bits of trivial locations (which are not required // to be zero). locations.clearBits(bits, opVal); @@ -973,7 +981,8 @@ void MemoryLifetimeVerifier::verify() { } // anonymous namespace -void SILFunction::verifyMemoryLifetime(CalleeCache *calleeCache) { - MemoryLifetimeVerifier verifier(this, calleeCache); +void SILFunction::verifyMemoryLifetime(CalleeCache *calleeCache, + DeadEndBlocks *deadEndBlocks) { + MemoryLifetimeVerifier verifier(this, calleeCache, deadEndBlocks); verifier.verify(); } diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp index 89d51d864bd5f..b5b34e0ad61cb 100644 --- a/lib/SIL/Verifier/SILVerifier.cpp +++ b/lib/SIL/Verifier/SILVerifier.cpp @@ -7380,7 +7380,7 @@ class SILVerifier : public SILVerifierBase { if (F->hasOwnership() && F->shouldVerifyOwnership() && !mod.getASTContext().hadError()) { - F->verifyMemoryLifetime(calleeCache); + F->verifyMemoryLifetime(calleeCache, &getDeadEndBlocks()); } } diff --git a/test/SIL/memory_lifetime.sil b/test/SIL/memory_lifetime.sil index 76c02a3538db9..406d77ad8958e 100644 --- a/test/SIL/memory_lifetime.sil +++ b/test/SIL/memory_lifetime.sil @@ -864,3 +864,11 @@ bb0: %10 = tuple () return %10 } + +sil [ossa] @storage_leaked_into_dead_ends : $@convention(thin) () -> () { + %t = apply undef() : $@convention(thin) () -> (@owned T) + %t_addr = alloc_stack $T + store %t to [init] %t_addr + dealloc_stack %t_addr + unreachable +} diff --git a/test/SILOptimizer/allocbox_to_stack_ownership.sil b/test/SILOptimizer/allocbox_to_stack_ownership.sil index d8ae775d7cb5a..4e3b03e2b897d 100644 --- a/test/SILOptimizer/allocbox_to_stack_ownership.sil +++ b/test/SILOptimizer/allocbox_to_stack_ownership.sil @@ -1205,39 +1205,6 @@ bb0(%0 : $*T, %1 : $*T, %2 : $*T): return %15 : $() } -// CHECK-LABEL: sil [ossa] @alloc_box_in_specialized_callee : -// CHECK-NOT: alloc_box -// CHECK-LABEL: } // end sil function 'alloc_box_in_specialized_callee' -sil [ossa] @alloc_box_in_specialized_callee : $@convention(thin) (Int) -> () { -bb0(%0 : $Int): - %1 = alloc_box ${ var Int } - %2 = project_box %1, 0 - store %0 to [trivial] %2 - %4 = function_ref @callee_with_allocbox : $@convention(thin) (@guaranteed { var Int }) -> () - %5 = apply %4(%1) : $@convention(thin) (@guaranteed { var Int }) -> () - destroy_value %1 - %r = tuple () - return %r -} - -// CHECK-LABEL: sil shared [ossa] @$s20callee_with_allocboxTf0s_n : -// CHECK-NOT: alloc_box -// CHECK-LABEL: } // end sil function '$s20callee_with_allocboxTf0s_n' - -// CHECK-LABEL: sil [ossa] @callee_with_allocbox : -// CHECK-NOT: alloc_box -// CHECK-LABEL: } // end sil function 'callee_with_allocbox' -sil [ossa] @callee_with_allocbox : $@convention(thin) (@guaranteed { var Int }) -> () { -bb0(%0 : @guaranteed ${ var Int }): - %1 = alloc_box ${ var Int } - %2 = project_box %1 : ${ var Int }, 0 - %3 = project_box %0 : ${ var Int }, 0 - copy_addr %3 to %2 - destroy_value %1 - %r = tuple () - return %r -} - sil @getC : $@convention(thin) () -> (@owned C) // CHECK-LABEL: sil [ossa] @leak_to_inf_loop_1 : {{.*}} {