From b7683a27cfaa5813cbe59bc1508ad11f668c004d Mon Sep 17 00:00:00 2001 From: Jeremy Schonfeld Date: Tue, 30 Sep 2025 12:58:32 -0700 Subject: [PATCH 1/4] (161666595) Data.mutableSpan and Data.mutableBytes do not perform uniqueness check --- Sources/FoundationEssentials/Data/Data.swift | 24 ++++++++-- .../FoundationEssentialsTests/DataTests.swift | 44 +++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/Sources/FoundationEssentials/Data/Data.swift b/Sources/FoundationEssentials/Data/Data.swift index 1f684186e..4de0fcf2e 100644 --- a/Sources/FoundationEssentials/Data/Data.swift +++ b/Sources/FoundationEssentials/Data/Data.swift @@ -2255,11 +2255,19 @@ public struct Data : Equatable, Hashable, RandomAccessCollection, MutableCollect start: UnsafeMutableRawPointer(Builtin.addressOfBorrow(self)), count: _representation.count ) - case .large(let slice): + case .large(var slice): + // Clear _representation during the unique check to avoid double counting the reference, and assign the mutated slice back to _representation afterwards + _representation = .empty + slice.ensureUniqueReference() + _representation = .large(slice) buffer = unsafe UnsafeMutableRawBufferPointer( start: slice.storage.mutableBytes?.advanced(by: slice.startIndex), count: slice.count ) - case .slice(let slice): + case .slice(var slice): + // Clear _representation during the unique check to avoid double counting the reference, and assign the mutated slice back to _representation afterwards + _representation = .empty + slice.ensureUniqueReference() + _representation = .slice(slice) buffer = unsafe UnsafeMutableRawBufferPointer( start: slice.storage.mutableBytes?.advanced(by: slice.startIndex), count: slice.count ) @@ -2288,11 +2296,19 @@ public struct Data : Equatable, Hashable, RandomAccessCollection, MutableCollect start: UnsafeMutableRawPointer(Builtin.addressOfBorrow(self)), count: _representation.count ) - case .large(let slice): + case .large(var slice): + // Clear _representation during the unique check to avoid double counting the reference, and assign the mutated slice back to _representation afterwards + _representation = .empty + slice.ensureUniqueReference() + _representation = .large(slice) buffer = unsafe UnsafeMutableRawBufferPointer( start: slice.storage.mutableBytes?.advanced(by: slice.startIndex), count: slice.count ) - case .slice(let slice): + case .slice(var slice): + // Clear _representation during the unique check to avoid double counting the reference, and assign the mutated slice back to _representation afterwards + _representation = .empty + slice.ensureUniqueReference() + _representation = .slice(slice) buffer = unsafe UnsafeMutableRawBufferPointer( start: slice.storage.mutableBytes?.advanced(by: slice.startIndex), count: slice.count ) diff --git a/Tests/FoundationEssentialsTests/DataTests.swift b/Tests/FoundationEssentialsTests/DataTests.swift index 64648c118..e97fcc819 100644 --- a/Tests/FoundationEssentialsTests/DataTests.swift +++ b/Tests/FoundationEssentialsTests/DataTests.swift @@ -1439,6 +1439,50 @@ private final class DataTests { #expect(data.count == 0) } } + + @Test func validateMutation_cow_mutableBytes() { + var data = Data(Array(repeating: 0, count: 32)) + holdReference(data) { + var bytes = data.mutableBytes + bytes.storeBytes(of: 1, toByteOffset: 0, as: UInt8.self) + + #expect(data[0] == 1) + #expect(heldData?[0] == 0) + } + + var data2 = Data(Array(repeating: 0, count: 32)) + // Escape the pointer to compare after a mutation without dereferencing the pointer + let originalPointer = data2.withUnsafeBytes { $0.baseAddress } + + var bytes = data2.mutableBytes + bytes.storeBytes(of: 1, toByteOffset: 0, as: UInt8.self) + #expect(data2[0] == 1) + data2.withUnsafeBytes { + #expect($0.baseAddress == originalPointer) + } + } + + @Test func validateMutation_cow_mutableSpan() { + var data = Data(Array(repeating: 0, count: 32)) + holdReference(data) { + var bytes = data.mutableSpan + bytes[0] = 1 + + #expect(data[0] == 1) + #expect(heldData?[0] == 0) + } + + var data2 = Data(Array(repeating: 0, count: 32)) + // Escape the pointer to compare after a mutation without dereferencing the pointer + let originalPointer = data2.withUnsafeBytes { $0.baseAddress } + + var bytes = data2.mutableSpan + bytes[0] = 1 + #expect(data2[0] == 1) + data2.withUnsafeBytes { + #expect($0.baseAddress == originalPointer) + } + } @Test func sliceHash() { let base1 = Data([0, 0xFF, 0xFF, 0]) From b545ffd17c48ba2c1ffedea4690b3f6b32e7b278 Mon Sep 17 00:00:00 2001 From: Jeremy Schonfeld Date: Wed, 1 Oct 2025 10:47:43 -0700 Subject: [PATCH 2/4] Address feedback --- .../FoundationEssentialsTests/DataTests.swift | 86 ++++++++++++------- 1 file changed, 55 insertions(+), 31 deletions(-) diff --git a/Tests/FoundationEssentialsTests/DataTests.swift b/Tests/FoundationEssentialsTests/DataTests.swift index e97fcc819..ff720c6c9 100644 --- a/Tests/FoundationEssentialsTests/DataTests.swift +++ b/Tests/FoundationEssentialsTests/DataTests.swift @@ -1441,7 +1441,7 @@ private final class DataTests { } @Test func validateMutation_cow_mutableBytes() { - var data = Data(Array(repeating: 0, count: 32)) + var data = Data(count: 32) holdReference(data) { var bytes = data.mutableBytes bytes.storeBytes(of: 1, toByteOffset: 0, as: UInt8.self) @@ -1450,7 +1450,7 @@ private final class DataTests { #expect(heldData?[0] == 0) } - var data2 = Data(Array(repeating: 0, count: 32)) + var data2 = Data(count: 32) // Escape the pointer to compare after a mutation without dereferencing the pointer let originalPointer = data2.withUnsafeBytes { $0.baseAddress } @@ -1463,7 +1463,7 @@ private final class DataTests { } @Test func validateMutation_cow_mutableSpan() { - var data = Data(Array(repeating: 0, count: 32)) + var data = Data(count: 32) holdReference(data) { var bytes = data.mutableSpan bytes[0] = 1 @@ -1472,7 +1472,7 @@ private final class DataTests { #expect(heldData?[0] == 0) } - var data2 = Data(Array(repeating: 0, count: 32)) + var data2 = Data(count: 32) // Escape the pointer to compare after a mutation without dereferencing the pointer let originalPointer = data2.withUnsafeBytes { $0.baseAddress } @@ -2429,17 +2429,16 @@ extension DataTests { // These tests require allocating an extremely large amount of data and are serialized to prevent the test runner from using all available memory at once @Suite("Large Data Tests", .serialized) struct LargeDataTests { - @Test - func largeSliceDataSpan() throws { #if _pointerBitWidth(_64) - let count = Int(Int32.max) + let largeCount = Int(Int32.max) #elseif _pointerBitWidth(_32) - let count = Int(Int16.max) + let largeCount = Int(Int16.max) #else #error("This test needs updating") #endif - - let source = Data(repeating: 0, count: count).dropFirst() + @Test + func largeSliceDataSpan() throws { + let source = Data(repeating: 0, count: largeCount).dropFirst() #expect(source.startIndex != 0) let span = source.span let isEmpty = span.isEmpty @@ -2448,20 +2447,12 @@ struct LargeDataTests { @Test func largeSliceDataMutableSpan() throws { -#if _pointerBitWidth(_64) - var count = Int(Int32.max) -#elseif _pointerBitWidth(_32) - var count = Int(Int16.max) -#else -#error("This test needs updating") -#endif - #if !canImport(Darwin) || FOUNDATION_FRAMEWORK - var source = Data(repeating: 0, count: count).dropFirst() + var source = Data(repeating: 0, count: largeCount).dropFirst() #expect(source.startIndex != 0) count = source.count var span = source.mutableSpan - #expect(span.count == count) + #expect(span.count == largeCount - 1) let i = try #require(span.indices.dropFirst().randomElement()) span[i] = .max #expect(source[i] == 0) @@ -2471,23 +2462,56 @@ struct LargeDataTests { @Test func largeSliceDataMutableRawSpan() throws { -#if _pointerBitWidth(_64) - var count = Int(Int32.max) -#elseif _pointerBitWidth(_32) - var count = Int(Int16.max) -#else -#error("This test needs updating") -#endif - - var source = Data(repeating: 0, count: count).dropFirst() + var source = Data(repeating: 0, count: largeCount).dropFirst() #expect(source.startIndex != 0) - count = source.count var span = source.mutableBytes let byteCount = span.byteCount - #expect(byteCount == count) + #expect(byteCount == largeCount - 1) let i = try #require(span.byteOffsets.dropFirst().randomElement()) span.storeBytes(of: -1, toByteOffset: i, as: Int8.self) #expect(source[i] == 0) #expect(source[i+1] == .max) } + + @Test func validateMutation_cow_largeMutableBytes() { + var data = Data(count: largeCount) + let heldData = data + var bytes = data.mutableBytes + bytes.storeBytes(of: 1, toByteOffset: 0, as: UInt8.self) + + #expect(data[0] == 1) + #expect(heldData[0] == 0) + + var data2 = Data(count: largeCount) + // Escape the pointer to compare after a mutation without dereferencing the pointer + let originalPointer = data2.withUnsafeBytes { $0.baseAddress } + + var bytes2 = data2.mutableBytes + bytes2.storeBytes(of: 1, toByteOffset: 0, as: UInt8.self) + #expect(data2[0] == 1) + data2.withUnsafeBytes { + #expect($0.baseAddress == originalPointer) + } + } + + @Test func validateMutation_cow_largeMutableSpan() { + var data = Data(count: largeCount) + let heldData = data + var bytes = data.mutableSpan + bytes[0] = 1 + + #expect(data[0] == 1) + #expect(heldData[0] == 0) + + var data2 = Data(count: largeCount) + // Escape the pointer to compare after a mutation without dereferencing the pointer + let originalPointer = data2.withUnsafeBytes { $0.baseAddress } + + var bytes2 = data2.mutableSpan + bytes2[0] = 1 + #expect(data2[0] == 1) + data2.withUnsafeBytes { + #expect($0.baseAddress == originalPointer) + } + } } From c1001577c17a6c9847cbc6f812918492e4e3d087 Mon Sep 17 00:00:00 2001 From: Jeremy Schonfeld Date: Wed, 1 Oct 2025 11:21:32 -0700 Subject: [PATCH 3/4] Fix Linux build failure --- Tests/FoundationEssentialsTests/DataTests.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/FoundationEssentialsTests/DataTests.swift b/Tests/FoundationEssentialsTests/DataTests.swift index ff720c6c9..3b7edfe57 100644 --- a/Tests/FoundationEssentialsTests/DataTests.swift +++ b/Tests/FoundationEssentialsTests/DataTests.swift @@ -2450,7 +2450,6 @@ struct LargeDataTests { #if !canImport(Darwin) || FOUNDATION_FRAMEWORK var source = Data(repeating: 0, count: largeCount).dropFirst() #expect(source.startIndex != 0) - count = source.count var span = source.mutableSpan #expect(span.count == largeCount - 1) let i = try #require(span.indices.dropFirst().randomElement()) From 1202f28cf209d5d9aa4e603c1de7e14a18ca0566 Mon Sep 17 00:00:00 2001 From: Jeremy Schonfeld Date: Wed, 1 Oct 2025 14:44:31 -0700 Subject: [PATCH 4/4] Fix test failure --- Tests/FoundationEssentialsTests/DataTests.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Tests/FoundationEssentialsTests/DataTests.swift b/Tests/FoundationEssentialsTests/DataTests.swift index 3b7edfe57..fefc0d27e 100644 --- a/Tests/FoundationEssentialsTests/DataTests.swift +++ b/Tests/FoundationEssentialsTests/DataTests.swift @@ -2473,6 +2473,8 @@ struct LargeDataTests { } @Test func validateMutation_cow_largeMutableBytes() { + // Avoid copying a large data on platforms with constrained memory limits + #if !canImport(Darwin) || os(macOS) var data = Data(count: largeCount) let heldData = data var bytes = data.mutableBytes @@ -2480,6 +2482,7 @@ struct LargeDataTests { #expect(data[0] == 1) #expect(heldData[0] == 0) + #endif var data2 = Data(count: largeCount) // Escape the pointer to compare after a mutation without dereferencing the pointer @@ -2494,6 +2497,8 @@ struct LargeDataTests { } @Test func validateMutation_cow_largeMutableSpan() { + // Avoid copying a large data on platforms with constrained memory limits + #if !canImport(Darwin) || os(macOS) var data = Data(count: largeCount) let heldData = data var bytes = data.mutableSpan @@ -2501,6 +2506,7 @@ struct LargeDataTests { #expect(data[0] == 1) #expect(heldData[0] == 0) + #endif var data2 = Data(count: largeCount) // Escape the pointer to compare after a mutation without dereferencing the pointer