Skip to content

Commit 688af95

Browse files
committed
FunctionSignatureOptimization: fix a miscompile in owned->guaranteed return value specialization
FSO can handle self-recursive calls. But this only works if the result of the self-recursive call is actually returned and not used otherwise. The check for this was missing. https://bugs.swift.org/browse/SR-12677 rdar://problem/62895040
1 parent 15956e4 commit 688af95

File tree

3 files changed

+44
-4
lines changed

3 files changed

+44
-4
lines changed

include/swift/SILOptimizer/Analysis/EpilogueARCAnalysis.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,11 @@ class EpilogueARCContext {
200200
// We are checking for retain. If this is a self-recursion. call
201201
// to the function (which returns an owned value) can be treated as
202202
// the retain instruction.
203-
if (auto *AI = dyn_cast<ApplyInst>(II))
204-
if (AI->getCalleeFunction() == II->getParent()->getParent())
205-
return true;
203+
if (auto *AI = dyn_cast<ApplyInst>(II)) {
204+
return AI->getCalleeFunction() == II->getParent()->getParent() &&
205+
RCFI->getRCIdentityRoot(AI) ==
206+
RCFI->getRCIdentityRoot(getArg(AI->getParent()));
207+
}
206208
// Check whether this is a retain instruction and the argument it
207209
// retains.
208210
return isRetainInstruction(II) &&

test/SILOptimizer/epilogue_arc_dumper.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ bb3:
180180
}
181181

182182
// CHECK-LABEL: START: epilogue_retains_in_diamond_top_and_left_split_block
183-
// CHECK: apply
183+
// CHECK-NOT: apply
184184
// CHECK: FINISH: epilogue_retains_in_diamond_top_and_left_split_block
185185
sil @epilogue_retains_in_diamond_top_and_left_split_block : $@convention(thin) (@owned foo) -> @owned foo {
186186
bb0(%0 : $foo):

test/SILOptimizer/functionsigopts.sil

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ public class KlassBar : KlassFoo {
6363
init()
6464
}
6565

66+
indirect public enum IndirectEnum : Hashable {
67+
case A
68+
case B(IndirectEnum)
69+
}
70+
6671
sil @use_Int : $@convention(thin) (Int) -> ()
6772
sil @use_Int64 : $@convention(thin) (Int64) -> ()
6873
sil @use_Generic : $@convention(thin) <T>(@in_guaranteed T) -> ()
@@ -514,6 +519,39 @@ bb0(%0 : $boo):
514519
return %0 : $boo
515520
}
516521

522+
// CHECK-LABEL: sil @no_fso_recursive_result_is_not_returned :
523+
// CHECK-NOT: release_value
524+
// CHECK: bb2:
525+
// CHECK-NEXT: retain_value %0
526+
// CHECK: return
527+
// CHECK-LABEL: } // end sil function 'no_fso_recursive_result_is_not_returned'
528+
sil @no_fso_recursive_result_is_not_returned : $@convention(method) (@guaranteed IndirectEnum, @guaranteed IndirectEnum) -> @owned IndirectEnum {
529+
bb0(%0 : $IndirectEnum, %1 : $IndirectEnum):
530+
switch_enum %1 : $IndirectEnum, case #IndirectEnum.B!enumelt: bb1, default bb2
531+
532+
bb1(%3 : ${ var IndirectEnum }):
533+
%4 = project_box %3 : ${ var IndirectEnum }, 0
534+
%5 = load %4 : $*IndirectEnum
535+
%6 = function_ref @no_fso_recursive_result_is_not_returned : $@convention(method) (@guaranteed IndirectEnum, @guaranteed IndirectEnum) -> @owned IndirectEnum
536+
537+
// The result of the self-recursion is not returned (but put in a box).
538+
// This prevents guaranteed -> owned return value specialization.
539+
%7 = apply %6(%0, %5) : $@convention(method) (@guaranteed IndirectEnum, @guaranteed IndirectEnum) -> @owned IndirectEnum
540+
%8 = alloc_box ${ var IndirectEnum }
541+
%9 = project_box %8 : ${ var IndirectEnum }, 0
542+
store %7 to %9 : $*IndirectEnum
543+
%11 = enum $IndirectEnum, #IndirectEnum.B!enumelt, %8 : ${ var IndirectEnum }
544+
br bb3(%11 : $IndirectEnum)
545+
546+
bb2:
547+
retain_value %0 : $IndirectEnum
548+
br bb3(%0 : $IndirectEnum)
549+
550+
bb3(%35 : $IndirectEnum):
551+
return %35 : $IndirectEnum
552+
}
553+
554+
517555
// Make sure we do not move the retain_value in the throw block.
518556
//
519557
// CHECK-LABEL: sil [serialized] [signature_optimized_thunk] [always_inline] @owned_to_unowned_retval_with_error_result : $@convention(thin) (@owned boo) -> (@owned boo, @error Error) {

0 commit comments

Comments
 (0)