Skip to content

Commit 85bb66c

Browse files
committed
[Threading][TSan] Fix TSan errors from lazy init on Linux.
Move the TSan functionality from Concurrency into Threading. Use it in the Linux `ulock` implementation so that TSan knows about `ulock` and will tolerate the newer `swift_once` implementation that uses it. rdar://110665213
1 parent 4d99f01 commit 85bb66c

File tree

10 files changed

+231
-72
lines changed

10 files changed

+231
-72
lines changed

include/swift/Threading/Impl/Linux/ulock.h

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
#include <atomic>
3535
#include <cstdint>
3636

37+
#include "swift/Threading/TSan.h"
38+
3739
namespace swift {
3840
namespace threading_impl {
3941
namespace linux {
@@ -59,32 +61,39 @@ inline void ulock_lock(ulock_t *lock) {
5961
const ulock_t tid = ulock_get_tid();
6062
do {
6163
ulock_t zero = 0;
62-
if (ulock_fastpath(__atomic_compare_exchange_n(
63-
lock, &zero, tid, true, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED)))
64-
return;
65-
64+
if (ulock_fastpath(__atomic_compare_exchange_n(lock, &zero, tid,
65+
true, __ATOMIC_ACQUIRE,
66+
__ATOMIC_RELAXED)))
67+
break;
6668
} while (ulock_futex(lock, FUTEX_LOCK_PI) != 0);
69+
70+
tsan::acquire(lock);
6771
}
6872

6973
inline bool ulock_trylock(ulock_t *lock) {
7074
ulock_t zero = 0;
7175
if (ulock_fastpath(__atomic_compare_exchange_n(lock, &zero, ulock_get_tid(),
7276
true, __ATOMIC_ACQUIRE,
73-
__ATOMIC_RELAXED)))
77+
__ATOMIC_RELAXED))
78+
|| ulock_futex(lock, FUTEX_TRYLOCK_PI) == 0) {
79+
tsan::acquire(lock);
7480
return true;
81+
}
7582

76-
return ulock_futex(lock, FUTEX_TRYLOCK_PI) == 0;
83+
return false;
7784
}
7885

7986
inline void ulock_unlock(ulock_t *lock) {
8087
const ulock_t tid = ulock_get_tid();
8188
do {
8289
ulock_t expected = tid;
83-
if (ulock_fastpath(__atomic_compare_exchange_n(
84-
lock, &expected, 0, true, __ATOMIC_RELEASE, __ATOMIC_RELAXED)))
85-
return;
86-
90+
if (ulock_fastpath(__atomic_compare_exchange_n(lock, &expected, 0,
91+
true, __ATOMIC_RELEASE,
92+
__ATOMIC_RELAXED)))
93+
break;
8794
} while (ulock_futex(lock, FUTEX_UNLOCK_PI) != 0);
95+
96+
tsan::release(lock);
8897
}
8998

9099
} // namespace linux

