[ownership] Add a new ReborrowVerifier

This updates how we model reborrow's lifetimes for ownership verification.
Today we follow and combine a borrow's lifetime through phi args as well.
Owned values lifetimes end at a phi arg. This discrepency in modeling
lifetimes leads to the OwnershipVerifier raising errors incorrectly for
cases such as this, where the borrow and the base value do not dominate
the end_borrow:

bb0:
  cond_br undef, bb1, bb2
bb1:
  %copy0 = copy_value %0
  %borrow0 = begin_borrow %copy0
  br bb3(%borrow0, %copy0)
bb2:
  %copy1 = copy_value %1
  %borrow1 = begin_borrow %copy1
  br bb3(%borrow1, %copy1)
bb3(%borrow, %baseVal):
  end_borrow %borrow
  destroy_value %baseVal

This PR adds a new ReborrowVerifier. The ownership verifier collects borrow's
lifetime ending users and populates the worklist of the ReborrowVerifier
with reborrows and the corresponding base value.
ReborrowVerifier then verifies that the lifetime of the reborrow is
within the lifetime of the base value.

Author: meg-gupta

include/swift/SIL/LinearLifetimeChecker.h

@@ -50,6 +50,7 @@ class LinearLifetimeChecker {
   class ErrorBuilder;
 
 private:
+  friend class ReborrowVerifier;
   friend class SILOwnershipVerifier;
   friend class SILValueOwnershipChecker;
 

include/swift/SIL/OwnershipUtils.h

@@ -66,6 +66,9 @@ bool isOwnedForwardingInstruction(SILInstruction *inst);
 /// previous terminator.
 bool isOwnedForwardingValue(SILValue value);
 
+/// Returns true if the instruction is a 'reborrow'.
+bool isReborrowInstruction(const SILInstruction *inst);
+
 class BorrowingOperandKind {
 public:
   enum Kind : uint8_t {
@@ -134,7 +137,7 @@ struct BorrowingOperand {
     return BorrowingOperand(*kind, op);
   }
 
-  void visitEndScopeInstructions(function_ref<void(Operand *)> func) const;
+  void visitLocalEndScopeInstructions(function_ref<void(Operand *)> func) const;
 
   /// Returns true if this borrow scope operand consumes guaranteed
   /// values and produces a new scope afterwards.
@@ -204,7 +207,7 @@ struct BorrowingOperand {
   /// \p errorFunction a callback that if non-null is passed an operand that
   /// triggers a mal-formed SIL error. This is just needed for the ownership
   /// verifier to emit good output.
-  bool getImplicitUses(
+  void getImplicitUses(
       SmallVectorImpl<Operand *> &foundUses,
       std::function<void(Operand *)> *errorFunction = nullptr) const;
 

include/swift/SIL/SILValue.h

@@ -40,7 +40,8 @@ class SILInstruction;
 class SILLocation;
 class DeadEndBlocks;
 class ValueBaseUseIterator;
-class ValueUseIterator;
+class ConsumingUseIterator;
+class NonConsumingUseIterator;
 class SILValue;
 
 /// An enumeration which contains values for all the concrete ValueBase
@@ -263,10 +264,20 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
 
   using use_iterator = ValueBaseUseIterator;
   using use_range = iterator_range<use_iterator>;
+  using consuming_use_iterator = ConsumingUseIterator;
+  using consuming_use_range = iterator_range<consuming_use_iterator>;
+  using non_consuming_use_iterator = NonConsumingUseIterator;
+  using non_consuming_use_range = iterator_range<non_consuming_use_iterator>;
 
   inline use_iterator use_begin() const;
   inline use_iterator use_end() const;
 
+  inline consuming_use_iterator consuming_use_begin() const;
+  inline consuming_use_iterator consuming_use_end() const;
+
+  inline non_consuming_use_iterator non_consuming_use_begin() const;
+  inline non_consuming_use_iterator non_consuming_use_end() const;
+
   /// Returns a range of all uses, which is useful for iterating over all uses.
   /// To ignore debug-info instructions use swift::getNonDebugUses instead
   /// (see comment in DebugUtils.h).
@@ -285,6 +296,12 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
   /// and it has a single consuming user. Returns .none otherwise.
   inline Operand *getSingleConsumingUse() const;
 
+  /// Returns a range of all consuming uses
+  inline consuming_use_range getConsumingUses() const;
+
+  /// Returns a range of all non consuming uses
+  inline non_consuming_use_range getNonConsumingUses() const;
+
   template <class T>
   inline T *getSingleUserOfType() const;
 
@@ -711,8 +728,10 @@ class Operand {
     TheValue->FirstUse = this;
   }
 
+  friend class ValueBase;
   friend class ValueBaseUseIterator;
-  friend class ValueUseIterator;
+  friend class ConsumingUseIterator;
+  friend class NonConsumingUseIterator;
   template <unsigned N> friend class FixedOperandList;
   friend class TrailingOperandsList;
 };
@@ -729,6 +748,7 @@ using OperandValueArrayRef = ArrayRefView<Operand, SILValue, getSILValueType>;
 /// An iterator over all uses of a ValueBase.
 class ValueBaseUseIterator : public std::iterator<std::forward_iterator_tag,
                                                   Operand*, ptrdiff_t> {
+protected:
   Operand *Cur;
 public:
   ValueBaseUseIterator() = default;
@@ -770,6 +790,74 @@ inline ValueBase::use_iterator ValueBase::use_end() const {
 inline iterator_range<ValueBase::use_iterator> ValueBase::getUses() const {
   return { use_begin(), use_end() };
 }
+
+class ConsumingUseIterator : public ValueBaseUseIterator {
+public:
+  explicit ConsumingUseIterator(Operand *cur) : ValueBaseUseIterator(cur) {}
+  ConsumingUseIterator &operator++() {
+    assert(Cur && "incrementing past end()!");
+    assert(Cur->isConsumingUse());
+    while ((Cur = Cur->NextUse)) {
+      if (Cur->isConsumingUse())
+        break;
+    }
+    return *this;
+  }
+
+  ConsumingUseIterator operator++(int unused) {
+    ConsumingUseIterator copy = *this;
+    ++*this;
+    return copy;
+  }
+};
+
+inline ValueBase::consuming_use_iterator
+ValueBase::consuming_use_begin() const {
+  auto cur = FirstUse;
+  while (cur && !cur->isConsumingUse()) {
+    cur = cur->NextUse;
+  }
+  return ValueBase::consuming_use_iterator(cur);
+}
+
+inline ValueBase::consuming_use_iterator ValueBase::consuming_use_end() const {
+  return ValueBase::consuming_use_iterator(nullptr);
+}
+
+class NonConsumingUseIterator : public ValueBaseUseIterator {
+public:
+  explicit NonConsumingUseIterator(Operand *cur) : ValueBaseUseIterator(cur) {}
+  NonConsumingUseIterator &operator++() {
+    assert(Cur && "incrementing past end()!");
+    assert(!Cur->isConsumingUse());
+    while ((Cur = Cur->NextUse)) {
+      if (!Cur->isConsumingUse())
+        break;
+    }
+    return *this;
+  }
+
+  NonConsumingUseIterator operator++(int unused) {
+    NonConsumingUseIterator copy = *this;
+    ++*this;
+    return copy;
+  }
+};
+
+inline ValueBase::non_consuming_use_iterator
+ValueBase::non_consuming_use_begin() const {
+  auto cur = FirstUse;
+  while (cur && cur->isConsumingUse()) {
+    cur = cur->NextUse;
+  }
+  return ValueBase::non_consuming_use_iterator(cur);
+}
+
+inline ValueBase::non_consuming_use_iterator
+ValueBase::non_consuming_use_end() const {
+  return ValueBase::non_consuming_use_iterator(nullptr);
+}
+
 inline bool ValueBase::hasOneUse() const {
   auto I = use_begin(), E = use_end();
   if (I == E) return false;
@@ -806,6 +894,15 @@ inline Operand *ValueBase::getSingleConsumingUse() const {
   return result;
 }
 
+inline ValueBase::consuming_use_range ValueBase::getConsumingUses() const {
+  return {consuming_use_begin(), consuming_use_end()};
+}
+
+inline ValueBase::non_consuming_use_range
+ValueBase::getNonConsumingUses() const {
+  return {non_consuming_use_begin(), non_consuming_use_end()};
+}
+
 inline bool ValueBase::hasTwoUses() const {
   auto iter = use_begin(), end = use_end();
   for (unsigned i = 0; i < 2; ++i) {

lib/SIL/Utils/OwnershipUtils.cpp

@@ -125,6 +125,15 @@ bool swift::isOwnershipForwardingInst(SILInstruction *i) {
   return isOwnershipForwardingValueKind(SILNodeKind(i->getKind()));
 }
 
+bool swift::isReborrowInstruction(const SILInstruction *i) {
+  switch (i->getKind()) {
+  case SILInstructionKind::BranchInst:
+    return true;
+  default:
+    return false;
+  }
+}
+
 //===----------------------------------------------------------------------===//
 //                           Borrowing Operand
 //===----------------------------------------------------------------------===//
@@ -163,7 +172,7 @@ llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &os,
   return os;
 }
 
-void BorrowingOperand::visitEndScopeInstructions(
+void BorrowingOperand::visitLocalEndScopeInstructions(
     function_ref<void(Operand *)> func) const {
   switch (kind) {
   case BorrowingOperandKind::BeginBorrow:
@@ -181,18 +190,8 @@ void BorrowingOperand::visitEndScopeInstructions(
     return;
   }
   case BorrowingOperandKind::Branch:
-    for (auto *succBlock :
-         cast<BranchInst>(op->getUser())->getSuccessorBlocks()) {
-      auto *arg = succBlock->getArgument(op->getOperandNumber());
-      for (auto *use : arg->getUses()) {
-        if (use->isConsumingUse()) {
-          func(use);
-        }
-      }
-    }
     return;
   }
-  llvm_unreachable("Covered switch isn't covered");
 }
 
 void BorrowingOperand::visitBorrowIntroducingUserResults(
@@ -267,46 +266,10 @@ void BorrowingOperand::visitUserResultConsumingUses(
   }
 }
 
-bool BorrowingOperand::getImplicitUses(
+void BorrowingOperand::getImplicitUses(
     SmallVectorImpl<Operand *> &foundUses,
     std::function<void(Operand *)> *errorFunction) const {
-  if (!areAnyUserResultsBorrowIntroducers()) {
-    visitEndScopeInstructions([&](Operand *op) { foundUses.push_back(op); });
-    return false;
-  }
-
-  // Ok, we have an instruction that introduces a new borrow scope and its
-  // result is that borrow scope. In such a case, we need to not just add the
-  // end scope instructions of this scoped operand, but also look through any
-  // guaranteed phis and add their end_borrow to this list as well.
-  SmallVector<BorrowingOperand, 8> worklist;
-  SmallPtrSet<Operand *, 8> visitedValue;
-  worklist.push_back(*this);
-  visitedValue.insert(op);
-  bool foundError = false;
-  while (!worklist.empty()) {
-    auto scopedOperand = worklist.pop_back_val();
-    scopedOperand.visitConsumingUsesOfBorrowIntroducingUserResults(
-        [&](Operand *op) {
-          if (auto subSub = BorrowingOperand::get(op)) {
-            if (!visitedValue.insert(op).second) {
-              if (errorFunction) {
-                (*errorFunction)(op);
-              }
-              foundError = true;
-              return;
-            }
-
-            worklist.push_back(*subSub);
-            visitedValue.insert(subSub->op);
-            return;
-          }
-
-          foundUses.push_back(op);
-        });
-  }
-
-  return foundError;
+  visitLocalEndScopeInstructions([&](Operand *op) { foundUses.push_back(op); });
 }
 
 //===----------------------------------------------------------------------===//

lib/SIL/Verifier/CMakeLists.txt

@@ -2,5 +2,6 @@ target_sources(swiftSIL PRIVATE
   LoadBorrowImmutabilityChecker.cpp
   LinearLifetimeChecker.cpp
   MemoryLifetime.cpp
+  ReborrowVerifier.cpp
   SILOwnershipVerifier.cpp
   SILVerifier.cpp)

lib/SIL/Verifier/LinearLifetimeChecker.cpp

@@ -305,10 +305,14 @@ void State::checkForSameBlockUseAfterFree(Operand *consumingUse,
   // must be instructions in the given block. Make sure that the non consuming
   // user is strictly before the consuming user.
   for (auto *nonConsumingUse : nonConsumingUsesInBlock) {
-    if (std::find_if(consumingUse->getUser()->getIterator(), userBlock->end(),
-                     [&nonConsumingUse](const SILInstruction &i) -> bool {
-                       return nonConsumingUse->getUser() == &i;
-                     }) == userBlock->end()) {
+    if (nonConsumingUse->getUser() != consumingUse->getUser()) {
+      if (std::find_if(consumingUse->getUser()->getIterator(), userBlock->end(),
+                       [&nonConsumingUse](const SILInstruction &i) -> bool {
+                         return nonConsumingUse->getUser() == &i;
+                       }) == userBlock->end()) {
+        continue;
+      }
+    } else if (isReborrowInstruction(consumingUse->getUser())) {
       continue;
     }