Merge pull request #59360 from eeckstein/fix-store-borrow-verification

Fix store borrow verification

Author: eeckstein

docs/SIL.rst

@@ -3745,6 +3745,8 @@ store_borrow
 
 Stores the value ``%0`` to a stack location ``%1``, which must be an
 ``alloc_stack $T``.
+The stack location must not be modified by other instructions than
+``store_borrow``.
 The stored value is alive until the ``dealloc_stack`` or until another
 ``store_borrow`` overwrites the value. During the its lifetime, the stored
 value must not be modified or destroyed.

lib/SIL/Verifier/MemoryLifetimeVerifier.cpp

@@ -89,6 +89,9 @@ class MemoryLifetimeVerifier {
   /// Register the destination address of a store_borrow as borrowed location.
   void registerStoreBorrowLocation(SILValue addr);
 
+  /// Registers all store_borrow instructions in a block.
+  void registerStoreBorrowsInBlock(SILBasicBlock *block);
+
   /// Handles locations of the predecessor's terminator, which are only valid
   /// in \p block.
   /// Example: @out results of try_apply. They are only valid in the
@@ -286,6 +289,13 @@ void MemoryLifetimeVerifier::registerStoreBorrowLocation(SILValue addr) {
   }
 }
 
+void MemoryLifetimeVerifier::registerStoreBorrowsInBlock(SILBasicBlock *block) {
+  for (SILInstruction &inst : *block) {
+    if (auto *sbi = dyn_cast<StoreBorrowInst>(&inst))
+      registerStoreBorrowLocation(sbi->getDest());
+  }
+}
+
 void MemoryLifetimeVerifier::initDataflow(BitDataflow &dataFlow) {
   // Initialize the entry and exit sets to all-bits-set. Except for the function
   // entry.
@@ -579,7 +589,6 @@ void MemoryLifetimeVerifier::checkBlock(SILBasicBlock *block, Bits &bits) {
       case SILInstructionKind::StoreBorrowInst: {
         SILValue destAddr = cast<StoreBorrowInst>(&I)->getDest();
         locations.setBits(bits, destAddr);
-        registerStoreBorrowLocation(destAddr);
         break;
       }
       case SILInstructionKind::CopyAddrInst: {
@@ -764,6 +773,7 @@ void MemoryLifetimeVerifier::verify() {
   locations.handleSingleBlockLocations([this](SILBasicBlock *block) {
     storeBorrowLocations.clear();
     Bits bits(locations.getNumLocations());
+    registerStoreBorrowsInBlock(block);
     checkBlock(block, bits);
   });
 }

test/SIL/memory_lifetime_failures.sil

@@ -361,6 +361,36 @@ bb0(%0 :  @guaranteed $Optional<T>):
   return %res : $()
 }
 
+// CHECK: SIL memory lifetime failure in @test_store_borrow_single_block: store-borrow location cannot be written
+sil [ossa] @test_store_borrow_single_block : $@convention(thin) (@guaranteed T) -> () {
+bb0(%0 : @guaranteed $T):
+  %stk = alloc_stack $T
+  %copy = copy_value %0 : $T
+  store %copy to [init] %stk : $*T
+  %ld = load [take] %stk : $*T
+  destroy_value %ld : $T
+  store_borrow %0 to %stk : $*T
+  dealloc_stack %stk : $*T
+  %8 = tuple ()
+  return %8 : $() 
+}
+
+// CHECK: SIL memory lifetime failure in @test_store_borrow_multi_block: store-borrow location cannot be written
+sil [ossa] @test_store_borrow_multi_block : $@convention(thin) (@guaranteed T) -> () {
+bb0(%0 : @guaranteed $T):
+  %stk = alloc_stack $T
+  %copy = copy_value %0 : $T
+  store %copy to [init] %stk : $*T
+  %ld = load [take] %stk : $*T
+  destroy_value %ld : $T
+  br bb1
+bb1:
+  store_borrow %0 to %stk : $*T
+  dealloc_stack %stk : $*T
+  %8 = tuple ()
+  return %8 : $() 
+}
+
 // CHECK: SIL memory lifetime failure in @test_cast_br_take_always: memory is not initialized, but should be
 sil [ossa] @test_cast_br_take_always : $@convention(thin) <U, V> (@in U) -> () {
 bb0(%0 : $*U):