include/swift/Threading/TSan.h

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
//===--- TSan.h - TSan support functions ---------------------- -*- C++ -*-===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
//
13+
// Helper functions for code that needs to integrate with the thread
14+
// sanitizer. In particular, TSan can't see inside the runtime libraries,
15+
// so we occasionally need to give it a hint that we're doing synchronization
16+
// in order to avoid false positives.
17+
//
18+
//===----------------------------------------------------------------------===//
19+
20+
#ifndef SWIFT_THREADING_TSAN_H
21+
#define SWIFT_THREADING_TSAN_H
22+
23+
namespace swift {
24+
25+
#if defined(_WIN32) || defined(__wasi__) || !__has_include(<dlfcn.h>)
26+
namespace tsan {
27+
28+
inline bool enabled() { return false; }
29+
template <typename T> T *acquire(T *ptr) { return ptr; }
30+
template <typename T> T *release(T *ptr) { return ptr; }
31+
template <typename T> T *consume(T *ptr) { return ptr; }
32+
33+
} // namespace tsan
34+
#else
35+
36+
namespace threading_impl {
37+
38+
extern bool tsan_enabled;
39+
extern void (*tsan_acquire)(const void *ptr);
40+
extern void (*tsan_release)(const void *ptr);
41+
42+
} // namespace threading_impl
43+
44+
namespace tsan {
45+
46+
/// Returns true if TSan is enabled
47+
inline bool enabled() {
48+
return threading_impl::tsan_enabled;
49+
}
50+
51+
/// Indicate to TSan that an acquiring load has occurred on the current
52+
/// thread. If some other thread does a releasing store with the same
53+
/// pointer, we are indicating to TSan that all writes that happened
54+
/// before that store will be visible to the current thread after the
55+
/// `acquire()`.
56+
template <typename T>
57+
T *acquire(T *ptr) {
58+
if (threading_impl::tsan_acquire) {
59+
threading_impl::tsan_acquire(ptr);
60+
}
61+
return ptr;
62+
}
63+
64+
/// Indicate to TSan that a releasing store has occurred on the current
65+
/// thread. If some other thread does an acquiring load with the same
66+
/// pointer, we are indicating to TSan that that thread will be able to
67+
/// see all writes that happened before the `release()`.
68+
template <typename T>
69+
T *release(T *ptr) {
70+
if (threading_impl::tsan_release) {
71+
threading_impl::tsan_release(ptr);
72+
}
73+
return ptr;
74+
}
75+
76+
/// Indicate to TSan that a consuming load has occurred on the current
77+
/// thread. If some other thread does a releasing store with the same
78+
/// pointer, we are indicating to TSan that all writes that happened
79+
/// before that store will be visible *to those operations that carry a
80+
/// dependency on the loaded value*.
81+
///
82+
/// TSan doesn't currently know about consume, so we lie and say it's an
83+
/// acquire instead.
84+
template <typename T>
85+
T *consume(T *ptr) {
86+
return acquire(ptr);
87+
}
88+
89+
} // namespace tsan
90+
91+
#endif
92+
93+
} // namespace swift
94+
95+
#endif

lib/Threading/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,5 @@ add_swift_host_library(swiftThreading STATIC
1010
Linux.cpp
1111
Pthreads.cpp
1212
Win32.cpp
13-
Errors.cpp)
13+
Errors.cpp
14+
TSan.cpp)

lib/Threading/Linux.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include "swift/Threading/Impl.h"
2020
#include "swift/Threading/Errors.h"
21+
#include "swift/Threading/TSan.h"
2122

