Skip to content

Commit f680507

Browse files
ylnJulian Lettner
authored andcommitted
[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 (cherry picked from commit 381dcc2)
1 parent 4b8db65 commit f680507

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
@@ -690,7 +690,7 @@ internal struct _UnsafePartiallyInitializedContiguousArrayBuffer<Element> {
690690
p = newResult.firstElementAddress + result.capacity
691691
remainingCapacity = newResult.capacity - result.capacity
692692
if !result.isEmpty {
693-
// This check prevents a data race writting to _swiftEmptyArrayStorage
693+
// This check prevents a data race writing to _swiftEmptyArrayStorage
694694
// Since count is always 0 there, this code does nothing anyway
695695
newResult.firstElementAddress.moveInitialize(
696696
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
// UNSUPPORTED: OS=tvos
@@ -13,28 +13,47 @@ import Foundation
1313

1414
let sem = DispatchSemaphore(value: 0)
1515

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

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

36-
sem.wait()
37-
sem.wait()
3857
print("Done!")
3958

4059
// CHECK: Done!

0 commit comments

Comments
 (0)