[ownership] Add a higher level construct called BorrowScopeIntroducingOperand

include/swift/SIL/OwnershipUtils.h

@@ -200,7 +200,86 @@ bool isOwnershipForwardingInst(SILInstruction *i);
 
 bool isGuaranteedForwardingInst(SILInstruction *i);
 
-struct BorrowScopeIntroducerKind {
+struct BorrowScopeOperandKind {
+  using UnderlyingKindTy = std::underlying_type<SILInstructionKind>::type;
+
+  enum Kind : UnderlyingKindTy {
+    BeginBorrow = UnderlyingKindTy(SILInstructionKind::BeginBorrowInst),
+    BeginApply = UnderlyingKindTy(SILInstructionKind::BeginApplyInst),
+  };
+
+  Kind value;
+
+  BorrowScopeOperandKind(Kind newValue) : value(newValue) {}
+  BorrowScopeOperandKind(const BorrowScopeOperandKind &other)
+      : value(other.value) {}
+  operator Kind() const { return value; }
+
+  static Optional<BorrowScopeOperandKind> get(SILInstructionKind kind) {
+    switch (kind) {
+    default:
+      return None;
+    case SILInstructionKind::BeginBorrowInst:
+      return BorrowScopeOperandKind(BeginBorrow);
+    case SILInstructionKind::BeginApplyInst:
+      return BorrowScopeOperandKind(BeginApply);
+    }
+  }
+
+  void print(llvm::raw_ostream &os) const;
+  LLVM_ATTRIBUTE_DEPRECATED(void dump() const, "only for use in the debugger");
+};
+
+/// An operand whose user instruction introduces a new borrow scope for the
+/// operand's value. The value of the operand must be considered as implicitly
+/// borrowed until the user's corresponding end scope instruction.
+struct BorrowScopeOperand {
+  BorrowScopeOperandKind kind;
+  Operand *op;
+
+  BorrowScopeOperand(Operand *op)
+      : kind(*BorrowScopeOperandKind::get(op->getUser()->getKind())), op(op) {}
+
+  /// If value is a borrow introducer return it after doing some checks.
+  static Optional<BorrowScopeOperand> get(Operand *op) {
+    auto *user = op->getUser();
+    auto kind = BorrowScopeOperandKind::get(user->getKind());
+    if (!kind)
+      return None;
+    return BorrowScopeOperand(*kind, op);
+  }
+
+  void visitEndScopeInstructions(function_ref<void(Operand *)> func) const {
+    switch (kind) {
+    case BorrowScopeOperandKind::BeginBorrow:
+      for (auto *use : cast<BeginBorrowInst>(op->getUser())->getUses()) {
+        if (isa<EndBorrowInst>(use->getUser())) {
+          func(use);
+        }
+      }
+      return;
+    case BorrowScopeOperandKind::BeginApply: {
+      auto *user = cast<BeginApplyInst>(op->getUser());
+      for (auto *use : user->getTokenResult()->getUses()) {
+        func(use);
+      }
+      return;
+    }
+    }
+    llvm_unreachable("Covered switch isn't covered");
+  }
+
+private:
+  /// Internal constructor for failable static constructor. Please do not expand
+  /// its usage since it assumes the code passed in is well formed.
+  BorrowScopeOperand(BorrowScopeOperandKind kind, Operand *op)
+      : kind(kind), op(op) {}
+};
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
+                              BorrowScopeOperandKind kind);
+
+struct BorrowScopeIntroducingValueKind {
   using UnderlyingKindTy = std::underlying_type<ValueKind>::type;
 
   /// Enum we use for exhaustive pattern matching over borrow scope introducers.
@@ -210,23 +289,23 @@ struct BorrowScopeIntroducerKind {
     SILFunctionArgument = UnderlyingKindTy(ValueKind::SILFunctionArgument),
   };
 
-  static Optional<BorrowScopeIntroducerKind> get(ValueKind kind) {
+  static Optional<BorrowScopeIntroducingValueKind> get(ValueKind kind) {
     switch (kind) {
     default:
       return None;
     case ValueKind::LoadBorrowInst:
-      return BorrowScopeIntroducerKind(LoadBorrow);
+      return BorrowScopeIntroducingValueKind(LoadBorrow);
     case ValueKind::BeginBorrowInst:
-      return BorrowScopeIntroducerKind(BeginBorrow);
+      return BorrowScopeIntroducingValueKind(BeginBorrow);
     case ValueKind::SILFunctionArgument:
-      return BorrowScopeIntroducerKind(SILFunctionArgument);
+      return BorrowScopeIntroducingValueKind(SILFunctionArgument);
     }
   }
 
   Kind value;
 
-  BorrowScopeIntroducerKind(Kind newValue) : value(newValue) {}
-  BorrowScopeIntroducerKind(const BorrowScopeIntroducerKind &other)
+  BorrowScopeIntroducingValueKind(Kind newValue) : value(newValue) {}
+  BorrowScopeIntroducingValueKind(const BorrowScopeIntroducingValueKind &other)
       : value(other.value) {}
   operator Kind() const { return value; }
 
@@ -238,10 +317,10 @@ struct BorrowScopeIntroducerKind {
   /// of the scope.
   bool isLocalScope() const {
     switch (value) {
-    case BorrowScopeIntroducerKind::BeginBorrow:
-    case BorrowScopeIntroducerKind::LoadBorrow:
+    case BorrowScopeIntroducingValueKind::BeginBorrow:
+    case BorrowScopeIntroducingValueKind::LoadBorrow:
       return true;
-    case BorrowScopeIntroducerKind::SILFunctionArgument:
+    case BorrowScopeIntroducingValueKind::SILFunctionArgument:
       return false;
     }
     llvm_unreachable("Covered switch isnt covered?!");
@@ -252,7 +331,7 @@ struct BorrowScopeIntroducerKind {
 };
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
-                              BorrowScopeIntroducerKind kind);
+                              BorrowScopeIntroducingValueKind kind);
 
 /// A higher level construct for working with values that represent the
 /// introduction of a new borrow scope.
@@ -271,26 +350,26 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
 /// borrow introducers can not have guaranteed results that are not creating a
 /// new borrow scope. No such instructions exist today.
 struct BorrowScopeIntroducingValue {
-  BorrowScopeIntroducerKind kind;
+  BorrowScopeIntroducingValueKind kind;
   SILValue value;
 
   BorrowScopeIntroducingValue(LoadBorrowInst *lbi)
-      : kind(BorrowScopeIntroducerKind::LoadBorrow), value(lbi) {}
+      : kind(BorrowScopeIntroducingValueKind::LoadBorrow), value(lbi) {}
   BorrowScopeIntroducingValue(BeginBorrowInst *bbi)
-      : kind(BorrowScopeIntroducerKind::BeginBorrow), value(bbi) {}
+      : kind(BorrowScopeIntroducingValueKind::BeginBorrow), value(bbi) {}
   BorrowScopeIntroducingValue(SILFunctionArgument *arg)
-      : kind(BorrowScopeIntroducerKind::SILFunctionArgument), value(arg) {
+      : kind(BorrowScopeIntroducingValueKind::SILFunctionArgument), value(arg) {
     assert(arg->getOwnershipKind() == ValueOwnershipKind::Guaranteed);
   }
 
   BorrowScopeIntroducingValue(SILValue v)
-      : kind(*BorrowScopeIntroducerKind::get(v->getKind())), value(v) {
+      : kind(*BorrowScopeIntroducingValueKind::get(v->getKind())), value(v) {
     assert(v.getOwnershipKind() == ValueOwnershipKind::Guaranteed);
   }
 
   /// If value is a borrow introducer return it after doing some checks.
   static Optional<BorrowScopeIntroducingValue> get(SILValue value) {
-    auto kind = BorrowScopeIntroducerKind::get(value->getKind());
+    auto kind = BorrowScopeIntroducingValueKind::get(value->getKind());
     if (!kind || value.getOwnershipKind() != ValueOwnershipKind::Guaranteed)
       return None;
     return BorrowScopeIntroducingValue(*kind, value);
@@ -334,7 +413,8 @@ struct BorrowScopeIntroducingValue {
 private:
   /// Internal constructor for failable static constructor. Please do not expand
   /// its usage since it assumes the code passed in is well formed.
-  BorrowScopeIntroducingValue(BorrowScopeIntroducerKind kind, SILValue value)
+  BorrowScopeIntroducingValue(BorrowScopeIntroducingValueKind kind,
+                              SILValue value)
       : kind(kind), value(value) {}
 };
 

lib/SIL/OwnershipUtils.cpp

@@ -81,22 +81,22 @@ bool swift::isOwnershipForwardingInst(SILInstruction *i) {
 //                             Borrow Introducers
 //===----------------------------------------------------------------------===//
 
-void BorrowScopeIntroducerKind::print(llvm::raw_ostream &os) const {
+void BorrowScopeIntroducingValueKind::print(llvm::raw_ostream &os) const {
   switch (value) {
-  case BorrowScopeIntroducerKind::SILFunctionArgument:
+  case BorrowScopeIntroducingValueKind::SILFunctionArgument:
     os << "SILFunctionArgument";
     return;
-  case BorrowScopeIntroducerKind::BeginBorrow:
+  case BorrowScopeIntroducingValueKind::BeginBorrow:
     os << "BeginBorrowInst";
     return;
-  case BorrowScopeIntroducerKind::LoadBorrow:
+  case BorrowScopeIntroducingValueKind::LoadBorrow:
     os << "LoadBorrowInst";
     return;
   }
   llvm_unreachable("Covered switch isn't covered?!");
 }
 
-void BorrowScopeIntroducerKind::dump() const {
+void BorrowScopeIntroducingValueKind::dump() const {
 #ifndef NDEBUG
   print(llvm::dbgs());
 #endif
@@ -107,13 +107,13 @@ void BorrowScopeIntroducingValue::getLocalScopeEndingInstructions(
   assert(isLocalScope() && "Should only call this given a local scope");
 
   switch (kind) {
-  case BorrowScopeIntroducerKind::SILFunctionArgument:
+  case BorrowScopeIntroducingValueKind::SILFunctionArgument:
     llvm_unreachable("Should only call this with a local scope");
-  case BorrowScopeIntroducerKind::BeginBorrow:
+  case BorrowScopeIntroducingValueKind::BeginBorrow:
     llvm::copy(cast<BeginBorrowInst>(value)->getEndBorrows(),
                std::back_inserter(scopeEndingInsts));
     return;
-  case BorrowScopeIntroducerKind::LoadBorrow:
+  case BorrowScopeIntroducingValueKind::LoadBorrow:
     llvm::copy(cast<LoadBorrowInst>(value)->getEndBorrows(),
                std::back_inserter(scopeEndingInsts));
     return;
@@ -125,17 +125,17 @@ void BorrowScopeIntroducingValue::visitLocalScopeEndingUses(
     function_ref<void(Operand *)> visitor) const {
   assert(isLocalScope() && "Should only call this given a local scope");
   switch (kind) {
-  case BorrowScopeIntroducerKind::SILFunctionArgument:
+  case BorrowScopeIntroducingValueKind::SILFunctionArgument:
     llvm_unreachable("Should only call this with a local scope");
-  case BorrowScopeIntroducerKind::BeginBorrow:
-    for (auto *use : cast<BeginBorrowInst>(value)->getUses()) {
+  case BorrowScopeIntroducingValueKind::BeginBorrow:
+    for (auto *use : value->getUses()) {
       if (isa<EndBorrowInst>(use->getUser())) {
         visitor(use);
       }
     }
     return;
-  case BorrowScopeIntroducerKind::LoadBorrow:
-    for (auto *use : cast<LoadBorrowInst>(value)->getUses()) {
+  case BorrowScopeIntroducingValueKind::LoadBorrow:
+    for (auto *use : value->getUses()) {
       if (isa<EndBorrowInst>(use->getUser())) {
         visitor(use);
       }
@@ -182,7 +182,7 @@ bool swift::getUnderlyingBorrowIntroducingValues(
 }
 
 llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &os,
-                                     BorrowScopeIntroducerKind kind) {
+                                     BorrowScopeIntroducingValueKind kind) {
   kind.print(os);
   return os;
 }

lib/SIL/SILOwnershipVerifier.cpp

@@ -278,14 +278,10 @@ bool SILValueOwnershipChecker::gatherUsers(
         // For correctness reasons we use indices to make sure that we can
         // append to NonLifetimeEndingUsers without needing to deal with
         // iterator invalidation.
-        for (unsigned i : indices(nonLifetimeEndingUsers)) {
-          if (auto *bbi = dyn_cast<BeginBorrowInst>(
-                  nonLifetimeEndingUsers[i]->getUser())) {
-            for (auto *use : bbi->getUses()) {
-              if (isa<EndBorrowInst>(use->getUser())) {
-                implicitRegularUsers.push_back(use);
-              }
-            }
+        for (auto *op : nonLifetimeEndingUsers) {
+          if (auto scopedOperand = BorrowScopeOperand::get(op)) {
+            scopedOperand->visitEndScopeInstructions(
+                [&](Operand *op) { implicitRegularUsers.push_back(op); });
           }
         }
       }

test/SIL/ownership-verifier/borrow_scope_introducing_operands.sil

@@ -0,0 +1,102 @@
+// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -enable-sil-verify-all=0 -o /dev/null 2>&1  %s | %FileCheck %s
+// REQUIRES: asserts
+
+// This file tests that when we emit an error, it is a true negative. This is
+// done by parsing the emitted output from the ownership verifier.
+
+import Builtin
+
+
+sil [ossa] @coroutine_callee : $@yield_once (@guaranteed Builtin.NativeObject) -> () {
+bb0(%0 : @guaranteed $Builtin.NativeObject):
+  yield (), resume bb1, unwind bb2
+
+bb1:
+  %r = tuple ()
+  return %r : $()
+
+bb2:
+  unwind
+}
+
+// CHECK-LABEL: Function: 'destroy_value_before_end_borrow'
+// CHECK: Found use after free?!
+// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
+// CHECK: Consuming User:   destroy_value %0 : $Builtin.NativeObject
+// CHECK: Non Consuming User:   end_borrow %1 : $Builtin.NativeObject
+// CHECK: Block: bb0
+sil [ossa] @destroy_value_before_end_borrow : $@convention(thin) (@owned Builtin.NativeObject) -> () {
+bb0(%0 : @owned $Builtin.NativeObject):
+  %1 = begin_borrow %0 : $Builtin.NativeObject
+  destroy_value %0 : $Builtin.NativeObject
+  end_borrow %1 : $Builtin.NativeObject
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine'
+// CHECK: Found use after free?!
+// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
+// CHECK: Consuming User:   destroy_value %0 : $Builtin.NativeObject
+// CHECK: Non Consuming User:   end_apply %2
+// CHECK: Block: bb0
+sil [ossa] @destroy_value_before_end_borrow_coroutine : $@convention(thin) (@owned Builtin.NativeObject) -> () {
+bb0(%0 : @owned $Builtin.NativeObject):
+  %coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
+  %token = begin_apply %coro(%0) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
+  destroy_value %0 : $Builtin.NativeObject
+  end_apply %token
+  %r = tuple ()
+  return %r : $()
+}
+
+// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine_2'
+// CHECK: Found use after free?!
+// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
+// CHECK: Consuming User:   destroy_value %0 : $Builtin.NativeObject
+// CHECK: Non Consuming User:   abort_apply %2
+// CHECK: Block: bb0
+sil [ossa] @destroy_value_before_end_borrow_coroutine_2 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
+bb0(%0 : @owned $Builtin.NativeObject):
+  %coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
+  %token = begin_apply %coro(%0) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
+  destroy_value %0 : $Builtin.NativeObject
+  abort_apply %token
+  %r = tuple ()
+  return %r : $()
+}
+
+// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine_3'
+// CHECK: Found use after free?!
+// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
+// CHECK: Consuming User:   destroy_value %0 : $Builtin.NativeObject
+// CHECK: Non Consuming User:   abort_apply %2
+// CHECK: Block: bb1
+
+// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine_3'
+// CHECK: Found use after free due to unvisited non lifetime ending uses?!
+// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
+// CHECK:     Remaining Users:
+// CHECK: User:  abort_apply %2
+// CHECK: Block: bb1
+
+sil [ossa] @destroy_value_before_end_borrow_coroutine_3 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
+bb0(%0 : @owned $Builtin.NativeObject):
+  %coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
+  %token = begin_apply %coro(%0) : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> ()
+  cond_br undef, bb1, bb2
+
+bb1:
+  destroy_value %0 : $Builtin.NativeObject
+  abort_apply %token
+  br bb3
+
+bb2:
+  end_apply %token
+  destroy_value %0 : $Builtin.NativeObject
+  br bb3
+
+bb3:
+  %r = tuple ()
+  return %r : $()
+}