[sil-combine] Do not perform existential type propagation on @in parameters when we have a copy of the underlying value.

Otherwise, we may get use-after-frees as in the added test.

rdar://50609145

Author: gottesmm

lib/SILGen/SILGenPoly.cpp

@@ -1119,12 +1119,7 @@ namespace {
             // This code assumes that we have each element at +1. So, if we do
             // not have a cleanup, we emit a load [copy]. This can occur if we
             // are translating in_guaranteed parameters.
-            IsTake_t isTakeVal = ([&] {
-              if (elt.isPlusZero()) {
-                return IsNotTake;
-              }
-              return IsTake;
-            }());
+            IsTake_t isTakeVal = elt.isPlusZero() ? IsNotTake : IsTake;
             elt = SGF.emitLoad(Loc, elt.forward(SGF),
                                SGF.getTypeLowering(elt.getType()), SGFContext(),
                                isTakeVal);

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

@@ -813,11 +813,12 @@ static bool canReplaceCopiedArg(FullApplySite Apply, SILValue Arg,
   if (!IEA)
     return false;
 
-  // If the witness method mutates Arg, we cannot replace Arg with
-  // the source of a copy. Otherwise the call would modify another value than
-  // the original argument.
+  // If the witness method does not take the value as guaranteed, we cannot
+  // replace Arg with the source of a copy. Otherwise the call would modify
+  // another value than the original argument (in the case of indirect mutating)
+  // or create a use-after-free (in the case of indirect consuming).
   auto origConv = Apply.getOrigCalleeConv();
-  if (origConv.getParamInfoForSILArg(ArgIdx).isIndirectMutating())
+  if (!origConv.getParamInfoForSILArg(ArgIdx).isIndirectInGuaranteed())
     return false;
 
   auto *DT = DA->get(Apply.getFunction());

test/SILOptimizer/sil_combine_concrete_existential.sil

@@ -499,3 +499,36 @@ bb0(%0 : $Array<Int>):
   %25 = tuple ()
   return %25 : $()
 }
+
+struct InGuaranteedArgTestNativeObjectWrapper {
+  var i: Builtin.NativeObject
+}
+
+protocol InGuaranteedArgTestProtocol {}
+extension InGuaranteedArgTestNativeObjectWrapper : InGuaranteedArgTestProtocol {}
+
+sil @in_guaranteed_arg_test_callee : $@convention(thin) <τ_0_0 where τ_0_0 : InGuaranteedArgTestProtocol> (@in τ_0_0) -> ()
+
+// The current transformation is not smart enough to remove the
+// destroy_addr. Simply substituting the copy source for the apply argument
+// would introduce a use-after-free. = (.
+//
+// CHECK-LABEL: sil @in_guaranteed_arg_test_caller : $@convention(thin) <τ_0_0 where τ_0_0 : InGuaranteedArgTestProtocol> (@in τ_0_0) -> () {
+// CHECK: apply {{%.*}}<@opened("C494A60E-71EA-11E9-B8C0-D0817AD3F8AD") InGuaranteedArgTestProtocol>
+// CHECK: } // end sil function 'in_guaranteed_arg_test_caller'
+sil @in_guaranteed_arg_test_caller : $@convention(thin) <τ_0_0 where τ_0_0 : InGuaranteedArgTestProtocol> (@in τ_0_0) -> () {
+bb0(%0 : $*τ_0_0):
+  %1 = alloc_stack $InGuaranteedArgTestProtocol
+  %2 = init_existential_addr %1 : $*InGuaranteedArgTestProtocol, $τ_0_0
+  copy_addr [take] %0 to [initialization] %2 : $*τ_0_0
+  %3 = function_ref @in_guaranteed_arg_test_callee : $@convention(thin) <τ_0_0 where τ_0_0 : InGuaranteedArgTestProtocol> (@in τ_0_0) -> ()
+  %4 = alloc_stack $InGuaranteedArgTestProtocol
+  copy_addr %1 to [initialization] %4 : $*InGuaranteedArgTestProtocol
+  %5 = open_existential_addr mutable_access %4 : $*InGuaranteedArgTestProtocol to $*@opened("C494A60E-71EA-11E9-B8C0-D0817AD3F8AD") InGuaranteedArgTestProtocol
+  apply %3<@opened("C494A60E-71EA-11E9-B8C0-D0817AD3F8AD") InGuaranteedArgTestProtocol>(%5) : $@convention(thin) <τ_0_0 where τ_0_0 : InGuaranteedArgTestProtocol> (@in τ_0_0) -> ()
+  dealloc_stack %4 : $*InGuaranteedArgTestProtocol
+  destroy_addr %1 : $*InGuaranteedArgTestProtocol
+  dealloc_stack %1 : $*InGuaranteedArgTestProtocol
+  %9999 = tuple()
+  return %9999 : $()
+}