Workaround OSSA dead borrow scope bugs

lib/SIL/Utils/OwnershipUtils.cpp

@@ -178,10 +178,17 @@ bool swift::findInnerTransitiveGuaranteedUses(
       break;
     }
     case OperandOwnership::Borrow:
-      BorrowingOperand(use).visitExtendedScopeEndingUses([&](Operand *endUse) {
-        leafUse(endUse);
-        return true;
-      });
+      // FIXME: visitExtendedScopeEndingUses can't return false here once dead
+      // borrows are disallowed.
+      if (!BorrowingOperand(use).visitExtendedScopeEndingUses(
+            [&](Operand *endUse) {
+              leafUse(endUse);
+              return true;
+            })) {
+        // Special case for dead borrows. This is dangerous because clients
+        // don't expect a begin_borrow to be in the use list.
+        leafUse(use);
+      }
       break;
     }
   }
@@ -275,10 +282,17 @@ bool swift::findInnerTransitiveGuaranteedUsesOfBorrowedValue(
       break;
     }
     case OperandOwnership::Borrow:
-      BorrowingOperand(use).visitExtendedScopeEndingUses([&](Operand *endUse) {
-        recordUse(endUse);
-        return true;
-      });
+      // FIXME: visitExtendedScopeEndingUses can't return false here once dead
+      // borrows are disallowed.
+      if (!BorrowingOperand(use).visitExtendedScopeEndingUses(
+            [&](Operand *endUse) {
+              recordUse(endUse);
+              return true;
+            })) {
+        // Special case for dead borrows. This is dangerous because clients
+        // don't expect a begin_borrow to be in the use list.
+        recordUse(use);
+      }
       break;
     }
   }
