Fix EscapeAnalysis connection graph for existential values.

Change the connection graph builder to map an existential address and
its opened address onto the same node.

Fixes <rdar://59559805> miscompile; use-after-free

Immediate bug: EscapeAnalysis::mayReleaseContent incorrectly returns
'false' when comparing a store into an existential's value with a
release of the existential.

How this was exposed: mayReleaseContent was made more aggressive so
that only the connection graph node representing the released object
is considered to be "released". This fails for existentials because a
separate connection graph node is created for the value within the
existential.

This is just one manifestation of a broader bug. Representing an
existential as two logically distinct objects could also result in
incorrect escaping information and incorrect alias analysis. Note that
copying directly into an existential address and storing into the
opened value in fact modify the same memory. Furthermore, when opening
an existential, no local reference count is used to keep the opened
value "alive" independent from the existential (this is another
assumption made by mayReleaseContent). This is always how existentials
were represented in the connection graph, but issues had never been
exposed.

Author: atrick

lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

@@ -122,6 +122,8 @@ SILValue EscapeAnalysis::getPointerBase(SILValue value) {
   case ValueKind::StructElementAddrInst:
   case ValueKind::StructExtractInst:
   case ValueKind::TupleElementAddrInst:
+  case ValueKind::InitExistentialAddrInst:
+  case ValueKind::OpenExistentialAddrInst:
   case ValueKind::BeginAccessInst:
   case ValueKind::UncheckedTakeEnumDataAddrInst:
   case ValueKind::UncheckedEnumDataInst:
@@ -2165,9 +2167,7 @@ void EscapeAnalysis::analyzeInstruction(SILInstruction *I,
     }
     case SILInstructionKind::RefElementAddrInst:
     case SILInstructionKind::RefTailAddrInst:
-    case SILInstructionKind::ProjectBoxInst:
-    case SILInstructionKind::InitExistentialAddrInst:
-    case SILInstructionKind::OpenExistentialAddrInst: {
+    case SILInstructionKind::ProjectBoxInst: {
       // For projections into objects, get the non-address reference operand and
       // return an interior content node that the reference points to.
       auto SVI = cast<SingleValueInstruction>(I);

test/SILOptimizer/escape_analysis.sil

@@ -1019,9 +1019,8 @@ bb0(%0 : $Builtin.Int64, %1 : $X, %2 : $X, %3 : $X):
 
 // CHECK-LABEL: CG of test_existential_addr
 // CHECK-NEXT:    Arg [ref] %0 Esc: A, Succ: 
-// CHECK-NEXT:    Val %1 Esc: , Succ: (%2)
-// CHECK-NEXT:    Con %2 Esc: , Succ: (%2.1)
-// CHECK-NEXT:    Con [ref] %2.1 Esc: , Succ: %0
+// CHECK-NEXT:    Val %1 Esc: , Succ: (%1.1)
+// CHECK-NEXT:    Con [ref] %1.1 Esc: , Succ: %0
 // CHECK-NEXT:  End
 sil @test_existential_addr : $@convention(thin) (@owned Pointer) -> () {
 bb0(%0 : $Pointer):

test/SILOptimizer/escape_analysis_invalidate.sil

@@ -1,8 +1,15 @@
-// RUN: %target-sil-opt %s -temp-rvalue-opt -enable-sil-verify-all -escapes-internal-verify | %FileCheck %s
+// RUN: %target-sil-opt %s -temp-rvalue-opt -enable-sil-verify-all -escapes-internal-verify -wmo | %FileCheck %s
 //
 // TempRValue iteratively uses EscapeAnalysis and deletes
 // instructions. Make sure that the connection graph remains valid
 // <rdar://57290845>.
+//
+// This test requires -wmo so EscapeAnalysis can find all
+// implementations of SomeProtocol.foo. Otherwise the existential
+// address appears to escape. As an alternative, we could be more
+// aggressive about considering address-type argument not to escape,
+// but that would require some limiting address_to_pointer to never
+// occur on an exclusive address argument.
 
 import Swift
 
@@ -82,3 +89,24 @@ bb0(%0 : $*SomeProtocol, %1 : $SomeClass):
   %v = tuple ()
   return %v : $()
 }
+
+sil hidden @$s26escape_analysis_invalidate12SomeInstanceV3fooyyF : $@convention(method) (SomeInstance) -> () {
+bb0(%0 : $SomeInstance):
+  debug_value %0 : $SomeInstance, let, name "self", argno 1 // id: %1
+  %2 = tuple ()                                   // user: %3
+  return %2 : $()                                 // id: %3
+}
+
+sil private [transparent] [thunk] @$s26escape_analysis_invalidate12SomeInstanceVAA0A8ProtocolA2aDP3fooyyFTW : $@convention(witness_method: SomeProtocol) (@in_guaranteed SomeInstance) -> () {
+bb0(%0 : $*SomeInstance):
+  %1 = load %0 : $*SomeInstance
+  // function_ref SomeInstance.foo()
+  %2 = function_ref @$s26escape_analysis_invalidate12SomeInstanceV3fooyyF : $@convention(method) (SomeInstance) -> ()
+  %3 = apply %2(%1) : $@convention(method) (SomeInstance) -> ()
+  %4 = tuple ()
+  return %4 : $()
+}
+
+sil_witness_table hidden SomeInstance: SomeProtocol module t {
+  method #SomeProtocol.foo!1: <Self where Self : SomeProtocol> (Self) -> () -> () : @$s26escape_analysis_invalidate12SomeInstanceVAA0A8ProtocolA2aDP3fooyyFTW     // protocol witness for SomeProtocol.foo() in conformance SomeInstance
+}

test/SILOptimizer/escape_analysis_reduced.sil

@@ -499,17 +499,18 @@ bb2:
 }
 
 // CHECK-LABEL: CG of testPendingMerge
-// CHECK-NEXT:   Arg %0 Esc: A, Succ: (%22)
-// CHECK-NEXT:   Arg [ref] %1 Esc: G, Succ: (%9.1)
+// CHECK-NEXT:   Arg %0 Esc: A, Succ: (%0.1)
+// CHECK-NEXT:   Con %0.1 Esc: A, Succ: %1, %4.1
+// CHECK-NEXT:   Arg [ref] %1 Esc: A, Succ: (%9.1)
 // CHECK-NEXT:   Val %2 Esc: , Succ: (%9)
 // CHECK-NEXT:   Val %4 Esc: , Succ: (%4.1)
-// CHECK-NEXT:   Con %4.1 Esc: A, Succ: (%9.1), %7, %9
-// CHECK-NEXT:   Val %5 Esc: , Succ: (%7)
-// CHECK-NEXT:   Con %7 Esc: A, Succ: (%9.1)
+// CHECK-NEXT:   Con %4.1 Esc: A, Succ: (%9.1), %5.1, %9
+// CHECK-NEXT:   Val %5 Esc: , Succ: (%5.1)
+// CHECK-NEXT:   Con %5.1 Esc: A, Succ: %1
 // CHECK-NEXT:   Con [ref] %9 Esc: A, Succ: (%9.1)
-// CHECK-NEXT:   Con [int] %9.1 Esc: G, Succ: (%9.1), %1
-// CHECK-NEXT:   Con %22 Esc: A, Succ: (%22.1)
-// CHECK-NEXT:   Con %22.1 Esc: A, Succ: %1, %4.1
+// CHECK-NEXT:   Con [int] %9.1 Esc: G, Succ: (%9.2)
+// CHECK-NEXT:   Con %9.2 Esc: G, Succ: (%9.3)
+// CHECK-NEXT:   Con %9.3 Esc: G, Succ:
 // CHECK-LABEL: End
 sil private @testPendingMerge : $@convention(thin) (@owned VariableNode) -> (@out ASTNode, @error Error) {
 bb0(%0 : $*ASTNode, %1 : $VariableNode):

test/SILOptimizer/escape_analysis_release_hoisting.sil

@@ -0,0 +1,48 @@
+// RUN: %target-sil-opt -enable-sil-verify-all -release-hoisting %s | %FileCheck %s
+// REQUIRES: CPU=x86_64
+// REQUIRES: OS=macosx
+
+sil_stage canonical
+
+import Builtin
+import Swift
+import SwiftShims
+
+// Test <rdar://59559805> miscompile; use-after-free
+// Avoid hoisting strong_release of an existential
+// over a store to the value.
+protocol ArrayElementProtocol {}
+
+struct ArrayElementStruct : ArrayElementProtocol {
+  @_hasStorage @_hasInitialValue var dummy: Int { get set }
+}
+
+// CHECK-LABEL: sil @testArrayStorageInitAfterFree : $@convention(thin) () -> () {
+// CHECK:   [[ARRAY:%.*]] = alloc_ref [stack] [tail_elems $ArrayElementProtocol * undef : $Builtin.Word] $_ContiguousArrayStorage<ArrayElementProtocol>
+// CHECK:   [[CAST:%.*]] = upcast [[ARRAY]] : $_ContiguousArrayStorage<ArrayElementProtocol> to $__ContiguousArrayStorageBase
+// CHECK:   [[TAIL:%.*]] = ref_tail_addr [[CAST]] : $__ContiguousArrayStorageBase, $ArrayElementProtocol
+// CHECK:   [[ADR:%.*]] = init_existential_addr [[TAIL]] : $*ArrayElementProtocol, $ArrayElementStruct
+// CHECK:   store %{{.*}} to [[ADR]] : $*ArrayElementStruct
+// CHECK:   strong_release [[ARRAY]] : $_ContiguousArrayStorage<ArrayElementProtocol>
+// CHECK-LABEL: } // end sil function 'testArrayStorageInitAfterFree'
+sil @testArrayStorageInitAfterFree : $@convention(thin) () -> () {
+bb0:
+  %0 = alloc_ref [stack] [tail_elems $ArrayElementProtocol * undef : $Builtin.Word] $_ContiguousArrayStorage<ArrayElementProtocol>
+  %1 = upcast %0 : $_ContiguousArrayStorage<ArrayElementProtocol> to $__ContiguousArrayStorageBase
+  %2 = struct $_SwiftArrayBodyStorage (undef : $Int, undef : $UInt)
+  %3 = struct $_ArrayBody (%2 : $_SwiftArrayBodyStorage)
+  %4 = ref_element_addr %1 : $__ContiguousArrayStorageBase, #__ContiguousArrayStorageBase.countAndCapacity
+  store %3 to %4 : $*_ArrayBody
+  %6 = ref_tail_addr %1 : $__ContiguousArrayStorageBase, $ArrayElementProtocol
+  %7 = value_to_bridge_object undef : $Builtin.Int64
+  %8 = struct $_StringObject (undef : $UInt64, %7 : $Builtin.BridgeObject)
+  %9 = struct $_StringGuts (%8 : $_StringObject)
+  %10 = struct $String (%9 : $_StringGuts)
+  %11 = struct $ArrayElementStruct (undef : $Int)
+  %12 = init_existential_addr %6 : $*ArrayElementProtocol, $ArrayElementStruct
+  store %11 to %12 : $*ArrayElementStruct
+  strong_release %0 : $_ContiguousArrayStorage<ArrayElementProtocol>
+  dealloc_ref [stack] %0 : $_ContiguousArrayStorage<ArrayElementProtocol>
+  %16 = tuple ()
+  return %16 : $()
+}