Skip to content

Commit c4795a7

Browse files
rmacnak-googleCommit Queue
authored and
Commit Queue
committed
[vm, compiler] Don't do safepoint transitions in generated code under TSAN.
Go to the runtime so TSAN sees the instrumented access to Thread::safepoint_state_, avoiding false data race reports. TEST=tsan Bug: #52024 Change-Id: Ic686bac2221d8ac6b9865ce9c82a4c36711037a7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/295740 Reviewed-by: Alexander Aprelev <[email protected]> Commit-Queue: Ryan Macnak <[email protected]>
1 parent 39fc15a commit c4795a7

File tree

4 files changed

+26
-12
lines changed

4 files changed

+26
-12
lines changed

runtime/platform/thread_sanitizer.h

+2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
#define NO_SANITIZE_THREAD __attribute__((no_sanitize("thread")))
1818
extern "C" void __tsan_acquire(void* addr);
1919
extern "C" void __tsan_release(void* addr);
20+
constexpr bool kUsingThreadSanitizer = true;
2021
#else
2122
#define NO_SANITIZE_THREAD
23+
constexpr bool kUsingThreadSanitizer = false;
2224
#endif
2325

2426
#if defined(USING_THREAD_SANITIZER)

runtime/vm/compiler/assembler/assembler_arm64.cc

