Skip to content

Commit c8b0423

Browse files
devincoughlindevincoughlin
committed
[TSan] Change TSan inout accesses to use __tsan_external_write callback (swiftlang#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 62974d8 commit c8b0423

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
@@ -80,31 +80,31 @@ testRace(name: "GlobalStructMutatingMethod",
8080
thread: { _ = globalForGlobalStructMutatingMethod.read() },
8181
thread: { globalForGlobalStructMutatingMethod.mutate() } )
8282
// CHECK-LABEL: Running GlobalStructMutatingMethod
83-
// CHECK: ThreadSanitizer: data race
83+
// CHECK: ThreadSanitizer: Swift access race
8484
// CHECK: Location is global
8585

8686
var globalForGlobalStructDifferentStoredPropertiesInout = UninstrumentedStruct()
8787
testRace(name: "GlobalStructDifferentStoredPropertiesInout",
8888
thread: { uninstrumentedTakesInout(&globalForGlobalStructDifferentStoredPropertiesInout.storedProperty1) },
8989
thread: { uninstrumentedTakesInout(&globalForGlobalStructDifferentStoredPropertiesInout.storedProperty2) } )
9090
// CHECK-LABEL: Running GlobalStructDifferentStoredPropertiesInout
91-
// CHECK: ThreadSanitizer: data race
91+
// CHECK: ThreadSanitizer: Swift access race
9292
// CHECK: Location is global
9393

9494
var globalForGlobalStructSameStoredPropertyInout = UninstrumentedStruct()
9595
testRace(name: "GlobalStructSameStoredPropertyInout",
9696
thread: { uninstrumentedTakesInout(&globalForGlobalStructSameStoredPropertyInout.storedProperty1) },
9797
thread: { uninstrumentedTakesInout(&globalForGlobalStructSameStoredPropertyInout.storedProperty1) } )
9898
// CHECK-LABEL: Running GlobalStructSameStoredPropertyInout
99-
// CHECK: ThreadSanitizer: data race
99+
// CHECK: ThreadSanitizer: Swift access race
100100

101101

102102
var globalForGlobalStructSubscriptDifferentIndexesInout = UninstrumentedStruct()
103103
testRace(name: "GlobalStructSubscriptDifferentIndexesInout",
104104
thread: { uninstrumentedTakesInout(&globalForGlobalStructSubscriptDifferentIndexesInout[0]) },
105105
thread: { uninstrumentedTakesInout(&globalForGlobalStructSubscriptDifferentIndexesInout[1]) } )
106106
// CHECK-LABEL: Running GlobalStructSubscriptDifferentIndexes
107-
// CHECK: ThreadSanitizer: data race
107+
// CHECK: ThreadSanitizer: Swift access race
108108
// CHECK: Location is global
109109

110110

@@ -113,45 +113,45 @@ testRace(name: "GlobalStructSubscriptDifferentIndexesGetSet",
113113
thread: { _ = globalForGlobalStructSubscriptDifferentIndexesGetSet[0] },
114114
thread: { globalForGlobalStructSubscriptDifferentIndexesGetSet[1] = 12 } )
115115
// CHECK-LABEL: Running GlobalStructSubscriptDifferentIndexesGetSet
116-
// CHECK: ThreadSanitizer: data race
116+
// CHECK: ThreadSanitizer: Swift access race
117117
// CHECK: Location is global
118118

119119
var globalForGlobalClassGeneralMethods = UninstrumentedClass()
120120
testRace(name: "GlobalClassGeneralMethods",
121121
thread: { _ = globalForGlobalClassGeneralMethods.read() },
122122
thread: { globalForGlobalClassGeneralMethods.mutate() } )
123123
// CHECK-LABEL: Running GlobalClassGeneralMethods
124-
// CHECK-NOT: ThreadSanitizer: data race
124+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
125125

126126
var globalForGlobalClassDifferentStoredPropertiesInout = UninstrumentedClass()
127127
testRace(name: "GlobalClassDifferentStoredPropertiesInout",
128128
thread: { uninstrumentedTakesInout(&globalForGlobalClassDifferentStoredPropertiesInout.storedProperty1) },
129129
thread: { uninstrumentedTakesInout(&globalForGlobalClassDifferentStoredPropertiesInout.storedProperty2) } )
130130
// CHECK-LABEL: Running GlobalClassDifferentStoredPropertiesInout
131-
// CHECK-NOT: ThreadSanitizer: data race
131+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
132132

133133
var globalForGlobalClassSubscriptDifferentIndexesInout = UninstrumentedClass()
134134
testRace(name: "GlobalClassSubscriptDifferentIndexesInout",
135135
thread: { uninstrumentedTakesInout(&globalForGlobalClassSubscriptDifferentIndexesInout[0]) },
136136
thread: { uninstrumentedTakesInout(&globalForGlobalClassSubscriptDifferentIndexesInout[1]) } )
137137
// CHECK-LABEL: Running GlobalClassSubscriptDifferentIndexesInout
138-
// CHECK-NOT: ThreadSanitizer: data race
138+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
139139

140140

141141
var globalForGlobalClassSameStoredPropertyInout = UninstrumentedClass()
142142
testRace(name: "GlobalClassSameStoredPropertyInout",
143143
thread: { uninstrumentedTakesInout(&globalForGlobalClassSameStoredPropertyInout.storedProperty1) },
144144
thread: { uninstrumentedTakesInout(&globalForGlobalClassSameStoredPropertyInout.storedProperty1) } )
145145
// CHECK-LABEL: Running GlobalClassSameStoredPropertyInout
146-
// CHECK: ThreadSanitizer: data race
146+
// CHECK: ThreadSanitizer: Swift access race
147147
// CHECK: Location is heap block
148148

149149
// These access a global declared in the TSanUninstrumented module
150150
testRace(name: "InoutAccessToStoredGlobalInUninstrumentedModule",
151151
thread: { uninstrumentedTakesInout(&storedGlobalInUninstrumentedModule1) },
152152
thread: { uninstrumentedTakesInout(&storedGlobalInUninstrumentedModule1) } )
153153
// CHECK-LABEL: Running InoutAccessToStoredGlobalInUninstrumentedModule
154-
// CHECK: ThreadSanitizer: data race
154+
// CHECK: ThreadSanitizer: Swift access race
155155
// CHECK: Location is global
156156

157157
// These access a global declared in the TSanUninstrumented module.
@@ -169,14 +169,14 @@ testRace(name: "InoutAccessToComputedGlobalInUninstrumentedModule",
169169
thread: { uninstrumentedTakesInout(&computedGlobalInUninstrumentedModule1) },
170170
thread: { uninstrumentedTakesInout(&computedGlobalInUninstrumentedModule1) } )
171171
// CHECK-LABEL: Running InoutAccessToComputedGlobalInUninstrumentedModule
172-
// CHECK-NOT: ThreadSanitizer: data race
172+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
173173

174174
// These access a computed global declared in the TSanUninstrumented module
175175
testRace(name: "ReadAndWriteToComputedGlobalInUninstrumentedModule",
176176
thread: { computedGlobalInUninstrumentedModule2 = 7 },
177177
thread: { _ = computedGlobalInUninstrumentedModule2 } )
178178
// CHECK-LABEL: Running ReadAndWriteToComputedGlobalInUninstrumentedModule
179-
// CHECK-NOT: ThreadSanitizer: data race
179+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
180180

181181

182182

@@ -187,7 +187,7 @@ testRace(name: "GlobalUninstrumentedClassStoredPropertyMutatingMethod",
187187
thread: { _ = globalForGlobalUninstrumentedClassStoredPropertyMutatingMethod.storedStructProperty.read() },
188188
thread: { globalForGlobalUninstrumentedClassStoredPropertyMutatingMethod.storedStructProperty.mutate() } )
189189
// CHECK-LABEL: Running GlobalUninstrumentedClassStoredPropertyMutatingMethod
190-
// CHECK-NOT: ThreadSanitizer: data race
190+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
191191

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

204204
// Note: TSan sees the race above because the inout instrumentation adds an
@@ -210,7 +210,7 @@ testRace(name: "GlobalUninstrumentedClassComputedPropertyInout",
210210
thread: { uninstrumentedTakesInout(&globalForGlobalUninstrumentedClassComputedPropertyInout.computedStructProperty.storedProperty1) },
211211
thread: { uninstrumentedTakesInout(&globalForGlobalUninstrumentedClassComputedPropertyInout.computedStructProperty.storedProperty1) } )
212212
// CHECK-LABEL: Running GlobalUninstrumentedClassComputedPropertyInout
213-
// CHECK-NO: ThreadSanitizer: data race
213+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
214214

215215
// In the above the write in instrumented code is to the value buffer allocated
216216
// at the call site so there is no data race if the getter and setters themselves
@@ -223,7 +223,7 @@ testRace(name: "GlobalInstrumentedClassStoredPropertyMutatingMethod",
223223
thread: { _ = globalForGlobalInstrumentedClassStoredPropertyMutatingMethod.storedStructProperty.read() },
224224
thread: { globalForGlobalInstrumentedClassStoredPropertyMutatingMethod.storedStructProperty.mutate() } )
225225
// CHECK-LABEL: Running GlobalInstrumentedClassStoredPropertyMutatingMethod
226-
// CHECK: ThreadSanitizer: data race
226+
// CHECK: ThreadSanitizer: Swift access race
227227
// CHECK: Location is heap block
228228
//
229229
// TSan does see this above race because the getter and materializeForSet is instrumented
@@ -254,7 +254,7 @@ func runCapturedLocalStructMutatingMethod() {
254254
thread: { l.mutate() } )
255255
}
256256
// CHECK-LABEL: Running CapturedLocalStructMutatingMethod
257-
// CHECK: ThreadSanitizer: data race
257+
// CHECK: ThreadSanitizer: Swift access race
258258
// CHECK: Location is heap block
259259

