[Exclusivity] Check that accesses are well-formed during diagnostics.

docs/SIL.rst

@@ -2456,7 +2456,7 @@ index_raw_pointer
   %2 = index_raw_pointer %0 : $Builtin.RawPointer, %1 : $Builtin.Int<n>
   // %0 must be of $Builtin.RawPointer type
   // %1 must be of a builtin integer type
-  // %2 will be of type $*T
+  // %2 will be of type $Builtin.RawPointer
 
 Given a ``Builtin.RawPointer`` value ``%0``, returns a pointer value at the
 byte offset ``%1`` relative to ``%0``.

lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp

@@ -365,6 +365,10 @@ static void diagnoseExclusivityViolation(const AccessedStorage &Storage,
                                          const BeginAccessInst *NewAccess,
                                          ASTContext &Ctx) {
 
+  DEBUG(llvm::dbgs() << "Conflict on " << *PriorAccess
+        << "\n  vs " << *NewAccess
+        << "\n  in function " << *PriorAccess->getFunction());
+
   // Can't have a conflict if both accesses are reads.
   assert(!(PriorAccess->getAccessKind() == SILAccessKind::Read &&
            NewAccess->getAccessKind() == SILAccessKind::Read));
@@ -431,30 +435,15 @@ static SILValue findUnderlyingObject(SILValue Value) {
 static AccessedStorage findAccessedStorage(SILValue Source) {
   SILValue Iter = Source;
   while (true) {
-    // Inductive cases: look through operand to find ultimate source.
-    if (auto *PBI = dyn_cast<ProjectBoxInst>(Iter)) {
-      Iter = PBI->getOperand();
-      continue;
-    }
-
-    if (auto *CVI = dyn_cast<CopyValueInst>(Iter)) {
-      Iter = CVI->getOperand();
-      continue;
-    }
-
-    if (auto *MII = dyn_cast<MarkUninitializedInst>(Iter)) {
-      Iter = MII->getOperand();
-      continue;
-    }
-
-    // Base cases: make sure ultimate source is recognized.
+    // Base case for globals: make sure ultimate source is recognized.
     if (auto *GAI = dyn_cast<GlobalAddrInst>(Iter)) {
       return AccessedStorage(GAI->getReferencedGlobal());
     }
 
+    // Base case for class objects.
     if (auto *REA = dyn_cast<RefElementAddrInst>(Iter)) {
       // Do a best-effort to find the identity of the object being projected
-      // from. It is OK to unsound here (i.e., miss when two ref_element_addrs
+      // from. It is OK to be unsound here (i.e. miss when two ref_element_addrs
       // actually refer the same address) because these will be dynamically
       // checked.
       SILValue Object = findUnderlyingObject(REA->getOperand());
@@ -463,18 +452,50 @@ static AccessedStorage findAccessedStorage(SILValue Source) {
       return AccessedStorage(AccessedStorageKind::ClassProperty, OP);
     }
 
-    if (isa<AllocBoxInst>(Iter) || isa<BeginAccessInst>(Iter) ||
-        isa<SILFunctionArgument>(Iter)) {
-      // Treat the instruction itself as the identity of the storage being
-      // being accessed.
+    switch (Iter->getKind()) {
+    // Inductive cases: look through operand to find ultimate source.
+    case ValueKind::ProjectBoxInst:
+    case ValueKind::CopyValueInst:
+    case ValueKind::MarkUninitializedInst:
+    case ValueKind::UncheckedAddrCastInst:
+    // Inlined access to subobjects.
+    case ValueKind::StructElementAddrInst:
+    case ValueKind::TupleElementAddrInst:
+    case ValueKind::UncheckedTakeEnumDataAddrInst:
+    case ValueKind::RefTailAddrInst:
+    case ValueKind::TailAddrInst:
+    case ValueKind::IndexAddrInst:
+      Iter = cast<SILInstruction>(Iter)->getOperand(0);
+      continue;
+
+    // Base address producers.
+    case ValueKind::AllocBoxInst:
+      // An AllocBox is a fully identified memory location.
+    case ValueKind::AllocStackInst:
+      // An AllocStack is a fully identified memory location, which may occur
+      // after inlining code already subjected to stack promotion.
+    case ValueKind::BeginAccessInst:
+      // The current access is nested within another access.
+      // View the outer access as a separate location because nested accesses do
+      // not conflict with each other.
+    case ValueKind::SILFunctionArgument:
+      // A function argument is effectively a nested access, enforced
+      // independently in the caller and callee.
+    case ValueKind::PointerToAddressInst:
+      // An addressor provides access to a global or class property via a
+      // RawPointer. Calling the addressor casts that raw pointer to an address.
       return AccessedStorage(Iter);
-    }
 
-    // For now we're still allowing arbitrary addresses here. Once
-    // we start doing a best-effort static check for dynamically-enforced
-    // accesses we should lock this down to only recognized sources.
-    assert(Iter->getType().isAddress() || Iter->getType().is<SILBoxType>());
-    return AccessedStorage(Iter);
+    // Unsupported address producers.
+    // Initialization is always local.
+    case ValueKind::InitEnumDataAddrInst:
+    case ValueKind::InitExistentialAddrInst:
+    // Accessing an existential value requires a cast.
+    case ValueKind::OpenExistentialAddrInst:
+    default:
+      DEBUG(llvm::dbgs() << "Bad memory access source: " << Iter);
+      llvm_unreachable("Unexpected access source.");
+    }
   }
 }
 

test/SILOptimizer/exclusivity_static_diagnostics.sil

@@ -462,3 +462,86 @@ bb23:
 bbUnreachable:
   br bb22
 }
+
+// Check that a pointer_to_address access passes diagnostics.
+//
+// CHECK-LABEL: sil hidden  @pointerToAddr
+sil hidden  @pointerToAddr : $@convention(thin) (Builtin.RawPointer) -> Int {
+bb0(%0: $Builtin.RawPointer):
+  %adr = pointer_to_address %0 : $Builtin.RawPointer to [strict] $*Int
+  %access = begin_access [read] [dynamic] %adr : $*Int
+  %val = load [trivial] %access : $*Int
+  end_access %access : $*Int
+  return %val : $Int
+}
+
+// Helper.
+struct S {
+  var x: Int
+}
+
+// Check inlined struct element access.
+// This only happens when mandatory passes are applied repeatedly.
+// e.g. importing from a debug standard library.
+//
+// CHECK-LABEL: sil hidden @inlinedStructElement
+sil hidden @inlinedStructElement : $@convention(thin) (@inout S) -> Int {
+bb0(%0 : $*S):
+  %2 = begin_access [modify] [static] %0 : $*S
+  %3 = struct_element_addr %2 : $*S, #S.x
+  %4 = begin_access [read] [static] %3 : $*Int
+  %5 = load [trivial] %4 : $*Int
+  end_access %4 : $*Int
+  end_access %2 : $*S
+  return %5 : $Int
+}
+
+// Check inlined tuple element access.
+// This only happens when mandatory passes are applied repeatedly.
+//
+// CHECK-LABEL: sil hidden @inlinedTupleElement
+sil hidden @inlinedTupleElement : $@convention(thin) (@inout (Int, Int)) -> Int {
+bb0(%0 : $*(Int, Int)):
+  %2 = begin_access [modify] [static] %0 : $*(Int, Int)
+  %3 = tuple_element_addr %2 : $*(Int, Int), 0
+  %4 = begin_access [read] [static] %3 : $*Int
+  %5 = load [trivial] %4 : $*Int
+  end_access %4 : $*Int
+  end_access %2 : $*(Int, Int)
+  return %5 : $Int
+}
+
+// Check inlined enum access.
+// This only happens when mandatory passes are applied repeatedly.
+//
+// CHECK-LABEL: sil hidden @inlinedEnumValue
+sil hidden @inlinedEnumValue : $@convention(thin) (Int) -> (@out Optional<Int>, Int) {
+bb0(%0 : $*Optional<Int>, %1 : $Int):
+  %6 = unchecked_take_enum_data_addr %0 : $*Optional<Int>, #Optional.some!enumelt.1
+  %7 = begin_access [read] [static] %6 : $*Int
+  %8 = load [trivial] %7 : $*Int
+  end_access %7 : $*Int
+  return %8 : $Int
+}
+
+// Helper.
+class Storage {}
+
+// Check inlined array access.
+// This only happens when mandatory passes are applied repeatedly.
+//
+// CHECK-LABEL: sil hidden @inlinedArrayProp
+sil hidden @inlinedArrayProp : $@convention(thin) (@guaranteed Storage, Builtin.Word) -> Int {
+bb0(%0 : $Storage, %1 : $Builtin.Word):
+  %2 = ref_tail_addr %0 : $Storage, $UInt
+  %3 = begin_access [read] [static] %2 : $*UInt
+  %4 = tail_addr %3 : $*UInt, %1 : $Builtin.Word, $Int
+  %5 = begin_access [read] [static] %4 : $*Int
+  %6 = index_addr %5 : $*Int, %1 : $Builtin.Word
+  %7 = begin_access [read] [static] %6 : $*Int
+  %8 = load [trivial] %7 : $*Int
+  end_access %7 : $*Int
+  end_access %5 : $*Int
+  end_access %3 : $*UInt
+  return %8 : $Int
+}