Insert end_cow_mutation_addr for lifetime dependent values dependent on modify accesses

meg-gupta · meg-gupta · commit c246b495452c · 2025-04-24T01:39:36.000-07:00
Array/ArraySlice/ContiguousArray have support for mutableSpan property.
These types are "optimized COW" types, we use compiler builtins begin_cow_mutation/end_cow_mutation to
optimize uniqueness checks. Since mutableSpan is a property and not a coroutine there is
no way to schedule the end_cow_mutaton operation at the end of the access.
This can lead to miscompiles in rare cases where we can end up using a stale storage buffer after a cow.

This PR inserts end_cow_mutation_addr to avoid this issue.

Note: We can end up with unnecessary end_cow_mutation_addr. But it is just a barrier to prevent invalid optimizations
and has no impact.
diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift
@@ -121,11 +121,25 @@ let lifetimeDependenceScopeFixupPass = FunctionPass(
     var scopeExtension = ScopeExtension(localReachabilityCache, context)
     let args = scopeExtension.extendScopes(dependence: newLifetimeDep)
 
+    if args.empty {
+      createEndCOWMutationIfNeeded(lifetimeDep: newLifetimeDep, context)
+    }
+
     // Redirect the dependence base to the function arguments. This may create additional mark_dependence instructions.
     markDep.redirectFunctionReturn(to: args, context)
   }
 }
 
+private func createEndCOWMutationIfNeeded(lifetimeDep: LifetimeDependence, _ context: FunctionPassContext) {
+  if let accessBase = lifetimeDep.parentValue as? BeginAccessInst,
+      accessBase.accessKind == .modify {
+    for endAccess in accessBase.endAccessInstructions {
+      let builder = Builder(before: endAccess, location: endAccess.location, context)
+      builder.createEndCOWMutationAddr(instance: accessBase)
+    }
+  }
+}
+
 private extension MarkDependenceInstruction {
   /// Rewrite the mark_dependence base operand to ignore inner borrow scopes (begin_borrow, load_borrow).
   ///
diff --git a/SwiftCompilerSources/Sources/SIL/Utilities/SequenceUtilities.swift b/SwiftCompilerSources/Sources/SIL/Utilities/SequenceUtilities.swift
@@ -133,6 +133,10 @@ public struct SingleInlineArray<Element>: RandomAccessCollection, FormattedLikeA
     singleElement == nil ? 0 : multipleElements.count + 1
   }
 
