MoveOnlyAddressChecker: More robust checking for consume-during-borrow.

jckarter · jckarter · commit 27a8852290e5 · 2024-06-25T14:10:02.000-07:00
- While an opaque borrow access occurs to part of a value, the entire scope of
  the access needs to be treated as a liveness range, so add the `EndAccess`es
  to the liveness range.
- The SIL verifier may crash the compiler on SILGen-generated code when the
  developer's source contains consume-during-borrow code patterns. Allow
  `load_borrow` instructions to be marked `[unchecked]`, which suppresses
  verifier checks until the move checker runs and gets a chance to properly
  diagnose these errors.

Fixes rdar://124360175.
diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h
@@ -4591,14 +4591,29 @@ class StoreInst
 /// instruction in its use-def list.
 class LoadBorrowInst :
     public UnaryInstructionBase<SILInstructionKind::LoadBorrowInst,
-                                SingleValueInstruction> {
+                                SingleValueInstruction>
+{
   friend class SILBuilder;
 
+  bool Unchecked = false;
+
 public:
   LoadBorrowInst(SILDebugLocation DebugLoc, SILValue LValue)
       : UnaryInstructionBase(DebugLoc, LValue,
                              LValue->getType().getObjectType()) {}
 
+  // True if the invariants on `load_borrow` have not been checked and
+  // should not be strictly enforced.
+  //
+  // This can only occur during raw SIL before move-only checking occurs.
+  // Developers can write incorrect code using noncopyable types that
+  // consumes or mutates a memory location while that location is borrowed,
+  // but the move-only checker must diagnose those problems before canonical
+  // SIL is formed.
+  bool isUnchecked() const { return Unchecked; }
+  
+  void setUnchecked(bool value) { Unchecked = value; }
+
   using EndBorrowRange =
       decltype(std::declval<ValueBase>().getUsersOfType<EndBorrowInst>());
 
diff --git a/lib/SIL/IR/SILPrinter.cpp b/lib/SIL/IR/SILPrinter.cpp
@@ -1733,6 +1733,9 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
   }
 
   void visitLoadBorrowInst(LoadBorrowInst *LBI) {
+    if (LBI->isUnchecked()) {
+      *this << "[unchecked] ";
+    }
     *this << getIDAndType(LBI->getOperand());
   }
 
diff --git a/lib/SIL/Parser/ParseSIL.cpp b/lib/SIL/Parser/ParseSIL.cpp
@@ -3733,12 +3733,28 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
 
   case SILInstructionKind::LoadBorrowInst: {
     SourceLoc AddrLoc;
+    
+    bool IsUnchecked = false;
+    StringRef AttrName;
+    SourceLoc AttrLoc;
+    if (parseSILOptional(AttrName, AttrLoc, *this)) {
+      if (AttrName == "unchecked") {
+        IsUnchecked = true;
+      } else {
+        P.diagnose(InstLoc.getSourceLoc(),
+                   diag::sil_invalid_attribute_for_instruction, AttrName,
+                   "load_borrow");
+        return true;
+      }
+    }
 
     if (parseTypedValueRef(Val, AddrLoc, B) ||
         parseSILDebugLocation(InstLoc, B))
       return true;
 
-    ResultVal = B.createLoadBorrow(InstLoc, Val);
+    auto LB = B.createLoadBorrow(InstLoc, Val);
+    LB->setUnchecked(IsUnchecked);
+    ResultVal = LB;
     break;
   }
 
diff --git a/lib/SIL/Verifier/MemoryLifetimeVerifier.cpp b/lib/SIL/Verifier/MemoryLifetimeVerifier.cpp
@@ -788,7 +788,9 @@ void MemoryLifetimeVerifier::checkBlock(SILBasicBlock *block, Bits &bits) {
           requireBitsSet(bits, sbi->getDest(), &I);
           locations.clearBits(bits, sbi->getDest());
         } else if (auto *lbi = dyn_cast<LoadBorrowInst>(ebi->getOperand())) {
-          requireBitsSet(bits, lbi->getOperand(), &I);
+          if (!lbi->isUnchecked()) {
+            requireBitsSet(bits, lbi->getOperand(), &I);
+          }
         }
         break;
       }
diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp
@@ -2623,8 +2623,13 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     requireSameType(LBI->getOperand()->getType().getObjectType(),
                     LBI->getType(),
                     "Load operand type and result type mismatch");
-    require(loadBorrowImmutabilityAnalysis.isImmutable(LBI),
-            "Found load borrow that is invalidated by a local write?!");
+    if (LBI->isUnchecked()) {
+      require(LBI->getModule().getStage() == SILStage::Raw,
+              "load_borrow can only be [unchecked] in raw SIL");
+    } else {
+      require(loadBorrowImmutabilityAnalysis.isImmutable(LBI),
+              "Found load borrow that is invalidated by a local write?!");
+    }
   }
 
   void checkBeginBorrowInst(BeginBorrowInst *bbi) {
diff --git a/lib/SILGen/SILGenLValue.cpp b/lib/SILGen/SILGenLValue.cpp
@@ -1231,6 +1231,11 @@ namespace {
         return base;
       }
       auto result = SGF.B.createLoadBorrow(loc, base.getValue());
+      // Mark the load_borrow as unchecked. We can't stop the source code from
+      // trying to mutate or consume the same lvalue during this borrow, so
+      // we don't want verifiers to trip before the move checker gets a chance
+      // to diagnose these situations.
+      result->setUnchecked(true);
       return SGF.emitFormalEvaluationManagedBorrowedRValueWithCleanup(loc,
          base.getValue(), result);
     }
diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
@@ -1258,23 +1258,29 @@ void UseState::initializeLiveness(
                             << *livenessInstAndValue.first;
                liveness.print(llvm::dbgs()));
   }
