Fix MutableSpan exclusive access to unsafe pointers

This fix enables exclusive access to a MutableSpan created from an UnsafeMutablePointer.

The compiler has a special case that allows MutableSpan to depend on a mutable
pointer *without* extending that pointer's access scope. That lets us implement
standard library code like this:

    mutating public func extracting(droppingLast k: Int) -> Self {
      //...
      let newSpan = unsafe Self(_unchecked: _pointer, byteCount: newCount)
      return unsafe _overrideLifetime(newSpan, mutating: &self)

Refine this special case so that is does not apply to inout parameters where the
programmer has an expectation that the unsafe pointer is not copied when being
passed as an argument. Now, we safely get an exclusivity violation when creating
two mutable spans from the same pointer field:

    @Lifetime(&self)
    mutating func getSpan() -> MutableSpan<T> {
      let span1 = makeMutableSpan(&self.pointer)
      let span2 = makeMutableSpan(&self.pointer) // ERROR: overlapping access
      return span1
    }

If we don't fix this now, it will likely be source breaking in the future.

Fixes rdar://153745332 (Swift allows constructing two MutableSpans to the same underlying pointer)

Author: atrick

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift

@@ -359,7 +359,7 @@ private struct LifetimeVariable {
 
   private func getFirstVariableIntroducer(of value: Value, _ context: some Context) -> Value? {
     var introducer: Value?
-    var useDefVisitor = VariableIntroducerUseDefWalker(context, scopedValue: value) {
+    var useDefVisitor = VariableIntroducerUseDefWalker(context, scopedValue: value, ignoreTrivialCopies: false) {
       introducer = $0
       return .abortWalk
     }

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceInsertion.swift

@@ -95,6 +95,7 @@ extension LifetimeDependentApply {
   struct LifetimeSource {
     let targetKind: TargetKind
     let convention: LifetimeDependenceConvention
+    let isInout: Bool
     let value: Value
   }
 
@@ -116,7 +117,8 @@ extension LifetimeDependentApply {
       guard let dep = applySite.resultDependence(on: operand) else {
         continue
       }
-      info.sources.push(LifetimeSource(targetKind: .result, convention: dep, value: operand.value))
+      let isInout = applySite.convention(of: operand)?.isInout ?? false
+      info.sources.push(LifetimeSource(targetKind: .result, convention: dep, isInout: isInout, value: operand.value))
     }
     return info
   }
@@ -135,6 +137,7 @@ extension LifetimeDependentApply {
         ? TargetKind.yieldAddress : TargetKind.yield
       info.sources.push(LifetimeSource(targetKind: targetKind,
                                        convention: .scope(addressable: false, addressableForDeps: false),
+                                       isInout: false,
                                        value: beginApply.token))
     }
     for operand in applySite.parameterOperands {
@@ -151,7 +154,9 @@ extension LifetimeDependentApply {
         // However this is neccessary for safety when begin_apply gets inlined which will delete the dependence on the token.
         for yieldedValue in beginApply.yieldedValues {
           let targetKind = yieldedValue.type.isAddress ? TargetKind.yieldAddress : TargetKind.yield
-          info.sources.push(LifetimeSource(targetKind: targetKind, convention: dep, value: operand.value))
+          let isInout = applySite.convention(of: operand)?.isInout ?? false
+          info.sources.push(LifetimeSource(targetKind: targetKind, convention: dep, isInout: isInout,
+                                           value: operand.value))
         }
       }
     }
@@ -180,7 +185,8 @@ extension LifetimeDependentApply {
       guard let dep = dep else {
         continue
       }
-      info.sources.push(LifetimeSource(targetKind: targetKind, convention: dep, value: operand.value))
+      let isInout = applySite.convention(of: operand)?.isInout ?? false
+      info.sources.push(LifetimeSource(targetKind: targetKind, convention: dep, isInout: isInout, value: operand.value))
     }
     return info
   }
@@ -223,7 +229,7 @@ private extension LifetimeDependentApply.LifetimeSourceInfo {
         return
       }
       // Create a new dependence on the apply's access to the argument.
-      for varIntroducer in gatherVariableIntroducers(for: source.value, context) {
+      for varIntroducer in gatherVariableIntroducers(for: source.value, ignoreTrivialCopies: !source.isInout, context) {
         let scope = LifetimeDependence.Scope(base: varIntroducer, context)
         log("Scoped lifetime from \(source.value)")
         log("  scope: \(scope)")
@@ -316,11 +322,12 @@ private func insertMarkDependencies(value: Value, initializer: Instruction?,
 /// - a variable declaration (begin_borrow [var_decl], move_value [var_decl])
 /// - a begin_access for a mutable variable access
 /// - the value or address "root" of the dependence chain
-func gatherVariableIntroducers(for value: Value, _ context: Context)
+func gatherVariableIntroducers(for value: Value, ignoreTrivialCopies: Bool, _ context: Context)
   -> SingleInlineArray<Value>
 {
   var introducers = SingleInlineArray<Value>()
-  var useDefVisitor = VariableIntroducerUseDefWalker(context, scopedValue: value) {
+  var useDefVisitor = VariableIntroducerUseDefWalker(context, scopedValue: value,
+                                                     ignoreTrivialCopies: ignoreTrivialCopies) {
     introducers.push($0)
     return .continueWalk
   }
@@ -403,11 +410,15 @@ struct VariableIntroducerUseDefWalker : LifetimeDependenceUseDefValueWalker, Lif
   // Call \p visit rather than calling this directly.
   private let visitorClosure: (Value) -> WalkResult
 
-  init(_ context: Context, scopedValue: Value, _ visitor: @escaping (Value) -> WalkResult) {
+  init(_ context: Context, scopedValue: Value, ignoreTrivialCopies: Bool, _ visitor: @escaping (Value) -> WalkResult) {
     self.context = context
-    self.isTrivialScope = scopedValue.type.isAddress
-      ? scopedValue.type.objectType.isTrivial(in: scopedValue.parentFunction)
-      : scopedValue.isTrivial(context)
+    if ignoreTrivialCopies {
+      self.isTrivialScope = scopedValue.type.isAddress
+        ? scopedValue.type.objectType.isTrivial(in: scopedValue.parentFunction)
+        : scopedValue.isTrivial(context)
+    } else {
+      self.isTrivialScope = false
+    }
     self.visitedValues = ValueSet(context)
     self.visitorClosure = visitor
   }
@@ -472,5 +483,5 @@ let variableIntroducerTest = FunctionTest("variable_introducer") {
     function, arguments, context in
   let value = arguments.takeValue()
   print("Variable introducers of: \(value)")
-  print(gatherVariableIntroducers(for: value, context))
+  print(gatherVariableIntroducers(for: value, ignoreTrivialCopies: false, context))
 }

test/SILOptimizer/lifetime_dependence/semantics.swift

@@ -106,13 +106,21 @@ struct InnerTrivial {
 struct TrivialHolder {
   var p: UnsafePointer<Int>
   var pa: UnsafePointer<AddressableInt>
+  var mp: UnsafeMutablePointer<Int>
 
   var addressableInt: AddressableInt { unsafeAddress { pa } }
 
   @_lifetime(borrow self)
   borrowing func span() -> Span<Int> {
     Span(base: p, count: 1)
   }
+
+  @_lifetime(&self)
+  mutating func mutableSpan() -> MutableSpan<Int> {
+    MutableSpan(base: mp, count: 1)
+  }
+
+  mutating func modify() {}
 }
 
 struct Holder {
@@ -454,6 +462,28 @@ func testInoutMutableBorrow(a: inout [Int]) -> MutableSpan<Int> {
   a.mutableSpan()
 }
 
+@_lifetime(&h)
+func testTrivialWriteConflict(h: inout TrivialHolder) -> MutableSpan<Int> {
+  let span = h.mutableSpan() // expected-error{{overlapping accesses to 'h', but modification requires exclusive access; consider copying to a local variable}}
+  h.modify() // expected-note{{conflicting access is here}}
+  return span
+}
+
+func makeMutableSpan(_ p: inout UnsafeMutablePointer<UInt8>) -> MutableSpan<UInt8> {
+  MutableSpan(base: p, count: 1)
+}
+
+struct TestInoutUnsafePointerExclusivity {
+  var pointer: UnsafeMutablePointer<UInt8>
+
+  @_lifetime(&self)
+  mutating func testInoutUnsafePointerExclusivity() -> MutableSpan<UInt8> {
+    let span1 = makeMutableSpan(&self.pointer) // expected-error{{overlapping accesses to 'self.pointer', but modification requires exclusive access; consider copying to a local variable}}
+    _ = makeMutableSpan(&self.pointer) // expected-note{{conflicting access is here}}
+    return span1
+  }
+}
+
 // =============================================================================
 // Scoped dependence on property access
 // =============================================================================