[move-function-addr] When inserting compensating destroy_addr, insert debug_value undef before the destroy_addr.

Otherwise in async contexts we may propagate forward an original non-invalidated
debug_value created a use of the invalidated memory. This then causes the
MemoryLifetimeVerifier to trigger.

I also discovered via this test case that we were not ignoring end_access as a
liveness use when visiting non-closure uses. With this change we ignore it
now. That being said, the reason why the end_access has not been an issue
previously is that due to the hacky way that the move builtin lowers, we
actually create the move outside of the relevant access scope (which even worse
is a read!). I am going to fix that in a different PR though.

Author: gottesmm

lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp

@@ -1363,6 +1363,10 @@ bool GatherLexicalLifetimeUseVisitor::visitUse(Operand *op,
   if (isa<DeallocStackInst>(op->getUser()))
     return true;
 
+  // Ignore end_access.
+  if (isa<EndAccessInst>(op->getUser()))
+    return true;
+
   LLVM_DEBUG(llvm::dbgs() << "Found liveness use: " << *op->getUser());
   useState.livenessUses.insert(op->getUser());
 
@@ -1507,6 +1511,24 @@ bool DataflowState::cleanupAllDestroyAddr(
       SILBuilderWithScope builder(iter);
       auto *dvi = builder.createDestroyAddr(
           RegularLocation::getAutoGeneratedLocation(), address);
+      // Create a debug_value undef if we have debug info to stop the async dbg
+      // info propagation from creating debug info for an already destroyed
+      // value. We use a separate builder since we need to control the debug
+      // scope/location to get llvm to do the right thing.
+      if (addressDebugInst) {
+        if (auto varInfo = addressDebugInst.getVarInfo()) {
+          // We need to always insert /after/ the reinit since the value will
+          // not be defined before the value.
+          SILBuilderWithScope dbgValueInsertBuilder(dvi);
+          dbgValueInsertBuilder.setCurrentDebugScope(
+              addressDebugInst->getDebugScope());
+          dbgValueInsertBuilder.createDebugValue(
+              addressDebugInst.inst->getLoc(),
+              SILUndef::get(address->getType(), dvi->getModule()), *varInfo,
+              false,
+              /*was moved*/ true);
+        }
+      }
       useState.destroys.insert(dvi);
       continue;
     }

test/SILOptimizer/move_function_kills_addresses_dbginfo.sil

@@ -5,6 +5,28 @@
 sil_stage raw
 
 import Builtin
+import Swift
+
+//////////////////
+// Declarations //
+//////////////////
+
+public protocol P {
+    static var value: P { get }
+    func doSomething()
+}
+
+sil @forceSplit : $@convention(thin) @async () -> ()
+sil @genericUse : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+
+enum Optional<T> {
+case some(T)
+case none
+}
+
+///////////
+// Tests //
+///////////
 
 // Make sure that when we process a move_addr on an alloc_stack/debug_value that
 // we properly put the [moved] marker on them.
@@ -71,3 +93,63 @@ bb3:
   %9999 = tuple()
   return %9999 : $()
 }
+
+// Make sure that we insert a debug_value undef after the destroy_addr that we
+// place in block bb2.
+// CHECK-LABEL: sil [ossa] @dbg_undef_in_inserted_destroy_addr : $@convention(thin) @async <T where T : P> (@inout T) -> () {
+// CHECK: bb2:
+// CHECK-NEXT: // function_ref forceSplit
+// CHECK-NEXT: function_ref @forceSplit
+// CHECK-NEXT: apply
+// CHECK-NEXT: debug_value [moved] undef
+// CHECK-NEXT: destroy_addr
+// CHECK-NEXT: hop_to_executor
+// CHECK-NEXT: br bb3
+sil [ossa] @dbg_undef_in_inserted_destroy_addr : $@convention(thin) @async <T where T : P> (@inout T) -> () {
+bb0(%0 : $*T):
+  debug_value %0 : $*T, var, name "msg", argno 1, expr op_deref
+  %2 = enum $Optional<Builtin.Executor>, #Optional.none!enumelt
+  hop_to_executor %2 : $Optional<Builtin.Executor>
+  %4 = function_ref @forceSplit : $@convention(thin) @async () -> ()
+  %5 = apply %4() : $@convention(thin) @async () -> ()
+  hop_to_executor %2 : $Optional<Builtin.Executor>
+  cond_br undef, bb1, bb2
+
+bb1:
+  %11 = alloc_stack $T
+  %12 = begin_access [modify] [static] %0 : $*T
+  mark_unresolved_move_addr %12 to %11 : $*T
+  end_access %12 : $*T
+  %21 = function_ref @genericUse : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+  %22 = apply %21<T>(%11) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+  destroy_addr %11 : $*T
+  dealloc_stack %11 : $*T
+  %25 = function_ref @forceSplit : $@convention(thin) @async () -> ()
+  %26 = apply %25() : $@convention(thin) @async () -> ()
+  hop_to_executor %2 : $Optional<Builtin.Executor>
+  br bb3
+
+bb2:
+  %29 = function_ref @forceSplit : $@convention(thin) @async () -> ()
+  %30 = apply %29() : $@convention(thin) @async () -> ()
+  hop_to_executor %2 : $Optional<Builtin.Executor>
+  br bb3
+
+bb3:
+  %33 = alloc_stack $P
+  %34 = metatype $@thick T.Type
+  %35 = witness_method $T, #P.value!getter : <Self where Self : P> (Self.Type) -> () -> P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@thick τ_0_0.Type) -> @out P
+  %36 = apply %35<T>(%33, %34) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@thick τ_0_0.Type) -> @out P
+  %37 = alloc_stack $T
+  unconditional_checked_cast_addr P in %33 : $*P to T in %37 : $*T
+  %39 = begin_access [modify] [static] %0 : $*T
+  copy_addr [take] %37 to %39 : $*T
+  end_access %39 : $*T
+  dealloc_stack %37 : $*T
+  dealloc_stack %33 : $*P
+  %44 = function_ref @forceSplit : $@convention(thin) @async () -> ()
+  %45 = apply %44() : $@convention(thin) @async () -> ()
+  hop_to_executor %2 : $Optional<Builtin.Executor>
+  %47 = tuple ()
+  return %47 : $()
+}