Skip to content

Commit e6de45a

Browse files
authored
[tsan] Don't treat uncontended pthread_once as a potentially blocking region (#132477)
guard_acquire is a helper function used to implement TSan's __cxa_guard_acquire and pthread_once interceptors. https://reviews.llvm.org/D54664 introduced optional hooks to support cooperative multi-threading. It worked by marking the entire guard_acquire call as a potentially blocking region. In principle, only the contended case needs to be a potentially blocking region. This didn't matter for __cxa_guard_acquire because the compiler emits an inline fast path before calling __cxa_guard_acquire. That is, once we call __cxa_guard_acquire at all, we know we're in the contended case. https://reviews.llvm.org/D107359 then unified the __cxa_guard_acquire and pthread_once interceptors, adding the hooks to pthread_once. However, unlike __cxa_guard_acquire, pthread_once callers are not expected to have an inline fast path. The fast path is inside the function. As a result, TSan unnecessarily calls into the cooperative multi-threading engine on every pthread_once call, despite applications generally expecting pthread_once to be fast after initialization. Fix this by deferring the hooks to the contended case inside guard_acquire.
1 parent ef2735d commit e6de45a

File tree

2 files changed

+79
-11
lines changed

2 files changed

+79
-11
lines changed

compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -892,10 +892,9 @@ constexpr u32 kGuardWaiter = 1 << 17;
892892

893893
static int guard_acquire(ThreadState *thr, uptr pc, atomic_uint32_t *g,
894894
bool blocking_hooks = true) {
895-
if (blocking_hooks)
896-
OnPotentiallyBlockingRegionBegin();
897-
auto on_exit = at_scope_exit([blocking_hooks] {
898-
if (blocking_hooks)
895+
bool in_potentially_blocking_region = false;
896+
auto on_exit = at_scope_exit([&] {
897+
if (in_potentially_blocking_region)
899898
OnPotentiallyBlockingRegionEnd();
900899
});
901900

@@ -912,8 +911,13 @@ static int guard_acquire(ThreadState *thr, uptr pc, atomic_uint32_t *g,
912911
} else {
913912
if ((cmp & kGuardWaiter) ||
914913
atomic_compare_exchange_strong(g, &cmp, cmp | kGuardWaiter,
915-
memory_order_relaxed))
914+
memory_order_relaxed)) {
915+
if (blocking_hooks && !in_potentially_blocking_region) {
916+
in_potentially_blocking_region = true;
917+
OnPotentiallyBlockingRegionBegin();
918+
}
916919
FutexWait(g, cmp | kGuardWaiter);
920+
}
917921
}
918922
}
919923
}

compiler-rt/test/tsan/cxa_guard_acquire.cpp

Lines changed: 70 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,95 @@
11
// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
22

3+
#include "test.h"
4+
#include <assert.h>
35
#include <sanitizer/tsan_interface.h>
46
#include <stdio.h>
57

8+
// We only enter a potentially blocking region on thread contention. To reliably
9+
// trigger this, we force the initialization function to block until another
10+
// thread has entered the potentially blocking region.
11+
12+
static bool init_done = false;
13+
614
namespace __tsan {
715

816
#if (__APPLE__)
917
__attribute__((weak))
1018
#endif
1119
void OnPotentiallyBlockingRegionBegin() {
12-
printf("Enter __cxa_guard_acquire\n");
20+
assert(!init_done);
21+
printf("Enter potentially blocking region\n");
22+
// Signal the other thread to finish initialization.
23+
barrier_wait(&barrier);
1324
}
1425

1526
#if (__APPLE__)
1627
__attribute__((weak))
1728
#endif
18-
void OnPotentiallyBlockingRegionEnd() { printf("Exit __cxa_guard_acquire\n"); }
29+
void OnPotentiallyBlockingRegionEnd() {
30+
printf("Exit potentially blocking region\n");
31+
}
1932

2033
} // namespace __tsan
2134

35+
struct LazyInit {
36+
LazyInit() {
37+
assert(!init_done);
38+
printf("Enter constructor\n");
39+
// Wait for the other thread to get to the blocking region.
40+
barrier_wait(&barrier);
41+
printf("Exit constructor\n");
42+
}
43+
};
44+
45+
const LazyInit &get_lazy_init() {
46+
static const LazyInit lazy_init;
47+
return lazy_init;
48+
}
49+
50+
void *thread(void *arg) {
51+
get_lazy_init();
52+
return nullptr;
53+
}
54+
55+
struct LazyInit2 {
56+
LazyInit2() { printf("Enter constructor 2\n"); }
57+
};
58+
59+
const LazyInit2 &get_lazy_init2() {
60+
static const LazyInit2 lazy_init2;
61+
return lazy_init2;
62+
}
63+
2264
int main(int argc, char **argv) {
2365
// CHECK: Enter main
2466
printf("Enter main\n");
25-
// CHECK-NEXT: Enter __cxa_guard_acquire
26-
// CHECK-NEXT: Exit __cxa_guard_acquire
27-
static int s = argc;
28-
(void)s;
67+
68+
// If initialization is contended, the blocked thread should enter a
69+
// potentially blocking region.
70+
//
71+
// CHECK-NEXT: Enter constructor
72+
// CHECK-NEXT: Enter potentially blocking region
73+
// CHECK-NEXT: Exit constructor
74+
// CHECK-NEXT: Exit potentially blocking region
75+
barrier_init(&barrier, 2);
76+
pthread_t th1, th2;
77+
pthread_create(&th1, nullptr, thread, nullptr);
78+
pthread_create(&th2, nullptr, thread, nullptr);
79+
pthread_join(th1, nullptr);
80+
pthread_join(th2, nullptr);
81+
82+
// Now that the value has been initialized, subsequent calls should not enter
83+
// a potentially blocking region.
84+
init_done = true;
85+
get_lazy_init();
86+
87+
// If uncontended, there is no potentially blocking region.
88+
//
89+
// CHECK-NEXT: Enter constructor 2
90+
get_lazy_init2();
91+
get_lazy_init2();
92+
2993
// CHECK-NEXT: Exit main
3094
printf("Exit main\n");
3195
return 0;

0 commit comments

Comments
 (0)