Skip to content

Commit c971816

Browse files
devincoughlindevincoughlin
authored
[TSan] Change TSan inout accesses to use __tsan_external_write callback (#11032)
Adopt the __tsan_external_write compiler-rt callbacks when IRGen'ing the TSan inout access builtin. rdar://problem/32260994 rdar://problem/33461691
1 parent 43a428c commit c971816

File tree

4 files changed

+37
-26
lines changed

4 files changed

+37
-26
lines changed

‎include/swift/Runtime/RuntimeFunctions.def

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,11 +1210,11 @@ FUNCTION(GetErrorValue, swift_getErrorValue, C_CC,
12101210
ARGS(ErrorPtrTy, Int8PtrPtrTy, OpenedErrorTriplePtrTy),
12111211
ATTRS(NoUnwind))
12121212

1213-
// void __tsan_write1(void *addr);
1213+
// void __tsan_external_write(void *addr, void *caller_pc, void *tag);
12141214
// This is a Thread Sanitizer instrumentation entry point in compiler-rt.
1215-
FUNCTION(TSanInoutAccess, __tsan_write1, C_CC,
1215+
FUNCTION(TSanInoutAccess, __tsan_external_write, C_CC,
12161216
RETURNS(VoidTy),
1217-
ARGS(Int8PtrTy),
1217+
ARGS(Int8PtrTy, Int8PtrTy, Int8PtrTy),
12181218
ATTRS(NoUnwind))
12191219

12201220
FUNCTION(GetKeyPath, swift_getKeyPath, C_CC,

‎lib/IRGen/IRGenFunction.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,18 @@ void IRGenFunction::emitTSanInoutAccessCall(llvm::Value *address) {
250250
llvm::Function *fn = cast<llvm::Function>(IGM.getTSanInoutAccessFn());
251251

252252
llvm::Value *castAddress = Builder.CreateBitCast(address, IGM.Int8PtrTy);
253-
Builder.CreateCall(fn, {castAddress});
253+
254+
// Passing 0 as the caller PC causes compiler-rt to get our PC.
255+
llvm::Value *callerPC = llvm::ConstantPointerNull::get(IGM.Int8PtrTy);
256+
257+
// A magic number agreed upon with compiler-rt to indicate a modifying
258+
// access.
259+
const unsigned kExternalTagSwiftModifyingAccess = 0x1;
260+
llvm::Value *tagValue =
261+
llvm::ConstantInt::get(IGM.SizeTy, kExternalTagSwiftModifyingAccess);
262+
llvm::Value *castTag = Builder.CreateIntToPtr(tagValue, IGM.Int8PtrTy);
263+
264+
Builder.CreateCall(fn, {castAddress, callerPC, castTag});
254265
}
255266

256267

‎test/IRGen/tsan_instrumentation.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ sil @_T020tsan_instrumentation11inoutAccessyyF : $@convention(thin) () -> () {
2323
bb0:
2424
%0 = global_addr @_T020tsan_instrumentation1gSiv : $*Int
2525
%1 = builtin "tsanInoutAccess"(%0 : $*Int) : $()
26-
// CHECK: call void @__tsan_write1(i8* bitcast ([[GLOBAL]]* @_T020tsan_instrumentation1gSiv to i8*))
26+
// CHECK: call void @__tsan_external_write(i8* bitcast ([[GLOBAL]]* @_T020tsan_instrumentation1gSiv to i8*), i8* null, i8* inttoptr ({{(i32|i64)}} 1 to i8*))
2727

2828
%2 = tuple ()
2929
return %2 : $()

‎test/Sanitizers/tsan-inout.swift

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -76,31 +76,31 @@ testRace(name: "GlobalStructMutatingMethod",
7676
thread: { _ = globalForGlobalStructMutatingMethod.read() },
7777
thread: { globalForGlobalStructMutatingMethod.mutate() } )
7878
// CHECK-LABEL: Running GlobalStructMutatingMethod
79-
// CHECK: ThreadSanitizer: data race
79+
// CHECK: ThreadSanitizer: Swift access race
8080
// CHECK: Location is global
8181

8282
var globalForGlobalStructDifferentStoredPropertiesInout = UninstrumentedStruct()
8383
testRace(name: "GlobalStructDifferentStoredPropertiesInout",
8484
thread: { uninstrumentedTakesInout(&globalForGlobalStructDifferentStoredPropertiesInout.storedProperty1) },
8585
thread: { uninstrumentedTakesInout(&globalForGlobalStructDifferentStoredPropertiesInout.storedProperty2) } )
8686
// CHECK-LABEL: Running GlobalStructDifferentStoredPropertiesInout
87-
// CHECK: ThreadSanitizer: data race
87+
// CHECK: ThreadSanitizer: Swift access race
8888
// CHECK: Location is global
8989

9090
var globalForGlobalStructSameStoredPropertyInout = UninstrumentedStruct()
9191
testRace(name: "GlobalStructSameStoredPropertyInout",
9292
thread: { uninstrumentedTakesInout(&globalForGlobalStructSameStoredPropertyInout.storedProperty1) },
9393
thread: { uninstrumentedTakesInout(&globalForGlobalStructSameStoredPropertyInout.storedProperty1) } )
9494
// CHECK-LABEL: Running GlobalStructSameStoredPropertyInout
95-
// CHECK: ThreadSanitizer: data race
95+
// CHECK: ThreadSanitizer: Swift access race
9696

9797

9898
var globalForGlobalStructSubscriptDifferentIndexesInout = UninstrumentedStruct()
9999
testRace(name: "GlobalStructSubscriptDifferentIndexesInout",
100100
thread: { uninstrumentedTakesInout(&globalForGlobalStructSubscriptDifferentIndexesInout[0]) },
101101
thread: { uninstrumentedTakesInout(&globalForGlobalStructSubscriptDifferentIndexesInout[1]) } )
102102
// CHECK-LABEL: Running GlobalStructSubscriptDifferentIndexes
103-
// CHECK: ThreadSanitizer: data race
103+
// CHECK: ThreadSanitizer: Swift access race
104104
// CHECK: Location is global
105105

106106

@@ -109,45 +109,45 @@ testRace(name: "GlobalStructSubscriptDifferentIndexesGetSet",
109109
thread: { _ = globalForGlobalStructSubscriptDifferentIndexesGetSet[0] },
110110
thread: { globalForGlobalStructSubscriptDifferentIndexesGetSet[1] = 12 } )
111111
// CHECK-LABEL: Running GlobalStructSubscriptDifferentIndexesGetSet
112-
// CHECK: ThreadSanitizer: data race
112+
// CHECK: ThreadSanitizer: Swift access race
113113
// CHECK: Location is global
114114

115115
var globalForGlobalClassGeneralMethods = UninstrumentedClass()
116116
testRace(name: "GlobalClassGeneralMethods",
117117
thread: { _ = globalForGlobalClassGeneralMethods.read() },
118118
thread: { globalForGlobalClassGeneralMethods.mutate() } )
119119
// CHECK-LABEL: Running GlobalClassGeneralMethods
120-
// CHECK-NOT: ThreadSanitizer: data race
120+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
121121

122122
var globalForGlobalClassDifferentStoredPropertiesInout = UninstrumentedClass()
123123
testRace(name: "GlobalClassDifferentStoredPropertiesInout",
124124
thread: { uninstrumentedTakesInout(&globalForGlobalClassDifferentStoredPropertiesInout.storedProperty1) },
125125
thread: { uninstrumentedTakesInout(&globalForGlobalClassDifferentStoredPropertiesInout.storedProperty2) } )
126126
// CHECK-LABEL: Running GlobalClassDifferentStoredPropertiesInout
127-
// CHECK-NOT: ThreadSanitizer: data race
127+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
128128

129129
var globalForGlobalClassSubscriptDifferentIndexesInout = UninstrumentedClass()
130130
testRace(name: "GlobalClassSubscriptDifferentIndexesInout",
131131
thread: { uninstrumentedTakesInout(&globalForGlobalClassSubscriptDifferentIndexesInout[0]) },
132132
thread: { uninstrumentedTakesInout(&globalForGlobalClassSubscriptDifferentIndexesInout[1]) } )
133133
// CHECK-LABEL: Running GlobalClassSubscriptDifferentIndexesInout
134-
// CHECK-NOT: ThreadSanitizer: data race
134+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
135135

136136

137137
var globalForGlobalClassSameStoredPropertyInout = UninstrumentedClass()
138138
testRace(name: "GlobalClassSameStoredPropertyInout",
139139
thread: { uninstrumentedTakesInout(&globalForGlobalClassSameStoredPropertyInout.storedProperty1) },
140140
thread: { uninstrumentedTakesInout(&globalForGlobalClassSameStoredPropertyInout.storedProperty1) } )
141141
// CHECK-LABEL: Running GlobalClassSameStoredPropertyInout
142-
// CHECK: ThreadSanitizer: data race
142+
// CHECK: ThreadSanitizer: Swift access race
143143
// CHECK: Location is heap block
144144

145145
// These access a global declared in the TSanUninstrumented module
146146
testRace(name: "InoutAccessToStoredGlobalInUninstrumentedModule",
147147
thread: { uninstrumentedTakesInout(&storedGlobalInUninstrumentedModule1) },
148148
thread: { uninstrumentedTakesInout(&storedGlobalInUninstrumentedModule1) } )
149149
// CHECK-LABEL: Running InoutAccessToStoredGlobalInUninstrumentedModule
150-
// CHECK: ThreadSanitizer: data race
150+
// CHECK: ThreadSanitizer: Swift access race
151151
// CHECK: Location is global
152152

153153
// These access a global declared in the TSanUninstrumented module.
@@ -165,14 +165,14 @@ testRace(name: "InoutAccessToComputedGlobalInUninstrumentedModule",
165165
thread: { uninstrumentedTakesInout(&computedGlobalInUninstrumentedModule1) },
166166
thread: { uninstrumentedTakesInout(&computedGlobalInUninstrumentedModule1) } )
167167
// CHECK-LABEL: Running InoutAccessToComputedGlobalInUninstrumentedModule
168-
// CHECK-NOT: ThreadSanitizer: data race
168+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
169169

170170
// These access a computed global declared in the TSanUninstrumented module
171171
testRace(name: "ReadAndWriteToComputedGlobalInUninstrumentedModule",
172172
thread: { computedGlobalInUninstrumentedModule2 = 7 },
173173
thread: { _ = computedGlobalInUninstrumentedModule2 } )
174174
// CHECK-LABEL: Running ReadAndWriteToComputedGlobalInUninstrumentedModule
175-
// CHECK-NOT: ThreadSanitizer: data race
175+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
176176

177177

178178

@@ -183,7 +183,7 @@ testRace(name: "GlobalUninstrumentedClassStoredPropertyMutatingMethod",
183183
thread: { _ = globalForGlobalUninstrumentedClassStoredPropertyMutatingMethod.storedStructProperty.read() },
184184
thread: { globalForGlobalUninstrumentedClassStoredPropertyMutatingMethod.storedStructProperty.mutate() } )
185185
// CHECK-LABEL: Running GlobalUninstrumentedClassStoredPropertyMutatingMethod
186-
// CHECK-NOT: ThreadSanitizer: data race
186+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
187187

188188
// Note: TSan doesn't see a race above because it doesn't see any load on the
189189
// read side because the getter for the class property is not instrumented.
@@ -194,7 +194,7 @@ testRace(name: "GlobalUninstrumentedClassStoredPropertyInout",
194194
thread: { uninstrumentedTakesInout(&globalForGlobalUninstrumentedClassStoredPropertyInout.storedStructProperty.storedProperty1) },
195195
thread: { uninstrumentedTakesInout(&globalForGlobalUninstrumentedClassStoredPropertyInout.storedStructProperty.storedProperty2) } )
196196
// CHECK-LABEL: Running GlobalUninstrumentedClassStoredPropertyInout
197-
// CHECK: ThreadSanitizer: data race
197+
// CHECK: ThreadSanitizer: Swift access race
198198
// CHECK: Location is heap block
199199

200200
// Note: TSan sees the race above because the inout instrumentation adds an
@@ -206,7 +206,7 @@ testRace(name: "GlobalUninstrumentedClassComputedPropertyInout",
206206
thread: { uninstrumentedTakesInout(&globalForGlobalUninstrumentedClassComputedPropertyInout.computedStructProperty.storedProperty1) },
207207
thread: { uninstrumentedTakesInout(&globalForGlobalUninstrumentedClassComputedPropertyInout.computedStructProperty.storedProperty1) } )
208208
// CHECK-LABEL: Running GlobalUninstrumentedClassComputedPropertyInout
209-
// CHECK-NO: ThreadSanitizer: data race
209+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
210210

211211
// In the above the write in instrumented code is to the value buffer allocated
212212
// at the call site so there is no data race if the getter and setters themselves
@@ -219,7 +219,7 @@ testRace(name: "GlobalInstrumentedClassStoredPropertyMutatingMethod",
219219
thread: { _ = globalForGlobalInstrumentedClassStoredPropertyMutatingMethod.storedStructProperty.read() },
220220
thread: { globalForGlobalInstrumentedClassStoredPropertyMutatingMethod.storedStructProperty.mutate() } )
221221
// CHECK-LABEL: Running GlobalInstrumentedClassStoredPropertyMutatingMethod
222-
// CHECK: ThreadSanitizer: data race
222+
// CHECK: ThreadSanitizer: Swift access race
223223
// CHECK: Location is heap block
224224
//
225225
// TSan does see this above race because the getter and materializeForSet is instrumented
@@ -250,7 +250,7 @@ func runCapturedLocalStructMutatingMethod() {
250250
thread: { l.mutate() } )
251251
}
252252
// CHECK-LABEL: Running CapturedLocalStructMutatingMethod
253-
// CHECK: ThreadSanitizer: data race
253+
// CHECK: ThreadSanitizer: Swift access race
254254
// CHECK: Location is heap block
255255

256256

@@ -261,7 +261,7 @@ func runCapturedLocalStructDifferentStoredPropertiesInout() {
261261
thread: { uninstrumentedTakesInout(&l.storedProperty2) } )
262262
}
263263
// CHECK-LABEL: Running CapturedLocalStructDifferentStoredPropertiesInout
264-
// CHECK: ThreadSanitizer: data race
264+
// CHECK: ThreadSanitizer: Swift access race
265265
// CHECK: Location is heap block
266266

267267

@@ -272,7 +272,7 @@ func runCapturedLocalClassGeneralMethods() {
272272
thread: { l.mutate() } )
273273
}
274274
// CHECK-LABEL: Running CapturedLocalClassGeneralMethods
275-
// CHECK-NOT: ThreadSanitizer: data race
275+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
276276

277277

278278
func runCapturedLocalDifferentStoredPropertiesInout() {
@@ -282,7 +282,7 @@ func runCapturedLocalDifferentStoredPropertiesInout() {
282282
thread: { uninstrumentedTakesInout(&l.storedProperty2) } )
283283
}
284284
// CHECK-LABEL: Running CapturedLocalClassDifferentStoredPropertiesInout
285-
// CHECK-NOT: ThreadSanitizer: data race
285+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
286286

287287
func runCapturedLocalSameStoredPropertyInout() {
288288
let l = UninstrumentedClass()
@@ -291,7 +291,7 @@ func runCapturedLocalSameStoredPropertyInout() {
291291
thread: { uninstrumentedTakesInout(&l.storedProperty1) } )
292292
}
293293
// CHECK-LABEL: Running CapturedLocalClassSameStoredPropertyInout
294-
// CHECK: ThreadSanitizer: data race
294+
// CHECK: ThreadSanitizer: Swift access race
295295
// CHECK: Location is heap block
296296

297297
runLocalTests()

0 commit comments

Comments
 (0)