+8-4
Original file line numberDiff line numberDiff line change
@@ -1643,12 +1643,14 @@ void Assembler::LeaveDartFrame() {
16431643
void Assembler::EnterFullSafepoint(Register state) {
16441644
// We generate the same number of instructions whether or not the slow-path is
16451645
// forced. This simplifies GenerateJitCallbackTrampolines.
1646+
// For TSAN, we always go to the runtime so TSAN is aware of the release
1647+
// semantics of entering the safepoint.
16461648

16471649
Register addr = TMP2;
16481650
ASSERT(addr != state);
16491651

16501652
Label slow_path, done, retry;
1651-
if (FLAG_use_slow_path) {
1653+
if (FLAG_use_slow_path || kUsingThreadSanitizer) {
16521654
b(&slow_path);
16531655
}
16541656

@@ -1663,7 +1665,7 @@ void Assembler::EnterFullSafepoint(Register state) {
16631665
stxr(TMP, state, addr);
16641666
cbz(&done, TMP); // 0 means stxr was successful.
16651667

1666-
if (!FLAG_use_slow_path) {
1668+
if (!FLAG_use_slow_path && !kUsingThreadSanitizer) {
16671669
b(&retry);
16681670
}
16691671

@@ -1701,11 +1703,13 @@ void Assembler::ExitFullSafepoint(Register state,
17011703
bool ignore_unwind_in_progress) {
17021704
// We generate the same number of instructions whether or not the slow-path is
17031705
// forced, for consistency with EnterFullSafepoint.
1706+
// For TSAN, we always go to the runtime so TSAN is aware of the acquire
1707+
// semantics of leaving the safepoint.
17041708
Register addr = TMP2;
17051709
ASSERT(addr != state);
17061710

17071711
Label slow_path, done, retry;
1708-
if (FLAG_use_slow_path) {
1712+
if (FLAG_use_slow_path || kUsingThreadSanitizer) {
17091713
b(&slow_path);
17101714
}
17111715

@@ -1720,7 +1724,7 @@ void Assembler::ExitFullSafepoint(Register state,
17201724
stxr(TMP, state, addr);
17211725
cbz(&done, TMP); // 0 means stxr was successful.
17221726

1723-
if (!FLAG_use_slow_path) {
1727+
if (!FLAG_use_slow_path && !kUsingThreadSanitizer) {
17241728
b(&retry);
17251729
}
17261730

runtime/vm/compiler/assembler/assembler_riscv.cc

+8-4
Original file line numberDiff line numberDiff line change
@@ -3863,12 +3863,14 @@ void Assembler::TransitionNativeToGenerated(Register state,
38633863
void Assembler::EnterFullSafepoint(Register state) {
38643864
// We generate the same number of instructions whether or not the slow-path is
38653865
// forced. This simplifies GenerateJitCallbackTrampolines.
3866+
// For TSAN, we always go to the runtime so TSAN is aware of the release
3867+
// semantics of entering the safepoint.
38663868

38673869
Register addr = RA;
38683870
ASSERT(addr != state);
38693871

38703872
Label slow_path, done, retry;
3871-
if (FLAG_use_slow_path) {
3873+
if (FLAG_use_slow_path || kUsingThreadSanitizer) {
38723874
j(&slow_path, Assembler::kNearJump);
38733875
}
38743876

@@ -3882,7 +3884,7 @@ void Assembler::EnterFullSafepoint(Register state) {
38823884
sc(state, state, Address(addr, 0));
38833885
beqz(state, &done, Assembler::kNearJump); // 0 means sc was successful.
38843886

3885-
if (!FLAG_use_slow_path) {
3887+
if (!FLAG_use_slow_path && !kUsingThreadSanitizer) {
38863888
j(&retry, Assembler::kNearJump);
38873889
}
38883890

@@ -3898,11 +3900,13 @@ void Assembler::ExitFullSafepoint(Register state,
38983900
bool ignore_unwind_in_progress) {
38993901
// We generate the same number of instructions whether or not the slow-path is
39003902
// forced, for consistency with EnterFullSafepoint.
3903+
// For TSAN, we always go to the runtime so TSAN is aware of the acquire
3904+
// semantics of leaving the safepoint.
39013905
Register addr = RA;
39023906
ASSERT(addr != state);
39033907

39043908
Label slow_path, done, retry;
3905-
if (FLAG_use_slow_path) {
3909+
if (FLAG_use_slow_path || kUsingThreadSanitizer) {
39063910
j(&slow_path, Assembler::kNearJump);
39073911
}
39083912

@@ -3916,7 +3920,7 @@ void Assembler::ExitFullSafepoint(Register state,
39163920
sc(state, state, Address(addr, 0));
39173921
beqz(state, &done, Assembler::kNearJump); // 0 means sc was successful.
39183922

3919-
if (!FLAG_use_slow_path) {
3923+
if (!FLAG_use_slow_path && !kUsingThreadSanitizer) {
39203924
j(&retry, Assembler::kNearJump);
39213925
}
39223926

runtime/vm/compiler/assembler/assembler_x64.cc

+8-4
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,10 @@ void Assembler::setcc(Condition condition, ByteRegister dst) {
129129
void Assembler::EnterFullSafepoint() {
130130
// We generate the same number of instructions whether or not the slow-path is
131131
// forced, to simplify GenerateJitCallbackTrampolines.
132+
// For TSAN, we always go to the runtime so TSAN is aware of the release
133+
// semantics of entering the safepoint.
132134
Label done, slow_path;
133-
if (FLAG_use_slow_path) {
135+
if (FLAG_use_slow_path || kUsingThreadSanitizer) {
134136
jmp(&slow_path);
135137
}
136138

@@ -144,7 +146,7 @@ void Assembler::EnterFullSafepoint() {
144146
popq(RAX);
145147
cmpq(TMP, Immediate(target::Thread::full_safepoint_state_unacquired()));
146148

147-
if (!FLAG_use_slow_path) {
149+
if (!FLAG_use_slow_path && !kUsingThreadSanitizer) {
148150
j(EQUAL, &done);
149151
}
150152

@@ -184,8 +186,10 @@ void Assembler::TransitionGeneratedToNative(Register destination_address,
184186
void Assembler::ExitFullSafepoint(bool ignore_unwind_in_progress) {
185187
// We generate the same number of instructions whether or not the slow-path is
186188
// forced, for consistency with EnterFullSafepoint.
189+
// For TSAN, we always go to the runtime so TSAN is aware of the acquire
190+
// semantics of leaving the safepoint.
187191
Label done, slow_path;
188-
if (FLAG_use_slow_path) {
192+
if (FLAG_use_slow_path || kUsingThreadSanitizer) {
189193
jmp(&slow_path);
190194
}
191195

@@ -201,7 +205,7 @@ void Assembler::ExitFullSafepoint(bool ignore_unwind_in_progress) {
201205
popq(RAX);
202206
cmpq(TMP, Immediate(target::Thread::full_safepoint_state_acquired()));
203207

204-
if (!FLAG_use_slow_path) {
208+
if (!FLAG_use_slow_path && !kUsingThreadSanitizer) {
205209
j(EQUAL, &done);
206210
}
207211

0 commit comments

Comments
 (0)