2223
namespace {
2324

@@ -61,7 +62,7 @@ void swift::threading_impl::once_slow(once_t &predicate, void (*fn)(void *),
6162
#endif
6263
if (predicate.flag.load(std::memory_order_acquire) == 0) {
6364
fn(context);
64-
predicate.flag.store(-1, std::memory_order_release);
65+
predicate.flag.store(tsan::enabled() ? 1 : -1, std::memory_order_release);
6566
}
6667
#if defined(__LP64__) || defined(_LP64)
6768
linux::ulock_unlock(&predicate.lock);

lib/Threading/TSan.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//===--- TSan.h - TSan support functions ----------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
//
13+
// Helper functions for code that needs to integrate with the thread
14+
// sanitizer. In particular, TSan can't see inside the runtime libraries,
15+
// so we occasionally need to give it a hint that we're doing synchronization
16+
// in order to avoid false positives.
17+
//
18+
//===----------------------------------------------------------------------===//
19+
20+
#include "swift/Threading/TSan.h"
21+
#include "swift/shims/Visibility.h"
22+
23+
namespace swift {
24+
namespace threading_impl {
25+
26+
bool tsan_enabled = false;
27+
void (*tsan_acquire)(const void *) = nullptr;
28+
void (*tsan_release)(const void *) = nullptr;
29+
30+
#if __has_include(<dlfcn.h>)
31+
#include <dlfcn.h>
32+
33+
// The TSan library code will call this function when it starts up
34+
extern "C" SWIFT_ATTRIBUTE_FOR_EXPORTS
35+
void __tsan_on_initialize() {
36+
tsan_enabled = true;
37+
tsan_acquire = (void (*)(const void *))dlsym(RTLD_DEFAULT, "__tsan_acquire");
38+
tsan_release = (void (*)(const void *))dlsym(RTLD_DEFAULT, "__tsan_release");
39+
40+
// Always call through to the next image
41+
void (*next_init)(void);
42+
next_init = (void (*)(void))dlsym(RTLD_NEXT, "__tsan_on_initialize");
43+
if (next_init)
44+
next_init();
45+
}
46+
#endif
47+
48+
} // namespace threading_impl
49+
} // namespace swift

stdlib/public/Concurrency/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ add_swift_target_library(swift_Concurrency ${SWIFT_STDLIB_LIBRARY_BUILD_TYPES} I
120120
TaskLocal.cpp
121121
TaskLocal.swift
122122
TaskSleep.swift
123-
ThreadSanitizer.cpp
124123
ThreadingError.cpp
125124
TracingSignpost.cpp
126125
AsyncStreamBuffer.swift

stdlib/public/Concurrency/TaskPrivate.h

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "swift/Runtime/Exclusivity.h"
3030
#include "swift/Runtime/HeapObject.h"
3131
#include "swift/Threading/Thread.h"
32+
#include "swift/Threading/TSan.h"
3233
#include <atomic>
3334
#include <new>
3435

@@ -99,15 +100,18 @@ void _swift_taskGroup_cancelAllChildren(TaskGroup *group);
99100
/// should generally use a higher-level function.
100101
void _swift_taskGroup_detachChild(TaskGroup *group, AsyncTask *child);
101102

102-
/// release() establishes a happens-before relation with a preceding acquire()
103-
/// on the same address.
104-
void _swift_tsan_acquire(void *addr);
105-
void _swift_tsan_release(void *addr);
106-
/// Technically, this consume relies on implicit HW address dependency ordering
107-
/// and is paired with a corresponding release. Since TSAN doesn't know how to
108-
/// reason about this, we tell TSAN it's an acquire instead. See also
109-
/// SWIFT_MEMORY_ORDER_CONSUME definition.
110-
#define _swift_tsan_consume _swift_tsan_acquire
103+
/// Tell TSAN about an acquiring load
104+
inline void _swift_tsan_acquire(void *addr) {
105+
swift::tsan::acquire(addr);
106+
}
107+
/// Tell TSAN about a releasing store
108+
inline void _swift_tsan_release(void *addr) {
109+
swift::tsan::release(addr);
110+
}
111+
/// Tell TSAN about a consuming load
112+
inline void _swift_tsan_consume(void *addr) {
113+
swift::tsan::consume(addr);
114+
}
111115

112116
/// Special values used with DispatchQueueIndex to indicate the global and main
113117
/// executors.

stdlib/public/Concurrency/ThreadSanitizer.cpp

Lines changed: 0 additions & 50 deletions
This file was deleted.

stdlib/public/Threading/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ add_swift_target_library(swiftThreading OBJECT_LIBRARY
99
"${SWIFT_SOURCE_DIR}/lib/Threading/Linux.cpp"
1010
"${SWIFT_SOURCE_DIR}/lib/Threading/Pthreads.cpp"
1111
"${SWIFT_SOURCE_DIR}/lib/Threading/Win32.cpp"
12+
"${SWIFT_SOURCE_DIR}/lib/Threading/TSan.cpp"
1213
INSTALL_IN_COMPONENT never_install)

test/Sanitizers/tsan/once.swift

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// RUN: %target-swiftc_driver %s -Xfrontend -parse-as-library -g -sanitize=thread %import-libdispatch -o %t_tsan-binary
2+
// RUN: %target-codesign %t_tsan-binary
3+
// RUN: env %env-TSAN_OPTIONS=abort_on_error=0 %target-run %t_tsan-binary 2>&1 | %FileCheck %s --implicit-check-not='ThreadSanitizer'
4+
// REQUIRES: executable_test
5+
// REQUIRES: tsan_runtime
6+
7+
// rdar://101876380
8+
// UNSUPPORTED: OS=ios
9+
10+
// FIXME: This should be covered by "tsan_runtime"; older versions of Apple OSs
11+
// don't support TSan.
12+
// UNSUPPORTED: remote_run
13+
14+
// Test that we do not report a race on initialization; Swift doesn't initialize
15+
// globals at start-up, but rather uses swift_once(). This is thread safe, but
16+
// on some platforms TSan wasn't seeing the synchronization, so would report
17+
// a false positive.
18+
19+
import Dispatch
20+
21+
var count = 0
22+
let foo = {
23+
count += 1
24+
return count
25+
}()
26+
27+
@main
28+
struct Main {
29+
static func main() {
30+
let q = DispatchQueue(label: "q", attributes: .concurrent)
31+
let finished = DispatchSemaphore(value: 0)
32+
let count = 100
33+
34+
for _ in 0..<count {
35+
q.async {
36+
print(foo)
37+
finished.signal()
38+
}
39+
}
40+
41+
for _ in 0..<count {
42+
finished.wait()
43+
}
44+
45+
print("Done!")
46+
}
47+
}
48+
49+
// CHECK-NOT: ThreadSanitizer: data race
50+
// CHECK: Done!

0 commit comments

Comments
 (0)