SILCombine: fix propagation of existential self into witness method call

In case of a mutating method call we replaced the self argument with the source of a copy_addr. This let the call modify the wrong self value.

rdar://problem/34753633

Author: eeckstein

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

@@ -567,8 +567,11 @@ SILCombiner::optimizeConcatenationOfStringLiterals(ApplyInst *AI) {
 /// initialized. This is either a init_existential_addr or the source of a
 /// copy_addr. Returns a null value if the address does not dominate the
 /// alloc_stack user \p ASIUser.
+/// If the value is copied from another stack location, \p isCopied is set to
+/// true.
 static SILValue getAddressOfStackInit(AllocStackInst *ASI,
-                                      SILInstruction *ASIUser) {
+                                      SILInstruction *ASIUser,
+                                      bool &isCopied) {
   SILInstruction *SingleWrite = nullptr;
   // Check that this alloc_stack is initialized only once.
   for (auto Use : ASI->getUses()) {
@@ -588,6 +591,7 @@ static SILValue getAddressOfStackInit(AllocStackInst *ASI,
         if (SingleWrite)
           return SILValue();
         SingleWrite = CAI;
+        isCopied = true;
       }
       continue;
     }
@@ -621,23 +625,29 @@ static SILValue getAddressOfStackInit(AllocStackInst *ASI,
   if (auto *CAI = dyn_cast<CopyAddrInst>(SingleWrite)) {
     // Try to derive the type from the copy_addr that was used to
     // initialize the alloc_stack.
+    assert(isCopied && "isCopied not set for a copy_addr");
     SILValue CAISrc = CAI->getSrc();
     if (auto *ASI = dyn_cast<AllocStackInst>(CAISrc))
-      return getAddressOfStackInit(ASI, CAI);
+      return getAddressOfStackInit(ASI, CAI, isCopied);
     return CAISrc;
   }
   return SingleWrite;
 }
 
 /// Find the init_existential, which could be used to determine a concrete
 /// type of the \p Self.
+/// If the value is copied from another stack location, \p isCopied is set to
+/// true.
 static SILInstruction *findInitExistential(FullApplySite AI, SILValue Self,
                                            ArchetypeType *&OpenedArchetype,
-                                           SILValue &OpenedArchetypeDef) {
+                                           SILValue &OpenedArchetypeDef,
+                                           bool &isCopied) {
+  isCopied = false;
   if (auto *Instance = dyn_cast<AllocStackInst>(Self)) {
     // In case the Self operand is an alloc_stack where a copy_addr copies the
     // result of an open_existential_addr to this stack location.
-    if (SILValue Src = getAddressOfStackInit(Instance, AI.getInstruction()))
+    if (SILValue Src = getAddressOfStackInit(Instance, AI.getInstruction(),
+                                             isCopied))
       Self = Src;
   }
 
@@ -647,7 +657,7 @@ static SILInstruction *findInitExistential(FullApplySite AI, SILValue Self,
     if (!ASI)
       return nullptr;
 
-    SILValue StackWrite = getAddressOfStackInit(ASI, Open);
+    SILValue StackWrite = getAddressOfStackInit(ASI, Open, isCopied);
     if (!StackWrite)
       return nullptr;
 
@@ -853,8 +863,10 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI,
   // determine a concrete type of the self.
   ArchetypeType *OpenedArchetype = nullptr;
   SILValue OpenedArchetypeDef;
+  bool isCopied = false;
   SILInstruction *InitExistential =
-    findInitExistential(AI, Self, OpenedArchetype, OpenedArchetypeDef);
+    findInitExistential(AI, Self, OpenedArchetype, OpenedArchetypeDef,
+                        isCopied);
   if (!InitExistential)
     return nullptr;
 
@@ -888,6 +900,18 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI,
   // Propagate the concrete type into the callee-operand if required.
   Propagate(ConcreteType, Conformance);
 
+  if (isCopied) {
+    // If the witness method is mutating self, we cannot replace self with
+    // the source of a copy. Otherwise the call would modify another value than
+    // the original self.
+    switch (AI.getArgumentConvention(AI.getNumArguments() - 1)) {
+      case SILArgumentConvention::ConventionType::Indirect_Inout:
+      case SILArgumentConvention::ConventionType::Indirect_InoutAliasable:
+        return nullptr;
+      default:
+        break;
+    }
+  }
   // Create a new apply instruction that uses the concrete type instead
   // of the existential type.
   auto *NewAI = createApplyWithConcreteType(AI, NewSelf, Self, ConcreteType,

test/SILOptimizer/sil_combine_apply.sil

@@ -308,4 +308,38 @@ bb0(%0 : $*SwiftP):
   %4 = apply %3() : $@callee_owned () -> ()
   %9999 = tuple()
   return %9999 : $()
-}
+}
+
+protocol MutatingProto {
+    mutating func mutatingMethod()
+}
+
+struct MStruct : MutatingProto { 
+
+    var somevar: Builtin.Int32
+
+    mutating func mutatingMethod()
+}
+
+// CHECK-LABEL: sil @dont_replace_copied_self_in_mutating_method_call
+sil @dont_replace_copied_self_in_mutating_method_call : $@convention(thin) (MStruct) -> (@out MutatingProto) {
+bb0(%0 : $*MutatingProto, %1 : $MStruct):
+  %2 = alloc_stack $MutatingProto
+  %4 = init_existential_addr %2 : $*MutatingProto, $MStruct
+  store %1 to %4 : $*MStruct
+  %9 = alloc_stack $MutatingProto
+  copy_addr %2 to [initialization] %9 : $*MutatingProto
+  // CHECK: [[E:%[0-9]+]] = open_existential_addr
+  %11 = open_existential_addr mutable_access %9 : $*MutatingProto to $*@opened("FC5F3CFA-A7A4-11E7-911F-685B35C48C83") MutatingProto
+  // CHECK: [[M:%[0-9]+]] = witness_method $MStruct,
+  %12 = witness_method $@opened("FC5F3CFA-A7A4-11E7-911F-685B35C48C83") MutatingProto, #MutatingProto.mutatingMethod!1 : <Self where Self : MutatingProto> (inout Self) -> () -> (), %11 : $*@opened("FC5F3CFA-A7A4-11E7-911F-685B35C48C83") MutatingProto : $@convention(witness_method) <τ_0_0 where τ_0_0 : MutatingProto> (@inout τ_0_0) -> ()
+  // CHECK: apply [[M]]<@opened("{{.*}}") MutatingProto>([[E]]) :
+  %13 = apply %12<@opened("FC5F3CFA-A7A4-11E7-911F-685B35C48C83") MutatingProto>(%11) : $@convention(witness_method) <τ_0_0 where τ_0_0 : MutatingProto> (@inout τ_0_0) -> ()
+  copy_addr [take] %9 to [initialization] %0 : $*MutatingProto
+  dealloc_stack %9 : $*MutatingProto
+  destroy_addr %2 : $*MutatingProto
+  dealloc_stack %2 : $*MutatingProto
+  %27 = tuple ()
+  return %27 : $()
+}
+