Skip to content

Commit 381dcc2

Browse files
committed
[TSan] Do not report benign race on _swiftEmptyArrayStorage
A previous commit [1] identified and fixed a benign race on `_swiftEmptyArrayStorage`. Unfortunately, we have some code duplication and the fix was not applied to all the necessary places. Essentially, we need to ensure that the "empty array storage" object that backs many "array like types" is never written to. I tried to improve the test to capture this, however, it is very finicky. Currently, it does not go red on my system even when I remove the check to avoid the benign race in Release mode. Relevant classes: Array, ArraySlice, ContiguousArray, ArrayBuffer, ContiguousArrayBuffer, SliceBuffer. [1] b9b4c78 rdar://55161564
1 parent f7b3e4b commit 381dcc2

File tree

5 files changed

+45
-20
lines changed

5 files changed

+45
-20
lines changed

stdlib/public/core/Array.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1162,7 +1162,7 @@ extension Array: RangeReplaceableCollection {
11621162
"newElements.underestimatedCount was an overestimate")
11631163
// can't check for overflow as sequences can underestimate
11641164

1165-
// This check prevents a data race writting to _swiftEmptyArrayStorage
1165+
// This check prevents a data race writing to _swiftEmptyArrayStorage
11661166
if writtenCount > 0 {
11671167
_buffer.count += writtenCount
11681168
}

stdlib/public/core/ArraySlice.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -953,7 +953,10 @@ extension ArraySlice: RangeReplaceableCollection {
953953
"newElements.underestimatedCount was an overestimate")
954954
// can't check for overflow as sequences can underestimate
955955

956-
_buffer.count += writtenCount
956+
// This check prevents a data race writing to _swiftEmptyArrayStorage
957+
if writtenCount > 0 {
958+
_buffer.count += writtenCount
959+
}
957960

958961
if writtenUpTo == buf.endIndex {
959962
// there may be elements that didn't fit in the existing buffer,

stdlib/public/core/ContiguousArray.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -801,7 +801,10 @@ extension ContiguousArray: RangeReplaceableCollection {
801801
"newElements.underestimatedCount was an overestimate")
802802
// can't check for overflow as sequences can underestimate
803803

804-
_buffer.count += writtenCount
804+
// This check prevents a data race writing to _swiftEmptyArrayStorage
805+
if writtenCount > 0 {
806+
_buffer.count += writtenCount
807+
}
805808

806809
if writtenUpTo == buf.endIndex {
807810
// there may be elements that didn't fit in the existing buffer,

stdlib/public/core/ContiguousArrayBuffer.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,7 @@ internal struct _UnsafePartiallyInitializedContiguousArrayBuffer<Element> {
688688
p = newResult.firstElementAddress + result.capacity
689689
remainingCapacity = newResult.capacity - result.capacity
690690
if !result.isEmpty {
691-
// This check prevents a data race writting to _swiftEmptyArrayStorage
691+
// This check prevents a data race writing to _swiftEmptyArrayStorage
692692
// Since count is always 0 there, this code does nothing anyway
693693
newResult.firstElementAddress.moveInitialize(
694694
from: result.firstElementAddress, count: result.capacity)
Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %target-swiftc_driver %s -target %sanitizers-target-triple -g -sanitize=thread -o %t_tsan-binary
22
// RUN: %target-codesign %t_tsan-binary
3-
// RUN: %target-run %t_tsan-binary 2>&1 | %FileCheck %s
3+
// RUN: %target-run %t_tsan-binary 2>&1 | %FileCheck %s --implicit-check-not='ThreadSanitizer'
44
// REQUIRES: executable_test
55
// REQUIRES: tsan_runtime
66
// REQUIRES: foundation
@@ -14,28 +14,47 @@ import Foundation
1414

1515
let sem = DispatchSemaphore(value: 0)
1616

17-
class T1: Thread {
17+
class T: Thread {
18+
let closure: () -> Void
19+
init(closure: @escaping () -> Void) {
20+
self.closure = closure
21+
}
1822
override func main() {
19-
var oneEmptyArray: [[String:String]] = []
20-
oneEmptyArray.append(contentsOf: [])
23+
closure()
2124
sem.signal()
2225
}
2326
}
24-
let t1 = T1()
25-
t1.start()
2627

27-
class T2: Thread {
28-
override func main() {
29-
var aCompletelyUnrelatedOtherEmptyArray: [[Double:Double]] = []
30-
aCompletelyUnrelatedOtherEmptyArray.append(contentsOf: [])
31-
sem.signal()
32-
}
28+
func runOnThread(closure: @escaping () -> Void) {
29+
let t = T(closure: closure)
30+
t.start()
31+
}
32+
33+
runOnThread(closure: {
34+
var oneEmptyArray: [[String:String]] = []
35+
oneEmptyArray.append(contentsOf: [])
36+
})
37+
runOnThread(closure: {
38+
var aCompletelyUnrelatedOtherEmptyArray: [[Double:Double]] = []
39+
aCompletelyUnrelatedOtherEmptyArray.append(contentsOf: [])
40+
})
41+
runOnThread(closure: {
42+
var array = Array<Int>()
43+
array.append(contentsOf: [])
44+
})
45+
runOnThread(closure: {
46+
var arraySlice = ArraySlice<Int>()
47+
arraySlice.append(contentsOf: [])
48+
})
49+
runOnThread(closure: {
50+
var contiguousArray = ContiguousArray<Int>()
51+
contiguousArray.append(contentsOf: [])
52+
})
53+
54+
for _ in 1...5 {
55+
sem.wait()
3356
}
34-
let t2 = T2()
35-
t2.start()
3657

37-
sem.wait()
38-
sem.wait()
3958
print("Done!")
4059

4160
// CHECK: Done!

0 commit comments

Comments
 (0)