+  public var empty: Bool {
+    endIndex == 0
+  }
+
   public subscript(_ index: Int) -> Element {
     _read {
       if index == 0 {
diff --git a/stdlib/public/core/Array.swift b/stdlib/public/core/Array.swift
@@ -1736,9 +1736,7 @@ extension Array {
     @_alwaysEmitIntoClient
     mutating get {
       _makeMutableAndUnique()
-      // NOTE: We don't have the ability to schedule a call to
-      //       ContiguousArrayBuffer.endCOWMutation().
-      //       rdar://146785284 (lifetime analysis for end of mutation)
+      // LifetimeDependence analysis inserts call to Builtin.endCOWMutation.
       let pointer = unsafe _buffer.firstElementAddress
       let count = _buffer.mutableCount
       let span = unsafe MutableSpan(_unsafeStart: pointer, count: count)
diff --git a/test/SILOptimizer/lifetime_dependence/lifetime_dependence_cow.swift b/test/SILOptimizer/lifetime_dependence/lifetime_dependence_cow.swift
@@ -0,0 +1,238 @@
+// RUN:%target-swift-frontend %s -emit-sil \
+// RUN:   -verify \
+// RUN:   -sil-verify-all \
+// RUN:   -disable-availability-checking \
+// RUN:   -enable-experimental-feature LifetimeDependence | %FileCheck %s
+
+// REQUIRES: swift_in_compiler
+// REQUIRES: swift_feature_LifetimeDependence
+
+@inline(never)
+func use<T>(_ t: T) {
+
+}
+
+// CHECK-LABEL: sil hidden @$s23lifetime_dependence_cow11testNoWriteyySaySiGzF :
+// CHECK: [[BA:%.*]] = begin_access [modify] [static] %0
+// CHECK: [[FUN1:%.*]] = function_ref @$sSa11mutableSpans07MutableB0VyxGvg :
+// CHECK: [[APPLY1:%.*]] = apply [[FUN1]]<Int>([[BA]]) : $@convention(method) <τ_0_0> (@inout Array<τ_0_0>) -> @lifetime(borrow 0) @owned MutableSpan<τ_0_0>
+// CHECK: [[FUN2:%.*]] = function_ref @$s23lifetime_dependence_cow3useyyxlF : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+// CHECK: [[APPLY2:%.*]] = apply [[FUN2]]<Int>({{.*}}) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+// CHECK: end_cow_mutation_addr [[BA]]
+// CHECK: end_access [[BA]]
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow11testNoWriteyySaySiGzF'
+func testNoWrite(_ array: inout [Int]) {
+  let span = array.mutableSpan // begin_cow_mutation
+  use(span.count)
+}
+
+// Generic element version
+func testNoWrite<T>(_ array: inout [T]) {
+  let span = array.mutableSpan // begin_cow_mutation
+  use(span.count)
+}
+
+// CHECK-LABEL: sil hidden @$s23lifetime_dependence_cow9testWriteyySaySiGzF :
+// CHECK: [[BA:%.*]] = begin_access [modify] [static] %0
+// CHECK: [[FUN1:%.*]] = function_ref @$sSa11mutableSpans07MutableB0VyxGvg :
+// CHECK: [[APPLY1:%.*]] = apply [[FUN1]]<Int>([[BA]]) : $@convention(method) <τ_0_0> (@inout Array<τ_0_0>) -> @lifetime(borrow 0) @owned MutableSpan<τ_0_0>
+// CHECK: end_cow_mutation_addr [[BA]]
+// CHECK: end_access [[BA]]
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow9testWriteyySaySiGzF'
+func testWrite(_ array: inout [Int]) {
+  var span = array.mutableSpan // begin_cow_mutation
+  for i in span.indices {
+    span[i] = 0
+  }
+}
+
+// CHECK-LABEL: sil hidden @$s23lifetime_dependence_cow9testWriteyys10ArraySliceVySiGzF :
+// CHECK: [[BA:%.*]] = begin_access [modify] [static] %0
+// CHECK: [[FUN1:%.*]] = function_ref @$ss10ArraySliceV11mutableSpans07MutableD0VyxGvg : $@convention(method) <τ_0_0> (@inout ArraySlice<τ_0_0>) -> @lifetime(borrow 0) @owned MutableSpan<τ_0_0>
+// CHECK: [[APPLY1:%.*]] = apply [[FUN1]]<Int>(%3) : $@convention(method) <τ_0_0> (@inout ArraySlice<τ_0_0>) -> @lifetime(borrow 0) @owned MutableSpan<τ_0_0>
+// CHECK: end_cow_mutation_addr [[BA]]
+// CHECK: end_access [[BA]]
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow9testWriteyys10ArraySliceVySiGzF'
+func testWrite(_ array: inout ArraySlice<Int>) {
+  var span = array.mutableSpan
+  for i in span.indices {
+    span[i] = 0
+  }
+}
+
+// CHECK-LABEL: sil hidden @$s23lifetime_dependence_cow9testWriteyys15ContiguousArrayVySiGzF :
+// CHECK: [[BA:%.*]] = begin_access [modify] [static] %0
+// CHECK: [[FUN1:%.*]] = function_ref @$ss15ContiguousArrayV11mutableSpans07MutableD0VyxGvg : $@convention(method) <τ_0_0> (@inout ContiguousArray<τ_0_0>) -> @lifetime(borrow 0) @owned MutableSpan<τ_0_0>
+// CHECK: [[APPLY1:%.*]] = apply [[FUN1]]<Int>([[BA]]) : $@convention(method) <τ_0_0> (@inout ContiguousArray<τ_0_0>) -> @lifetime(borrow 0) @owned MutableSpan<τ_0_0>
+// CHECK: end_cow_mutation_addr [[BA]]
+// CHECK: end_access [[BA]]
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow9testWriteyys15ContiguousArrayVySiGzF'
+func testWrite(_ array: inout ContiguousArray<Int>) {
+  var span = array.mutableSpan
+  for i in span.indices {
+    span[i] = 0
+  }
+}
+
+// CHECK-LABEL: sil hidden @$s23lifetime_dependence_cow9testLocalyyF :
+// CHECK: [[BA:%.*]] = begin_access [modify] [static] %0
+// CHECK: [[FUN1:%.*]] = function_ref @$sSa11mutableSpans07MutableB0VyxGvg :
+// CHECK: [[APPLY1:%.*]] = apply [[FUN1]]<Int>([[BA]]) : $@convention(method) <τ_0_0> (@inout Array<τ_0_0>) -> @lifetime(borrow 0) @owned MutableSpan<τ_0_0>
+// CHECK: end_cow_mutation_addr [[BA]]
+// CHECK: end_access [[BA]]
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow9testLocalyyF'
+func testLocal() {
+  var array = [1, 2, 3]
+  var span = array.mutableSpan // begin_cow_mutation
+  for i in span.indices {
+    span[i] = 0
+  }
+}
+
+// CHECK-LABEL: sil hidden [noinline] @$s23lifetime_dependence_cow14getMutableSpanys0eF0VySiGSaySiGzF :
+// CHECK-NOT: end_cow_mutation
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow14getMutableSpanys0eF0VySiGSaySiGzF'
+@inline(never)
+func getMutableSpan(_ array: inout [Int]) -> MutableSpan<Int> {
+  return array.mutableSpan // begin_cow_mutation
+}
+
+// CHECK-LABEL: sil hidden @$s23lifetime_dependence_cow13modifyViaSpanyySaySiGzF :
+// CHECK: [[BA:%.*]] = begin_access [modify] [static] %0
+// CHECK: [[FUN:%.*]] = function_ref @$s23lifetime_dependence_cow14getMutableSpanys0eF0VySiGSaySiGzF : $@convention(thin) (@inout Array<Int>) -> @lifetime(borrow 0) @owned MutableSpan<Int>
+// CHECK: [[APPLY:%.*]] = apply [[FUN]]([[BA]]) : $@convention(thin) (@inout Array<Int>) -> @lifetime(borrow 0) @owned MutableSpan<Int>
+// CHECK: end_cow_mutation_addr [[BA]]
+// CHECK: end_access [[BA]]
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow13modifyViaSpanyySaySiGzF'
+func modifyViaSpan(_ array: inout [Int]) {
+  var span = getMutableSpan(&array)
+  for i in span.indices {
+    span[i] = 0
+  }
+}
+
+@inline(never)
+func modifyBlackhole(_ array: inout [Int]) {}
+
+// CHECK-LABEL: sil hidden @$s23lifetime_dependence_cow11modifyTwiceyySaySiGzF :
+// CHECK: [[BA:%.*]] = begin_access [modify] [static] %0
+// CHECK: [[FUN:%.*]] = function_ref @$s23lifetime_dependence_cow14getMutableSpanys0eF0VySiGSaySiGzF : $@convention(thin) (@inout Array<Int>) -> @lifetime(borrow 0) @owned MutableSpan<Int>
+// CHECK: [[APPLY:%.*]] = apply [[FUN]]([[BA]]) : $@convention(thin) (@inout Array<Int>) -> @lifetime(borrow 0) @owned MutableSpan<Int>
+// CHECK: end_cow_mutation_addr [[BA]]
+// CHECK: end_access [[BA]]
+// CHECK: [[FUN2:%.*]] = function_ref @$s23lifetime_dependence_cow15modifyBlackholeyySaySiGzF : $@convention(thin) (@inout Array<Int>) -> ()
+// CHECK: [[APPLY2:%.*]] = apply [[FUN2]]({{.*}}) : $@convention(thin) (@inout Array<Int>) -> ()
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow11modifyTwiceyySaySiGzF'
+func modifyTwice(_ array: inout [Int]) {
+  var span = getMutableSpan(&array)
+  for i in span.indices {
+    span[i] = 0
+  }
+  modifyBlackhole(&array)
+}
+
+// CHECK-LABEL: sil hidden [noinline] @$s23lifetime_dependence_cow7getSpanys0E0VySiGSaySiGzF :
+// CHECK-NOT: end_cow_mutation
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow7getSpanys0E0VySiGSaySiGzF'
+@inline(never)
+@lifetime(&array)
+func getSpan(_ array: inout [Int]) -> Span<Int> {
+  return array.span
+}
+
+// This test has an unnecessary end_cow_mutation
+
+// CHECK-LABEL: sil hidden @$s23lifetime_dependence_cow17inoutNoModifySpanyySaySiGzF : $@convention(thin) (@inout Array<Int>) -> () {
+// CHECK: end_cow_mutation
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow17inoutNoModifySpanyySaySiGzF'
+func inoutNoModifySpan(_ array: inout [Int]) {
+  let span = getSpan(&array)
+  for i in span.indices {
+    use(span[i])
+  }
+}
+
+struct Holder {
+  var array: [Int]
+}
+
+// CHECK-LABEL: sil hidden [noinline] @$s23lifetime_dependence_cow14getMutableSpanys0eF0VySiGAA6HolderVzF :
+// CHECK-NOT: end_cow_mutation
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow14getMutableSpanys0eF0VySiGAA6HolderVzF'
+@inline(never)
+func getMutableSpan(_ holder: inout Holder) -> MutableSpan<Int> {
+  return holder.array.mutableSpan // begin_cow_mutation
+}
+
+// CHECK-LABEL: sil hidden @$s23lifetime_dependence_cow14invisibleArrayyyAA6HolderVzF :
+// CHECK: [[BA:%.*]] = begin_access [modify] [static] %0
+// CHECK: [[FUNC:%.*]] = function_ref @$s23lifetime_dependence_cow14getMutableSpanys0eF0VySiGAA6HolderVzF : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned MutableSpan<Int>
+// CHECK: [[APPLY:%.*]] = apply [[FUNC]]([[BA]]) : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned MutableSpan<Int>
+// CHECK: end_cow_mutation_addr [[BA]]
+// CHECK: end_access [[BA]]
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow14invisibleArrayyyAA6HolderVzF'
+func invisibleArray(_ holder: inout Holder) {
+  var span = getMutableSpan(&holder)
+  for i in span.indices {
+    span[i] = 0
+  }
+}
+
+@inline(never)
+func mayGetMutableSpan(_ array: inout [Int]) -> MutableSpan<Int>? {
+  if Int.random(in: 1...10_000) % 2 == 0 {
+    return array.mutableSpan // begin_cow_mutation
+  }
+  return nil
+}
+
+// CHECK-LABEL: sil hidden @$s23lifetime_dependence_cow16mayModifyViaSpanyySaySiGzF :
+// CHECK: end_cow_mutation_addr
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow16mayModifyViaSpanyySaySiGzF'
+func mayModifyViaSpan(_ array: inout [Int]) {
+  guard var span = mayGetMutableSpan(&array) else {
+    return
+  }
+  for i in span.indices {
+    span[i] = 0
+  }
+}
+
+@inline(never)
+@lifetime(&array1, &array2)
+func getOneMutableSpan(_ array1: inout [Int], _ array2: inout [Int]) -> MutableSpan<Int> {
+  if Int.random(in: 1...10_000) % 2 == 0 {
+    return array1.mutableSpan
+  }
+  return array2.mutableSpan
+}
+
+// CHECK-LABEL: sil hidden @$s23lifetime_dependence_cow16modifyViaOneSpanyySaySiGz_ACztF :
+// CHECK: end_cow_mutation_addr
+// CHECK: end_cow_mutation_addr
+// CHECK-LABEL: } // end sil function '$s23lifetime_dependence_cow16modifyViaOneSpanyySaySiGz_ACztF'
+func modifyViaOneSpan(_ array1: inout [Int], _ array2: inout [Int]) {
+  var span = getOneMutableSpan(&array1, &array2)
+  for i in span.indices {
+    span[i] = 0
+  }
+}
+
+protocol P {
+  mutating func getMutableSpan() -> MutableSpan<Int>
+}
+
+struct C : P {
+  var array: [Int]
+  mutating func getMutableSpan() -> MutableSpan<Int> {
+    array.mutableSpan
+  }
+}
+
+func testWriteOpaque<T : P>(_ data: inout T) {
+  var span = data.getMutableSpan()
+  for i in span.indices {
+    span[i] = 0
+  }
+}
+
