Skip to content

Commit ebac2e4

Browse files
committed
[AllocBoxToStack] PAI frontier looks thru copies.
When determining where to destroy a captured alloc_box, the frontier of the capturing partial_apply is computed. Previously, that computation just used the uses of the partial_apply. If the partial_apply were copied and the original destroyed before the apply, however, the result would be a miscompile where the destroy_addr/dealloc_stack for the alloc_stack to which the alloc_box was promoted was destroyed before the apply of the closure which used the address.
1 parent 49d0226 commit ebac2e4

File tree

2 files changed

+52
-1
lines changed

2 files changed

+52
-1
lines changed

lib/SILOptimizer/Transforms/AllocBoxToStack.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "swift/SIL/BasicBlockDatastructures.h"
2121
#include "swift/SIL/Dominance.h"
2222
#include "swift/SIL/MemAccessUtils.h"
23+
#include "swift/SIL/NodeDatastructures.h"
2324
#include "swift/SIL/SILArgument.h"
2425
#include "swift/SIL/SILBuilder.h"
2526
#include "swift/SIL/SILCloner.h"
@@ -1101,7 +1102,21 @@ specializeApplySite(SILOptFunctionBuilder &FuncBuilder, ApplySite Apply,
11011102
// borrows its captures, and we don't need to adjust capture lifetimes.
11021103
if (!PAI->isOnStack()) {
11031104
if (PAFrontier.empty()) {
1104-
ValueLifetimeAnalysis VLA(PAI, PAI->getUses());
1105+
SmallVector<SILInstruction *, 8> users;
1106+
InstructionWorklist worklist(PAI->getFunction());
1107+
worklist.push(PAI);
1108+
while (auto *inst = worklist.pop()) {
1109+
auto *svi = cast<SingleValueInstruction>(inst);
1110+
for (auto *use : svi->getUses()) {
1111+
auto *user = use->getUser();
1112+
SingleValueInstruction *svi;
1113+
if ((svi = dyn_cast<CopyValueInst>(user))) {
1114+
worklist.push(svi);
1115+
}
1116+
users.push_back(user);
1117+
}
1118+
}
1119+
ValueLifetimeAnalysis VLA(PAI, users);
11051120
pass.CFGChanged |= !VLA.computeFrontier(
11061121
PAFrontier, ValueLifetimeAnalysis::AllowToModifyCFG);
11071122
assert(!PAFrontier.empty() &&

test/SILOptimizer/allocboxtostack_localapply_ossa.sil

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,3 +488,39 @@ bb0(%0 : @guaranteed ${ var Int }, %1 : @guaranteed ${ var Int }):
488488
%8 = apply %7(%0, %1) : $@convention(thin) (@guaranteed { var Int }, @guaranteed { var Int }) -> Int
489489
return %8 : $Int
490490
}
491+
492+
class C {}
493+
494+
sil @getC : $@convention(thin) () -> (@owned C)
495+
496+
sil [ossa] @borrow_c_box : $@convention(thin) (@guaranteed { var C }) -> () {
497+
entry(%box : @guaranteed ${var C}):
498+
%retval = tuple ()
499+
return %retval : $()
500+
}
501+
502+
// CHECK-LABEL: sil [ossa] @test_copy_applied : {{.*}} {
503+
// CHECK: [[CLOSURE:%[^,]+]] = partial_apply
504+
// CHECK: [[COPY:%[^,]+]] = copy_value [[CLOSURE]]
505+
// CHECK: destroy_value [[CLOSURE]]
506+
// CHECK: apply [[COPY]]()
507+
// CHECK: destroy_addr
508+
// CHECK: dealloc_stack
509+
// CHECK-LABEL: } // end sil function 'test_copy_applied'
510+
sil [ossa] @test_copy_applied : $@convention(thin) () -> () {
511+
bb0:
512+
%box = alloc_box ${ var C }, var, name "x"
513+
%addr = project_box %box : ${ var C }, 0
514+
%getC = function_ref @getC : $@convention(thin) () -> (@owned C)
515+
%c = apply %getC() : $@convention(thin) () -> (@owned C)
516+
store %c to [init] %addr : $*C
517+
518+
%borrow_int_box = function_ref @borrow_c_box : $@convention(thin) (@guaranteed { var C }) -> ()
519+
%closure = partial_apply [callee_guaranteed] %borrow_int_box(%box) : $@convention(thin) (@guaranteed { var C }) -> ()
520+
%copy = copy_value %closure : $@callee_guaranteed () -> ()
521+
destroy_value %closure : $@callee_guaranteed () -> ()
522+
apply %copy() : $@callee_guaranteed () -> ()
523+
destroy_value %copy : $@callee_guaranteed () -> ()
524+
%retval = tuple ()
525+
return %retval : $()
526+
}

0 commit comments

Comments
 (0)