@@ -427,21 +441,29 @@ bool BorrowingOperand::visitScopeEndingUses(
   switch (kind) {
   case BorrowingOperandKind::Invalid:
     llvm_unreachable("Using invalid case");
-  case BorrowingOperandKind::BeginBorrow:
+  case BorrowingOperandKind::BeginBorrow: {
+    bool deadBorrow = true;
     for (auto *use : cast<BeginBorrowInst>(op->getUser())->getUses()) {
       if (use->isLifetimeEnding()) {
+        deadBorrow = false;
         if (!func(use))
           return false;
       }
     }
-    return true;
+    // FIXME: special case for dead borrows. This is dangerous because clients
+    // only expect visitScopeEndingUses to return false if the visitor returned
+    // false.
+    return !deadBorrow;
+  }
   case BorrowingOperandKind::BeginApply: {
+    bool deadApply = true;
     auto *user = cast<BeginApplyInst>(op->getUser());
     for (auto *use : user->getTokenResult()->getUses()) {
+      deadApply = false;
       if (!func(use))
         return false;
     }
-    return true;
+    return !deadApply;
   }
   // These are instantaneous borrow scopes so there aren't any special end
   // scope instructions.
@@ -450,12 +472,16 @@ bool BorrowingOperand::visitScopeEndingUses(
   case BorrowingOperandKind::Yield:
     return true;
   case BorrowingOperandKind::Branch: {
+    bool deadBranch = true;
     auto *br = cast<BranchInst>(op->getUser());
-    for (auto *use : br->getArgForOperand(op)->getUses())
-      if (use->isLifetimeEnding())
+    for (auto *use : br->getArgForOperand(op)->getUses()) {
+      if (use->isLifetimeEnding()) {
+        deadBranch = false;
         if (!func(use))
           return false;
-    return true;
+      }
+    }
+    return !deadBranch;
   }
   }
   llvm_unreachable("Covered switch isn't covered");
@@ -522,10 +548,15 @@ BorrowedValue BorrowingOperand::getBorrowIntroducingUserResult() {
 void BorrowingOperand::getImplicitUses(
     SmallVectorImpl<Operand *> &foundUses,
     std::function<void(Operand *)> *errorFunction) const {
-  visitScopeEndingUses([&](Operand *op) {
-    foundUses.push_back(op);
+  // FIXME: this visitScopeEndingUses should never return false once dead
+  // borrows are disallowed.
+  if (!visitScopeEndingUses([&](Operand *endOp) {
+    foundUses.push_back(endOp);
     return true;
-  });
+  })) {
+    // Special-case for dead borrows.
+    foundUses.push_back(op);
+  }
 }
 
 //===----------------------------------------------------------------------===//

lib/SILOptimizer/Utils/CanonicalizeBorrowScope.cpp

@@ -327,13 +327,18 @@ class FindBorrowScopeUses {
       // For borrows, record the scope-ending instructions to outer use
       // points. Note: The logic in filterOuterBorrowUseInsts that checks
       // whether a borrow scope is an outer use must visit the same set of uses.
-      borrowingOper.visitExtendedScopeEndingUses([&](Operand *endBorrow) {
+      //
+      // FIXME: visitExtendedScopeEndingUses can't return false here once dead
+      // borrows are disallowed.
+      if (!borrowingOper.visitExtendedScopeEndingUses([&](Operand *endBorrow) {
         auto *endInst = endBorrow->getUser();
         if (!isUserInLiveOutBlock(endInst)) {
           useInsts.insert(endInst);
         }
         return true;
-      });
+      })) {
+        useInsts.insert(user);
+      }
     }
     return true;
   }

test/SILOptimizer/copy_propagation.sil

@@ -817,3 +817,22 @@ bb4:
   return %v : $()
 }
 
+// Test a dead begin_borrow (with no scope ending uses). Make sure
+// copy-propagation doesn't end the lifetime before the dead borrow.
+//
+// CHECK-LABEL: sil hidden [ossa] @testDeadBorrow : $@convention(thin) (@owned C) -> () {
+// CHECK: bb0(%0 : @owned $C):
+// CHECK:   copy_value %0 : $C
+// CHECK:   destroy_value
+// CHECK:   copy_value %0 : $C
+// CHECK:   begin_borrow
+// CHECK:   unreachable
+// CHECK-LABEL: } // end sil function 'testDeadBorrow'
+sil hidden [ossa] @testDeadBorrow : $@convention(thin) (@owned C) -> () {
+bb0(%0 : @owned $C):
+  %1 = copy_value %0 : $C
+  destroy_value %1 : $C
+  %6 = copy_value %0 : $C
+  %7 = begin_borrow %6 : $C
+  unreachable
+}

test/SILOptimizer/semantic-arc-opts.sil

@@ -9,6 +9,7 @@ import Builtin
 //////////////////
 
 typealias AnyObject = Builtin.AnyObject
+protocol Error {}
 
 enum MyNever {}
 enum FakeOptional<T> {
@@ -1525,3 +1526,107 @@ bb3(%7 : @owned $FakeOptional<Builtin.NativeObject>):
   %9999 = tuple()
   return %9999 : $()
 }
+
+// SemanticARCOptVisitor::performGuaranteedCopyValueOptimization should not optimize this case.
+//
+// CHECK-LABEL: sil [ossa] @testDeadInterleavedBorrow : $@convention(c) (@owned FakeOptional<AnyObject>) -> () {
+// CHECK: [[B:%.*]] = begin_borrow [lexical] %0 : $FakeOptional<AnyObject>
+// CHECK: copy_value
+// CHECK: switch_enum
+// CHECK: bb{{.*}}([[COPY:%.*]] : @owned $AnyObject):
+// CHECK: begin_borrow [lexical] [[COPY]] : $AnyObject
+// CHECK: end_borrow [[B]] : $FakeOptional<AnyObject>
+// CHECK: end_borrow
+// CHECK: destroy_value [[COPY]] : $AnyObject
+// CHECK-LABEL: } // end sil function 'testDeadInterleavedBorrow'
+sil [ossa] @testDeadInterleavedBorrow : $@convention(c) (@owned FakeOptional<AnyObject>) -> () {
+bb0(%opt : @owned $FakeOptional<AnyObject>):
+    %opt_borrow = begin_borrow [lexical] %opt : $FakeOptional<AnyObject>
+    %opt_borrow_copy = copy_value %opt_borrow : $FakeOptional<AnyObject>
+    switch_enum %opt_borrow_copy : $FakeOptional<AnyObject>, case #FakeOptional.some!enumelt: some, case #FakeOptional.none!enumelt: none
+
+some(%obj : @owned $AnyObject):
+    %_borrow = begin_borrow [lexical] %obj : $AnyObject
+    end_borrow %opt_borrow : $FakeOptional<AnyObject>
+    end_borrow %_borrow : $AnyObject
+    destroy_value %obj : $AnyObject
+    br die
+
+die:
+    unreachable
+
+none:
+    end_borrow %opt_borrow : $FakeOptional<AnyObject>
+    br exit
+
+exit:
+    destroy_value %opt : $FakeOptional<AnyObject>
+    %res = tuple ()
+    return %res : $()
+}
+
+// SemanticARCOptVisitor::performGuaranteedCopyValueOptimization should not optimize this case.
+//
+// CHECK-LABEL: sil [ossa] @testDeadAndAliveInterleavedBorrow : $@convention(c) (@owned FakeOptional<AnyObject>) -> () {
+// CHECK: [[B:%.*]] = begin_borrow [lexical] %0 : $FakeOptional<AnyObject>
+// CHECK: copy_value
+// CHECK: switch_enum
+//
+// CHECK: bb{{.*}}([[COPY:%.*]] : @owned $AnyObject):
+// CHECK: [[BB:%.*]] = begin_borrow [lexical] [[COPY]] : $AnyObject
+//
+// CHECK: try_apply undef
+//
+// CHECK: [[RESULT:%.*]] = apply undef(%{{.*}}) : $@convention(method) (@guaranteed AnyObject) -> @owned FakeOptional<AnyObject>
+// CHECK: switch_enum [[RESULT]] : $FakeOptional<AnyObject>, case #FakeOptional.some!enumelt: {{.*}}, case #FakeOptional.none!enumelt: [[NONEBB:bb[0-9]?]]
+//
+// CHECK: [[NONEBB]]:
+// CHECK: end_borrow [[B]] : $FakeOptional<AnyObject>
+// CHECK: end_borrow [[BB]] : $AnyObject
+// CHECK: destroy_value [[COPY]] : $AnyObject
+// CHECK: destroy_value %0 : $FakeOptional<AnyObject>
+// CHECK-LABEL: } // end sil function 'testDeadAndAliveInterleavedBorrow'
+sil [ossa] @testDeadAndAliveInterleavedBorrow : $@convention(c) (@owned FakeOptional<AnyObject>) -> () {
+bb0(%opt : @owned $FakeOptional<AnyObject>):
+    %opt_borrow = begin_borrow [lexical] %opt : $FakeOptional<AnyObject>
+    %opt_borrow_copy = copy_value %opt_borrow : $FakeOptional<AnyObject>
+    switch_enum %opt_borrow_copy : $FakeOptional<AnyObject>, case #FakeOptional.some!enumelt: some, case #FakeOptional.none!enumelt: none
+
+some(%obj : @owned $AnyObject):
+    %obj_borrow = begin_borrow [lexical] %obj : $AnyObject
+    %obj_borrow_copy = copy_value %obj_borrow : $AnyObject
+    try_apply undef(%obj_borrow_copy) : $@convention(method) (@owned AnyObject) -> (@owned AnyObject, @error Error), normal good, error bad
+
+good(%obj2 : @owned $AnyObject):
+    %obj3 = apply undef(%obj2) : $@convention(method) (@guaranteed AnyObject) -> @owned FakeOptional<AnyObject>
+    destroy_value %obj2 : $AnyObject
+    switch_enum %obj3 : $FakeOptional<AnyObject>, case #FakeOptional.some!enumelt: good_some, case #FakeOptional.none!enumelt: good_none
+
+good_some(%obj4 : @owned $AnyObject):
+    destroy_value %obj4 : $AnyObject
+    end_borrow %obj_borrow : $AnyObject
+    destroy_value %obj : $AnyObject
+    br exit
+
+good_none:
+    end_borrow %opt_borrow : $FakeOptional<AnyObject>
+    end_borrow %obj_borrow : $AnyObject
+    destroy_value %obj : $AnyObject
+    destroy_value %opt : $FakeOptional<AnyObject>
+    br die
+
+bad(%error1 : @owned $Error):
+    br die
+
+die:
+    unreachable
+
+none:
+    br exit
+
+exit:
+    end_borrow %opt_borrow : $FakeOptional<AnyObject>
+    destroy_value %opt : $FakeOptional<AnyObject>
+    %res = tuple ()
+    return %res : $()
+}