260260

@@ -265,7 +265,7 @@ func runCapturedLocalStructDifferentStoredPropertiesInout() {
265265
thread: { uninstrumentedTakesInout(&l.storedProperty2) } )
266266
}
267267
// CHECK-LABEL: Running CapturedLocalStructDifferentStoredPropertiesInout
268-
// CHECK: ThreadSanitizer: data race
268+
// CHECK: ThreadSanitizer: Swift access race
269269
// CHECK: Location is heap block
270270

271271

@@ -276,7 +276,7 @@ func runCapturedLocalClassGeneralMethods() {
276276
thread: { l.mutate() } )
277277
}
278278
// CHECK-LABEL: Running CapturedLocalClassGeneralMethods
279-
// CHECK-NOT: ThreadSanitizer: data race
279+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
280280

281281

282282
func runCapturedLocalDifferentStoredPropertiesInout() {
@@ -286,7 +286,7 @@ func runCapturedLocalDifferentStoredPropertiesInout() {
286286
thread: { uninstrumentedTakesInout(&l.storedProperty2) } )
287287
}
288288
// CHECK-LABEL: Running CapturedLocalClassDifferentStoredPropertiesInout
289-
// CHECK-NOT: ThreadSanitizer: data race
289+
// CHECK-NOT: ThreadSanitizer: {{.*}} race
290290

291291
func runCapturedLocalSameStoredPropertyInout() {
292292
let l = UninstrumentedClass()
@@ -295,7 +295,7 @@ func runCapturedLocalSameStoredPropertyInout() {
295295
thread: { uninstrumentedTakesInout(&l.storedProperty1) } )
296296
}
297297
// CHECK-LABEL: Running CapturedLocalClassSameStoredPropertyInout
298-
// CHECK: ThreadSanitizer: data race
298+
// CHECK: ThreadSanitizer: Swift access race
299299
// CHECK: Location is heap block
300300

301301
runLocalTests()

0 commit comments

Comments
 (0)