Merge pull request #24835 from eeckstein/fix-silcombine

Author: swift-ci

lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

@@ -91,8 +91,16 @@ SILCombiner::visitAllocExistentialBoxInst(AllocExistentialBoxInst *AEBI) {
   if (SingleStore && SingleRelease) {
     assert(SingleProjection && "store without a projection");
     // Release the value that was stored into the existential box. The box
-    // is going away so we need to release the stored value now.
-    Builder.setInsertionPoint(SingleStore);
+    // is going away so we need to release the stored value.
+    // NOTE: It's important that the release is inserted at the single
+    // release of the box and not at the store, because a balancing retain could
+    // be _after_ the store, e.g:
+    //      %box = alloc_existential_box
+    //      %addr = project_existential_box %box
+    //      store %value to %addr
+    //      retain_value %value    // must insert the release after this retain
+    //      strong_release %box
+    Builder.setInsertionPoint(SingleRelease);
     Builder.createReleaseValue(AEBI->getLoc(), SingleStore->getSrc(),
                                SingleRelease->getAtomicity());
 

test/SILOptimizer/sil_combine.sil

@@ -2806,6 +2806,32 @@ bb3(%19 : $Double):                               // Preds: bb1 bb2
   return %19 : $Double                            // id: %20
 }
 
+enum ErrorEnum: Error {
+    case errorCase(label: [Error])
+    case other
+}
+
+// CHECK-LABEL: sil @insert_compensating_release_at_release_of_box
+// CHECK:      [[L:%[0-9]+]] = load
+// CHECK-NEXT: [[T:%[0-9]+]] = tuple
+// CHECK-NEXT: retain_value [[L]]
+// CHECK-NEXT: release_value [[T]]
+// CHECK-NEXT: tuple
+// CHECK-NEXT: return
+sil @insert_compensating_release_at_release_of_box : $@convention(method) (@in_guaranteed Array<Error>) -> () {
+bb0(%0 : $*Array<Error>):
+  %20 = load %0 : $*Array<Error>
+  %22 = tuple $(label: Array<Error>) (%20)
+  %23 = enum $ErrorEnum, #ErrorEnum.errorCase!enumelt.1, %22 : $(label: Array<Error>)
+  %36 = alloc_existential_box $Error, $ErrorEnum
+  %37 = project_existential_box $ErrorEnum in %36 : $Error
+  store %23 to %37 : $*ErrorEnum
+  retain_value %20 : $Array<Error>
+  strong_release %36 : $Error
+  %52 = tuple ()
+  return %52 : $()
+}
+
 sil [reabstraction_thunk] @_TTRXFo_oSS_dSb_XFo_iSS_iSb_ : $@convention(thin) (@in String, @owned @callee_owned (@owned String) -> Bool) -> @out Bool
 sil [reabstraction_thunk] @_TTRXFo_iSS_iSb_XFo_oSS_dSb_ : $@convention(thin) (@owned String, @owned @callee_owned (@in String) -> @out Bool) -> Bool
 

test/SILOptimizer/silcombine_aebox_miscompile.swift

@@ -0,0 +1,37 @@
+// RUN: %empty-directory(%t) 
+// RUN: %target-build-swift -O -module-name=a %s -o %t/a.out
+// RUN: %target-run %t/a.out | %FileCheck %s
+
+// REQUIRES: executable_test
+
+// This is an end-to-end test for rdar://problem/50759056.
+
+enum ErrorEnum: Error {
+  case errorCase([Error])
+  case other
+}
+
+final class Myclass {
+  var e = [Error]()
+  var b = true
+
+  @inline(never)
+  func foo() {
+    e.append(ErrorEnum.other)
+    if b {
+      bar(ErrorEnum.errorCase(e))
+    }
+  }
+
+  @inline(never)
+  func bar(_: Error?) {
+    b = false
+    foo()
+  }
+}
+
+let c = Myclass()
+c.foo()
+
+// CHECK: [a.ErrorEnum.other, a.ErrorEnum.other]
+print(c.e)