+  
+  auto updateForLivenessAccess = [&](BeginAccessInst *beginAccess,
+                                     const SmallBitVector &livenessMask) {
+    for (auto *endAccess : beginAccess->getEndAccesses()) {
+      liveness.updateForUse(endAccess, livenessMask, false /*lifetime ending*/);
+    }
+  };
 
   for (auto livenessInstAndValue : nonconsumingUses) {
     if (auto *lbi = dyn_cast<LoadBorrowInst>(livenessInstAndValue.first)) {
       auto accessPathWithBase =
           AccessPathWithBase::computeInScope(lbi->getOperand());
       if (auto *beginAccess =
               dyn_cast_or_null<BeginAccessInst>(accessPathWithBase.base)) {
-        for (auto *endAccess : beginAccess->getEndAccesses()) {
-          liveness.updateForUse(endAccess, livenessInstAndValue.second,
-                                false /*lifetime ending*/);
-        }
+        updateForLivenessAccess(beginAccess, livenessInstAndValue.second);
       } else {
         for (auto *ebi : lbi->getEndBorrows()) {
           liveness.updateForUse(ebi, livenessInstAndValue.second,
                                 false /*lifetime ending*/);
         }
       }
+    } else if (auto *bai = dyn_cast<BeginAccessInst>(livenessInstAndValue.first)) {
+      updateForLivenessAccess(bai, livenessInstAndValue.second);
     } else {
       liveness.updateForUse(livenessInstAndValue.first,
                             livenessInstAndValue.second,
diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp
@@ -214,6 +214,22 @@ void MoveOnlyChecker::checkAddresses() {
 
 namespace {
 
+static bool canonicalizeLoadBorrows(SILFunction *F) {
+  bool changed = false;
+  for (auto &block : *F) {
+    for (auto &inst : block) {
+      if (auto *lbi = dyn_cast<LoadBorrowInst>(&inst)) {
+        if (lbi->isUnchecked()) {
+          changed = true;
+          lbi->setUnchecked(false);
+        }
+      }
+    }
+  }
+  
+  return changed;
+}
+
 class MoveOnlyCheckerPass : public SILFunctionTransform {
   void run() override {
     auto *fn = getFunction();
@@ -228,8 +244,11 @@ class MoveOnlyCheckerPass : public SILFunctionTransform {
     // If an earlier pass told use to not emit diagnostics for this function,
     // clean up any copies, invalidate the analysis, and return early.
     if (fn->hasSemanticsAttr(semantics::NO_MOVEONLY_DIAGNOSTICS)) {
-      if (cleanupNonCopyableCopiesAfterEmittingDiagnostic(getFunction()))
+      bool didChange = canonicalizeLoadBorrows(fn);
+      didChange |= cleanupNonCopyableCopiesAfterEmittingDiagnostic(getFunction());
+      if (didChange) {
         invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
+      }
       return;
     }
 
@@ -252,6 +271,11 @@ class MoveOnlyCheckerPass : public SILFunctionTransform {
                                                    checker.diagnosticEmitter);
     }
 
+    // Remaining borrows
+    // should be correctly immutable. We can canonicalize any remaining
+    // `load_borrow [unchecked]` instructions.
+    checker.madeChange |= canonicalizeLoadBorrows(fn);
+
     checker.madeChange |=
         cleanupNonCopyableCopiesAfterEmittingDiagnostic(fn);
 
diff --git a/lib/Serialization/DeserializeSIL.cpp b/lib/Serialization/DeserializeSIL.cpp
@@ -2260,7 +2260,6 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn,
   UNARY_INSTRUCTION(EndLifetime)
   UNARY_INSTRUCTION(ExtendLifetime)
   UNARY_INSTRUCTION(CopyBlock)
-  UNARY_INSTRUCTION(LoadBorrow)
   UNARY_INSTRUCTION(EndInitLetRef)
   REFCOUNTING_INSTRUCTION(StrongRetain)
   REFCOUNTING_INSTRUCTION(StrongRelease)
@@ -2271,6 +2270,17 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn,
 #undef UNARY_INSTRUCTION
 #undef REFCOUNTING_INSTRUCTION
 
+  case SILInstructionKind::LoadBorrowInst: {
+    assert(RecordKind == SIL_ONE_OPERAND && "Layout should be OneOperand.");
+    auto LB = Builder.createLoadBorrow(
+        Loc, getLocalValue(Builder.maybeGetFunction(), ValID,
+                           getSILType(MF->getType(TyID),
+                                      (SILValueCategory)TyCategory, Fn)));
+    LB->setUnchecked(Attr != 0);
+    ResultInst = LB;
+    break;
+  }
+
   case SILInstructionKind::BeginBorrowInst: {
     assert(RecordKind == SIL_ONE_OPERAND && "Layout should be OneOperand.");
     auto isLexical = IsLexical_t(Attr & 0x1);
diff --git a/lib/Serialization/SerializeSIL.cpp b/lib/Serialization/SerializeSIL.cpp
@@ -1601,6 +1601,8 @@ void SILSerializer::writeSILInstruction(const SILInstruction &SI) {
     } else if (auto *I = dyn_cast<CopyableToMoveOnlyWrapperValueInst>(&SI)) {
       Attr = I->getForwardingOwnershipKind() == OwnershipKind::Owned ? true
                                                                      : false;
+    } else if (auto *LB = dyn_cast<LoadBorrowInst>(&SI)) {
+      Attr = LB->isUnchecked();
     }
     writeOneOperandLayout(SI.getKind(), Attr, SI.getOperand(0));
     break;
diff --git a/test/SILGen/moveonly.swift b/test/SILGen/moveonly.swift
@@ -974,7 +974,7 @@ public struct LoadableSubscriptGetOnlyTester : ~Copyable {
 // The get call
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
 // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[ACCESS]]
-// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [[MARK]]
+// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [unchecked] [[MARK]]
 // CHECK: [[TEMP:%.*]] = alloc_stack $AddressOnlyProtocol
 // CHECK: [[TEMP_MARK:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[TEMP]]
 // CHECK: apply {{%.*}}([[TEMP_MARK]], {{%.*}}, [[LOAD_BORROW]])
@@ -989,7 +989,7 @@ public struct LoadableSubscriptGetOnlyTester : ~Copyable {
 // CHECK: [[M2_PROJECT:%.*]] = project_box [[M2_BORROW]]
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
 // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[ACCESS]]
-// CHECK: [[LOAD:%.*]] = load_borrow [[MARK]]
+// CHECK: [[LOAD:%.*]] = load_borrow [unchecked] [[MARK]]
 // CHECK: apply {{%.*}}([[M2_PROJECT]], {{%.*}}, [[LOAD]])
 // CHECK: end_borrow [[LOAD]]
 // CHECK: end_access [[ACCESS]]
@@ -1030,7 +1030,7 @@ public func testSubscriptGetOnly_BaseLoadable_ResultAddressOnly_Let() {
 // CHECK:   [[MARK:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[ARG]]
 //
 // CHECK:   [[ACCESS:%.*]] = begin_access [read] [unknown] [[MARK]]
-// CHECK:   [[LOAD_BORROW:%.*]] = load_borrow [[ACCESS]]
+// CHECK:   [[LOAD_BORROW:%.*]] = load_borrow [unchecked] [[ACCESS]]
 // CHECK:   [[TEMP:%.*]] = alloc_stack $AddressOnlyProtocol
 // CHECK:   [[TEMP_MARK:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[TEMP]]
 // CHECK:   apply {{%.*}}([[TEMP_MARK]], {{%.*}}, [[LOAD_BORROW]])
@@ -1049,7 +1049,7 @@ public func testSubscriptGetOnly_BaseLoadable_ResultAddressOnly_InOut(m: inout L
 // The get call
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [dynamic] [[GLOBAL_ADDR]]
 // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[ACCESS]]
-// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [[MARK]]
+// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [unchecked] [[MARK]]
 // CHECK: [[TEMP:%.*]] = alloc_stack $AddressOnlyProtocol
 // CHECK: [[TEMP_MARK:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[TEMP]]
 // CHECK: apply {{%.*}}([[TEMP_MARK]], {{%.*}}, [[LOAD_BORROW]])
@@ -1078,7 +1078,7 @@ public struct LoadableSubscriptGetOnlyTesterNonCopyableStructParent : ~Copyable
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
 // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[ACCESS]]
 // CHECK: [[GEP:%.*]] = struct_element_addr [[MARK]]
-// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [[GEP]]
+// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [unchecked] [[GEP]]
 // CHECK: [[TEMP:%.*]] = alloc_stack $AddressOnlyProtocol
 // CHECK: [[TEMP_MARK:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[TEMP]]
 // CHECK: apply {{%.*}}([[TEMP_MARK]], {{%.*}}, [[LOAD_BORROW]])
@@ -1090,7 +1090,7 @@ public struct LoadableSubscriptGetOnlyTesterNonCopyableStructParent : ~Copyable
 // The second get call.
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
 // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[ACCESS]]
-// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [[MARK]]
+// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [unchecked] [[MARK]]
 // CHECK: [[VALUE:%.*]] = apply {{%.*}}([[LOAD_BORROW]])
 //
 // CHECK: [[BORROWED_VALUE:%.*]] = begin_borrow [[VALUE]]
@@ -1137,7 +1137,7 @@ public func testSubscriptGetOnlyThroughNonCopyableParentStruct_BaseLoadable_Resu
 //
 // CHECK:   [[ACCESS:%.*]] = begin_access [read] [unknown] [[MARK]]
 // CHECK:   [[GEP:%.*]] = struct_element_addr [[ACCESS]]
-// CHECK:   [[LOAD:%.*]] = load_borrow [[GEP]]
+// CHECK:   [[LOAD:%.*]] = load_borrow [unchecked] [[GEP]]
 // CHECK:   [[TEMP:%.*]] = alloc_stack $AddressOnlyProtocol
 // CHECK:   [[TEMP_MARK:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[TEMP]]
 // CHECK:   apply {{%.*}}([[TEMP_MARK]], {{%.*}}, [[LOAD]])
@@ -1155,7 +1155,7 @@ public func testSubscriptGetOnlyThroughNonCopyableParentStruct_BaseLoadable_Resu
 // CHECK:   [[ACCESS:%.*]] = begin_access [read] [dynamic] [[GLOBAL]]
 // CHECK:   [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[ACCESS]]
 // CHECK:   [[GEP:%.*]] = struct_element_addr [[MARK]]
-// CHECK:   [[LOAD:%.*]] = load_borrow [[GEP]]
+// CHECK:   [[LOAD:%.*]] = load_borrow [unchecked] [[GEP]]
 // CHECK:   [[TEMP:%.*]] = alloc_stack $AddressOnlyProtocol
 // CHECK:   [[TEMP_MARK:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[TEMP]]
 // CHECK:   apply {{%.*}}([[TEMP_MARK]], {{%.*}}, [[LOAD]])
@@ -1192,7 +1192,7 @@ public class LoadableSubscriptGetOnlyTesterClassParent {
 // CHECK: [[TEMP:%.*]] = alloc_stack $LoadableSubscriptGetOnlyTester
 // CHECK: [[TEMP_MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[TEMP]]
 // CHECK: [[TEMP_MARK_BORROW:%.*]] = store_borrow [[CORO_RESULT]] to [[TEMP_MARK]]
-// CHECK: [[LOAD:%.*]] = load_borrow [[TEMP_MARK_BORROW]]
+// CHECK: [[LOAD:%.*]] = load_borrow [unchecked] [[TEMP_MARK_BORROW]]
 // CHECK: [[TEMP2:%.*]] = alloc_stack $
 // CHECK: [[TEMP2_MARK:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[TEMP2]]
 // CHECK: apply {{%.*}}([[TEMP2_MARK]], {{%.*}}, [[LOAD]])
@@ -1214,7 +1214,7 @@ public class LoadableSubscriptGetOnlyTesterClassParent {
 // CHECK: [[TEMP_MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[TEMP]]
 // CHECK: [[TEMP_MARK_BORROW:%.*]] = store_borrow [[CORO_RESULT]] to [[TEMP_MARK]]
 // CHECK: [[GEP:%.*]] = struct_element_addr [[TEMP_MARK_BORROW]]
-// CHECK: [[LOAD:%.*]] = load_borrow [[GEP]]
+// CHECK: [[LOAD:%.*]] = load_borrow [unchecked] [[GEP]]
 // CHECK: [[TEMP2:%.*]] = alloc_stack $
 // CHECK: [[TEMP2_MARK:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[TEMP2]]
 // CHECK: apply {{%.*}}([[TEMP2_MARK]], {{%.*}}, [[LOAD]])
@@ -1226,7 +1226,7 @@ public class LoadableSubscriptGetOnlyTesterClassParent {
 // Third read.
 //
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
-// CHECK: [[LOAD:%.*]] = load_borrow [[ACCESS]]
+// CHECK: [[LOAD:%.*]] = load_borrow [unchecked] [[ACCESS]]
 // CHECK: ([[CORO_RESULT_ORIG:%.*]], [[CORO_TOKEN:%.*]]) = begin_apply {{%.*}}([[LOAD]])
 // CHECK: [[CORO_RESULT_CP:%.*]] = copy_value [[CORO_RESULT_ORIG]]
 // CHECK: [[CORO_RESULT_MK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[CORO_RESULT_CP]]
@@ -1455,7 +1455,7 @@ public struct LoadableSubscriptGetSetTesterNonCopyableStructParent : ~Copyable {
 // The second get call.
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
 // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[ACCESS]]
-// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [[MARK]]
+// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [unchecked] [[MARK]]
 // CHECK: [[VALUE:%.*]] = apply {{%.*}}([[LOAD_BORROW]])
 // CHECK: [[BORROWED_VALUE:%.*]] = begin_borrow [[VALUE]]
 // CHECK: [[TEMP:%.*]] = alloc_stack $AddressOnlyProtocol
@@ -1652,7 +1652,7 @@ public class LoadableSubscriptGetSetTesterClassParent {
 // Third read.
 //
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
-// CHECK: [[CLASS:%.*]] = load_borrow [[ACCESS]]
+// CHECK: [[CLASS:%.*]] = load_borrow [unchecked] [[ACCESS]]
 // CHECK: ([[CORO_RESULT_ORIG:%.*]], [[CORO_TOKEN:%.*]]) = begin_apply {{%.*}}([[CLASS]])
 // CHECK: [[CORO_RESULT_CP:%.*]] = copy_value [[CORO_RESULT_ORIG]]
 // CHECK: [[CORO_RESULT_MK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[CORO_RESULT_CP]]
@@ -1911,7 +1911,7 @@ public struct LoadableSubscriptReadModifyTesterNonCopyableStructParent : ~Copyab
 // The second get call.
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
 // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[ACCESS]]
-// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [[MARK]]
+// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [unchecked] [[MARK]]
 // CHECK: [[VALUE:%.*]] = apply {{%.*}}([[LOAD_BORROW]])
 // CHECK: [[BORROWED_VALUE:%.*]] = begin_borrow [[VALUE]]
 // CHECK: ([[CORO_RESULT_ORIG:%.*]], [[CORO_TOKEN:%.*]]) = begin_apply {{%.*}}({{%.*}}, [[BORROWED_VALUE]])
@@ -2092,7 +2092,7 @@ public class LoadableSubscriptReadModifyTesterClassParent {
 // Third read.
 //
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
-// CHECK: [[CLASS:%.*]] = load_borrow [[ACCESS]]
+// CHECK: [[CLASS:%.*]] = load_borrow [unchecked] [[ACCESS]]
 // CHECK: ([[CORO_RESULT_ORIG:%.*]], [[CORO_TOKEN:%.*]]) = begin_apply {{%.*}}([[CLASS]])
 // CHECK: [[CORO_RESULT_CP:%.*]] = copy_value [[CORO_RESULT_ORIG]]
 // CHECK: [[CORO_RESULT_MK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[CORO_RESULT_CP]]
@@ -2348,7 +2348,7 @@ public struct LoadableSubscriptGetModifyTesterNonCopyableStructParent : ~Copyabl
 // The second get call.
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
 // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[ACCESS]]
-// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [[MARK]]
+// CHECK: [[LOAD_BORROW:%.*]] = load_borrow [unchecked] [[MARK]]
 // CHECK: [[VALUE:%.*]] = apply {{%.*}}([[LOAD_BORROW]])
 // CHECK: [[BORROWED_VALUE:%.*]] = begin_borrow [[VALUE]]
 // CHECK: [[TEMP:%.*]] = alloc_stack $AddressOnlyProtocol
@@ -2502,7 +2502,7 @@ public class LoadableSubscriptGetModifyTesterClassParent {
 // Third read.
 //
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
-// CHECK: [[CLASS:%.*]] = load_borrow [[ACCESS]]
+// CHECK: [[CLASS:%.*]] = load_borrow [unchecked] [[ACCESS]]
 // CHECK: ([[CORO_RESULT_ORIG:%.*]], [[CORO_TOKEN:%.*]]) = begin_apply {{%.*}}([[CLASS]])
 // CHECK: [[CORO_RESULT_CP:%.*]] = copy_value [[CORO_RESULT_ORIG]]
 // CHECK: [[CORO_RESULT_MK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[CORO_RESULT_CP]]
diff --git a/test/SILOptimizer/moveonly_consume_during_borrow_1.swift b/test/SILOptimizer/moveonly_consume_during_borrow_1.swift
@@ -0,0 +1,15 @@
+// RUN: %target-swift-frontend -emit-sil -verify %s
+
+func foo(x: consuming Foo) { // expected-error{{'x' used after consume}}
+    x.bar.foo(x) // expected-error{{overlapping accesses to 'x'}} expected-note 3 {{}}
+}
+
+struct Foo: ~Copyable {
+    var bar: Bar {
+        _read { fatalError() }
+    }
+}
+
+struct Bar: ~Copyable {
+    func foo(_: consuming Foo) {}
+}
diff --git a/test/SILOptimizer/moveonly_consume_during_borrow_2.swift b/test/SILOptimizer/moveonly_consume_during_borrow_2.swift

Original file line number	Diff line number	Diff line change
`@@ -1733,6 +1733,9 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {`
`1733`	`1733`	`}`
`1734`	`1734`
`1735`	`1735`	`void visitLoadBorrowInst(LoadBorrowInst *LBI) {`
	`1736`	`+ if (LBI->isUnchecked()) {`
	`1737`	`+ *this << "[unchecked] ";`
	`1738`	`+ }`
`1736`	`1739`	`*this << getIDAndType(LBI->getOperand());`
`1737`	`1740`	`}`
`1738`	`1741`
Original file line number	Diff line number	Diff line change
`@@ -788,7 +788,9 @@ void MemoryLifetimeVerifier::checkBlock(SILBasicBlock *block, Bits &bits) {`
`788`	`788`	`requireBitsSet(bits, sbi->getDest(), &I);`
`789`	`789`	`locations.clearBits(bits, sbi->getDest());`
`790`	`790`	`} else if (auto *lbi = dyn_cast<LoadBorrowInst>(ebi->getOperand())) {`
`791`		`- requireBitsSet(bits, lbi->getOperand(), &I);`
	`791`	`+ if (!lbi->isUnchecked()) {`
	`792`	`+ requireBitsSet(bits, lbi->getOperand(), &I);`
	`793`	`+ }`
`792`	`794`	`}`
`793`	`795`	`break;`
`794`	`796`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1231,6 +1231,11 @@ namespace {`
`1231`	`1231`	`return base;`
`1232`	`1232`	`}`
`1233`	`1233`	`auto result = SGF.B.createLoadBorrow(loc, base.getValue());`
	`1234`	`+ // Mark the load_borrow as unchecked. We can't stop the source code from`
	`1235`	`+ // trying to mutate or consume the same lvalue during this borrow, so`
	`1236`	`+ // we don't want verifiers to trip before the move checker gets a chance`
	`1237`	`+ // to diagnose these situations.`
	`1238`	`+ result->setUnchecked(true);`
`1234`	`1239`	`return SGF.emitFormalEvaluationManagedBorrowedRValueWithCleanup(loc,`
`1235`	`1240`	`base.getValue(), result);`
`1236`	`1241`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1601,6 +1601,8 @@ void SILSerializer::writeSILInstruction(const SILInstruction &SI) {`
`1601`	`1601`	`} else if (auto *I = dyn_cast<CopyableToMoveOnlyWrapperValueInst>(&SI)) {`
`1602`	`1602`	`Attr = I->getForwardingOwnershipKind() == OwnershipKind::Owned ? true`
`1603`	`1603`	`: false;`
	`1604`	`+ } else if (auto *LB = dyn_cast<LoadBorrowInst>(&SI)) {`
	`1605`	`+ Attr = LB->isUnchecked();`
`1604`	`1606`	`}`
`1605`	`1607`	`writeOneOperandLayout(SI.getKind(), Attr, SI.getOperand(0));`
`1606`	`1608`	`break;`