Merge pull request #82451 from atrick/62-unsafe-mutablespan

[6.2] Fix MutableSpan exclusive access to unsafe pointers

Author: atrick

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift

@@ -199,9 +199,16 @@ private struct DiagnoseDependence {
     if function.hasUnsafeNonEscapableResult {
       return .continueWalk
     }
-    // If the dependence scope is global, then it has immortal lifetime.
-    if case .global = dependence.scope {
+    // Check for immortal lifetime.
+    switch dependence.scope {
+    case .global:
       return .continueWalk
+    case let .unknown(value):
+      if value.type.isVoid {
+        return .continueWalk
+      }
+    default:
+      break
     }
     // Check that the parameter dependence for this result is the same
     // as the current dependence scope.
@@ -352,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,8 +229,8 @@ private extension LifetimeDependentApply.LifetimeSourceInfo {
         return
       }
       // Create a new dependence on the apply's access to the argument.
-      for varIntoducer in gatherVariableIntroducers(for: source.value, context) {
-        let scope = LifetimeDependence.Scope(base: varIntoducer, 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)")
         bases.append(scope.parentValue)
@@ -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
   }
@@ -467,5 +478,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))
 }

SwiftCompilerSources/Sources/Optimizer/Utilities/ForwardingUtils.swift

@@ -118,7 +118,7 @@ extension ForwardingUseDefWalker {
   }
   mutating func walkUpDefault(forwarded value: Value, _ path: PathContext)
     -> WalkResult {
-    if let inst = value.forwardingInstruction {
+    if let inst = value.forwardingInstruction, !inst.forwardedOperands.isEmpty {
       return walkUp(instruction: inst, path)
     }
     if let phi = Phi(value) {

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 {
@@ -456,6 +464,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
 // =============================================================================

test/SILOptimizer/lifetime_dependence/verify_diagnostics.swift

@@ -106,6 +106,11 @@ public struct NEInt: ~Escapable {
   init(owner: borrowing NCInt) {
     self.i = owner.i
   }
+
+  @_lifetime(immortal)
+  init(immortal i: Int) {
+    self.i = i
+  }
 }
 
 struct TestDeinitCallsAddressor: ~Copyable, ~Escapable {
@@ -228,3 +233,13 @@ class ClassStorage {
     _ = ne
   }
 }
+
+// =============================================================================
+// Immortal
+// =============================================================================
+
+@_lifetime(immortal)
+func testVoid() -> NEInt {
+  let ne = NEInt(immortal: 3)
+  return _overrideLifetime(ne, borrowing: ())
+}