Merge pull request swiftlang#82450 from atrick/unsafe-mutablespan

Fix MutableSpan exclusive access to unsafe pointers

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
 // =============================================================================