From f7c3c6f437c82b94435d9ff53b8ba23c71e4a19a Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Tue, 9 Dec 2025 14:31:54 -0800 Subject: [PATCH] Fix InteriorUseWalker: consider partial_apply [on_stack] an escape Fixes a bug in MandatoryDestroyHoisting where a captured value is destroyed before a copy of the closure. On-stack closures can be copied, and all copied uses must be within the borrow scope of the captured operand. This is just like any other non-Escapable value, so treat it as such by checking `Value.mayEscape` rather than `Type.Escapable`. Originally, I wanted to make it illegal to copy of partial_apply [on_stack], but it looks like we still allow it. I would rather not complicate any logic yet with special handling for this case. To fix any performance concerns, we might be able to simplify the representation instead by banning copy_value on on-stack closures. Fixes rdar://165850554 swift-frontend crash: While running "CopyPropagation" - Invalid SIL provided to OSSACompleteLifetime?!) --- .../Utilities/OwnershipLiveness.swift | 8 +++-- .../mandatory-destroy-hoisting.sil | 31 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/Utilities/OwnershipLiveness.swift b/SwiftCompilerSources/Sources/Optimizer/Utilities/OwnershipLiveness.swift index f8166be37ff..2ee481e04cc 100644 --- a/SwiftCompilerSources/Sources/Optimizer/Utilities/OwnershipLiveness.swift +++ b/SwiftCompilerSources/Sources/Optimizer/Utilities/OwnershipLiveness.swift @@ -626,9 +626,13 @@ extension InteriorUseWalker: OwnershipUseVisitor { if value.type.isTrivial(in: function) { return .continueWalk } - guard value.type.isEscapable(in: function) else { + guard value.mayEscape else { // Non-escapable dependent values can be lifetime-extended by copying, which is not handled by - // InteriorUseWalker. LifetimeDependenceDefUseWalker does this. + // InteriorUseWalker. LifetimeDependenceDefUseWalker does this. Alternatively, we could continue to `walkDownUses` + // but later recognize a copy of a `mayEscape` value to be a pointer escape at that point. + // + // This includes partial_apply [on_stack] which can currently be copied. Although a better solution would be to + // make copying an on-stack closure illegal SIL. return pointerEscapingUse(of: operand) } if useVisitor(operand) == .abortWalk { diff --git a/test/SILOptimizer/mandatory-destroy-hoisting.sil b/test/SILOptimizer/mandatory-destroy-hoisting.sil index 9cf275856de..96609b93aec 100644 --- a/test/SILOptimizer/mandatory-destroy-hoisting.sil +++ b/test/SILOptimizer/mandatory-destroy-hoisting.sil @@ -319,8 +319,11 @@ bb0(%0 : @owned $String): return %r } +// TODO: If InteriorLiveness handles `partial_apply [on_stack]`, then the destroy can be hoisted. +// // CHECK-LABEL: sil [ossa] @partial_apply : // CHECK: destroy_value %1 +// CHECK-NEXT: debug_step // CHECK-NEXT: destroy_value %0 // CHECK: } // end sil function 'partial_apply' sil [ossa] @partial_apply : $@convention(thin) (@owned String) -> () { @@ -445,3 +448,31 @@ bb0: %r = tuple () return %r } + +sil @closure1 : $@convention(thin) (@guaranteed C) -> () + +// rdar165850554: InteriorUseDefWalker must consider copies of an on-stack partial apply when computing liveness of the +// borrowed operand. + +// CHECK-LABEL: sil private [ossa] @test_closure_copy : +// CHECK: %1 = copy_value %0 +// CHECK: debug_step +// CHECK-NEXT: destroy_value %1 +// CHECK-LABEL: } // end sil function 'test_closure_copy' +sil private [ossa] @test_closure_copy : $@convention(thin) (@owned C) -> () { +bb0(%0 : @owned $C): + // create a non-lexical value to trigger mandatory destroy hoisting + %1 = copy_value %0 + %2 = function_ref @closure1 : $@convention(thin) (@guaranteed C) -> () + %3 = partial_apply [callee_guaranteed] [on_stack] %2(%1) : $@convention(thin) (@guaranteed C) -> () + %4 = copy_value %3 + // force a pointer-escape + %5 = unchecked_bitwise_cast %4 to $@noescape @callee_guaranteed () -> () + destroy_value %3 + destroy_value %0 + destroy_value %4 + debug_step + destroy_value %1 + %10 = tuple () + return %10 +}