Teach exclusivity access marker verifier to handle UnsafePointer.

My previous attempt to strengthen this verification ignored the fact that a
projection or addressor that returns UnsafePointer can have nested access after
being passed as an @inout argument. After inlining, this caused the verifier to
assert.

Instead, handle access to an UnsafePointer as a valid but unidentified access. I
was trying to avoid this because an UnsafePointer could refer to a global or
class property, which we can never consider "Unidentified". However, this is
reasonably safe because it can only result from a _nested_ access. If a global
or class property is being addressed, it must already have its own dynamic
access within the addressor, and that access will be exposed to the
optimizer. If a non-public KeyPath is being addressed, a keypath instruction
must already exist somewhere in the module which is exposed to the optimizer.

Fixes <rdar://problem/41660554> Swift CI (macOS release master, OSS): SIL verification failed: Unknown formal access pattern: storage.

Author: atrick

lib/SIL/MemAccessUtils.cpp

@@ -181,6 +181,16 @@ static bool isExternalGlobalAddressor(ApplyInst *AI) {
   return funcRef->isGlobalInit() && funcRef->isExternalDeclaration();
 }
 
+// Return true if the given StructExtractInst extracts the RawPointer from
+// Unsafe[Mutable]Pointer.
+static bool isUnsafePointerExtraction(StructExtractInst *SEI) {
+  assert(isa<BuiltinRawPointerType>(SEI->getType().getASTType()));
+  auto &C = SEI->getModule().getASTContext();
+  auto *decl = SEI->getStructDecl();
+  return decl == C.getUnsafeMutablePointerDecl()
+    || decl == C.getUnsafePointerDecl();
+}
+
 // Given an address base is a block argument, verify that it is actually a box
 // projected from a switch_enum. This is a valid pattern at any SIL stage
 // resulting in a block-type phi. In later SIL stages, the optimizer may form
@@ -248,6 +258,14 @@ AccessedStorage swift::findAccessedStorage(SILValue sourceAddr) {
       // Don't currently allow any other calls to return an accessed address.
       return AccessedStorage();
 
+    case ValueKind::StructExtractInst:
+      // Handle nested access to a KeyPath projection. The projection itself
+      // uses a Builtin. However, the returned UnsafeMutablePointer may be
+      // converted to an address and accessed via an inout argument.
+      if (isUnsafePointerExtraction(cast<StructExtractInst>(address)))
+        return AccessedStorage(address, AccessedStorage::Unidentified);
+      return AccessedStorage();
+
     // A block argument may be a box value projected out of
     // switch_enum. Address-type block arguments are not allowed.
     case ValueKind::SILPHIArgument:
@@ -297,8 +315,8 @@ AccessedStorage swift::findAccessedStorage(SILValue sourceAddr) {
       continue;
 
     // Access to a Builtin.RawPointer. Treat this like the inductive cases
-    // above because some RawPointer's originate from identified locations. See
-    // the special case for global addressors, which return RawPointer above.
+    // above because some RawPointers originate from identified locations. See
+    // the special case for global addressors, which return RawPointer, above.
     //
     // If the inductive search does not find a valid addressor, it will
     // eventually reach the default case that returns in invalid location. This

test/Interpreter/enforce_exclusive_access.swift

@@ -306,7 +306,7 @@ ExclusiveAccessTestSuite.test("SequentialKeyPathWritesDontOverlap") {
 
   c[keyPath: getF] = 7
   c[keyPath: getF] = 8 // no-trap
-  c[keyPath: getF] += c[keyPath: getF] + 1 // no-trap
+  c[keyPath: getF] += c[keyPath: getF] // no-trap
 }
 
 ExclusiveAccessTestSuite.test("KeyPathInoutKeyPathWriteClassStoredProp")

test/SILOptimizer/exclusivity_static_diagnostics.sil

@@ -1106,3 +1106,31 @@ bb0(%0 : $UnsafePointer<Int32>, %1 : $*Int32):
   %v = tuple ()
   return %v : $()
 }
+
+// <rdar://problem/41660554> Swift CI (macOS release master, OSS): SIL verification failed: Unknown formal access pattern: storage
+// Test access marker verification of a KeyPath projection with nested @inout access after inlining.
+sil @takeInoutInt : $@convention(thin) (@inout Int) -> ()
+
+// The compiler intrinsic _projectXXXKeyPath returns a tuple of
+// UnsafePointer and Optional AnyObject. After inlining, the unsafe
+// pointer may appear to originate from anywhere, including a phi.
+// There's currently no way to tell that the UnsafePointer originated
+// from a KeyPath projection. This pattern could occur with any
+// addressor. In either case, the nested access is valid but
+// unidentified. Addressors that require enforcement must start a
+// dynamic access within the addressor itself, before returning an
+// UnsafePointer.
+sil @testNestedKeypathAccess : $@convention(thin) (@guaranteed (UnsafeMutablePointer<Int>, Optional<AnyObject>)) -> () {
+bb0(%0 : $(UnsafeMutablePointer<Int>, Optional<AnyObject>)):
+  %up = tuple_extract %0 : $(UnsafeMutablePointer<Int>, Optional<AnyObject>), 0
+  %o = tuple_extract %0 : $(UnsafeMutablePointer<Int>, Optional<AnyObject>), 1
+  %p = struct_extract %up : $UnsafeMutablePointer<Int>, #UnsafeMutablePointer._rawValue
+  %adr = pointer_to_address %p : $Builtin.RawPointer to [strict] $*Int
+  %dep = mark_dependence %adr : $*Int on %o : $Optional<AnyObject>
+  %access = begin_access [modify] [static] %dep : $*Int
+  %f = function_ref @takeInoutInt : $@convention(thin) (@inout Int) -> ()
+  %call = apply %f(%access) : $@convention(thin) (@inout Int) -> ()
+  end_access %access : $*Int
+  %v = tuple ()
+  return %v : $()
+}