-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[TSan] guard lock_during_write flag on Apple platforms changes to exclude Go #163204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TSan] guard lock_during_write flag on Apple platforms changes to exclude Go #163204
Conversation
There are currently build errors when checking the TSan Go runtime due to the implementation of this flag: ``` ../rtl/tsan_rtl.cpp:46:11: error: no member named 'cur_thread_init' in namespace '__tsan' 46 | __tsan::cur_thread_init()->in_internal_write_call = value; | ^~~~~~~~~~~~~~~ ../../sanitizer_common/sanitizer_mac.cpp:109:38: error: redefinition of '__tsan_set_in_internal_write_call' 109 | SANITIZER_WEAK_ATTRIBUTE extern void __tsan_set_in_internal_write_call( | ^ ../rtl/tsan_rtl.cpp:45:13: note: previous definition is here 45 | extern void __tsan_set_in_internal_write_call(bool value) { | ^ ``` This patch guards all changes related to the flag behind `!SANITIZER_GO` to avoid these errors occurring.
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Dan Blackwell (DanBlackwell) ChangesThere are currently build errors when checking the TSan Go runtime due to the implementation of this flag (as pointed out here):
This patch guards all changes related to the flag behind Full diff: https://github.com/llvm/llvm-project/pull/163204.diff 8 Files Affected:
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index 0040f79a0540b..b0a29db908639 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -105,9 +105,11 @@ extern "C" {
mach_msg_type_number_t *infoCnt);
}
+# if !SANITIZER_GO
// Weak symbol no-op when TSan is not linked
SANITIZER_WEAK_ATTRIBUTE extern void __tsan_set_in_internal_write_call(
bool value) {}
+# endif
namespace __sanitizer {
@@ -179,11 +181,15 @@ uptr internal_read(fd_t fd, void *buf, uptr count) {
}
uptr internal_write(fd_t fd, const void *buf, uptr count) {
+# if SANITIZER_GO
+ return write(fd, buf, count);
+# else
// We need to disable interceptors when writing in TSan
__tsan_set_in_internal_write_call(true);
uptr res = write(fd, buf, count);
__tsan_set_in_internal_write_call(false);
return res;
+# endif
}
uptr internal_stat(const char *path, void *buf) {
diff --git a/compiler-rt/lib/tsan/rtl/tsan_flags.cpp b/compiler-rt/lib/tsan/rtl/tsan_flags.cpp
index 50632d2016376..efaaef8b7ae98 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_flags.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_flags.cpp
@@ -20,7 +20,7 @@
#include "tsan_rtl.h"
#include "ubsan/ubsan_flags.h"
-#if SANITIZER_APPLE
+#if SANITIZER_APPLE && !SANITIZER_GO
namespace __sanitizer {
template <>
@@ -55,7 +55,7 @@ inline bool FlagHandler<LockDuringWriteSetting>::Format(char *buffer,
}
} // namespace __sanitizer
-#endif
+#endif // SANITIZER_APPLE && !SANITIZER_GO
namespace __tsan {
diff --git a/compiler-rt/lib/tsan/rtl/tsan_flags.h b/compiler-rt/lib/tsan/rtl/tsan_flags.h
index 477d08d334605..e63d7c405a6c1 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_flags.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_flags.h
@@ -16,7 +16,7 @@
#include "sanitizer_common/sanitizer_flags.h"
#include "sanitizer_common/sanitizer_deadlock_detector_interface.h"
-#if SANITIZER_APPLE
+#if SANITIZER_APPLE && !SANITIZER_GO
enum LockDuringWriteSetting {
kLockDuringAllWrites,
kNoLockDuringWritesCurrentProcess,
diff --git a/compiler-rt/lib/tsan/rtl/tsan_flags.inc b/compiler-rt/lib/tsan/rtl/tsan_flags.inc
index 64cc0919c0090..77ab910f08fbc 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_flags.inc
+++ b/compiler-rt/lib/tsan/rtl/tsan_flags.inc
@@ -81,7 +81,7 @@ TSAN_FLAG(bool, print_full_thread_history, false,
"If set, prints thread creation stacks for the threads involved in "
"the report and their ancestors up to the main thread.")
-#if SANITIZER_APPLE
+#if SANITIZER_APPLE && !SANITIZER_GO
TSAN_FLAG(LockDuringWriteSetting, lock_during_write, kLockDuringAllWrites,
"Determines whether to obtain a lock while writing logs or error "
"reports. "
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors.h b/compiler-rt/lib/tsan/rtl/tsan_interceptors.h
index d4b65ab1aaa6a..f8cc8ff3b406f 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors.h
@@ -1,7 +1,7 @@
#ifndef TSAN_INTERCEPTORS_H
#define TSAN_INTERCEPTORS_H
-#if SANITIZER_APPLE
+#if SANITIZER_APPLE && !SANITIZER_GO
# include "sanitizer_common/sanitizer_mac.h"
#endif
#include "sanitizer_common/sanitizer_stacktrace.h"
@@ -47,7 +47,7 @@ inline bool in_symbolizer() {
inline bool MustIgnoreInterceptor(ThreadState *thr) {
return !thr->is_inited || thr->ignore_interceptors || thr->in_ignored_lib
-#if SANITIZER_APPLE
+#if SANITIZER_APPLE && !SANITIZER_GO
|| (flags()->lock_during_write != kLockDuringAllWrites &&
thr->in_internal_write_call)
#endif
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 0c358042d1b56..714220a0109a8 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -31,7 +31,7 @@
#include "sanitizer_common/sanitizer_tls_get_addr.h"
#include "sanitizer_common/sanitizer_vector.h"
#include "tsan_fd.h"
-#if SANITIZER_APPLE
+#if SANITIZER_APPLE && !SANITIZER_GO
# include "tsan_flags.h"
#endif
#include "tsan_interceptors.h"
@@ -1668,7 +1668,7 @@ TSAN_INTERCEPTOR(int, pthread_barrier_wait, void *b) {
TSAN_INTERCEPTOR(int, pthread_once, void *o, void (*f)()) {
SCOPED_INTERCEPTOR_RAW(pthread_once, o, f);
-#if SANITIZER_APPLE
+#if SANITIZER_APPLE && !SANITIZER_GO
if (flags()->lock_during_write != kLockDuringAllWrites &&
cur_thread_init()->in_internal_write_call) {
// This is needed to make it through process launch without hanging
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
index b8041d724d342..feee566f44829 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
@@ -40,7 +40,7 @@ SANITIZER_WEAK_DEFAULT_IMPL
void __tsan_test_only_on_fork() {}
#endif
-#if SANITIZER_APPLE
+#if SANITIZER_APPLE && !SANITIZER_GO
// Override weak symbol from sanitizer_common
extern void __tsan_set_in_internal_write_call(bool value) {
__tsan::cur_thread_init()->in_internal_write_call = value;
@@ -901,7 +901,7 @@ void ForkChildAfter(ThreadState* thr, uptr pc, bool start_thread) {
ThreadIgnoreSyncBegin(thr, pc);
}
-# if SANITIZER_APPLE
+# if SANITIZER_APPLE && !SANITIZER_GO
// This flag can have inheritance disabled - we are the child so act
// accordingly
if (flags()->lock_during_write == kNoLockDuringWritesCurrentProcess)
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.h b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
index 77390f090f8af..635654616b781 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
@@ -236,7 +236,7 @@ struct alignas(SANITIZER_CACHE_LINE_SIZE) ThreadState {
const ReportDesc *current_report;
-#if SANITIZER_APPLE
+#if SANITIZER_APPLE && !SANITIZER_GO
bool in_internal_write_call;
#endif
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Confirmed that our builds are green now. Thanks! |
Great, thanks for the review @zmodem! |
…lude Go (llvm#163204) There are currently build errors when checking the TSan Go runtime due to the implementation of this flag (as pointed out [here](llvm#157928 (comment))): ``` ../rtl/tsan_rtl.cpp:46:11: error: no member named 'cur_thread_init' in namespace '__tsan' 46 | __tsan::cur_thread_init()->in_internal_write_call = value; | ^~~~~~~~~~~~~~~ ../../sanitizer_common/sanitizer_mac.cpp:109:38: error: redefinition of '__tsan_set_in_internal_write_call' 109 | SANITIZER_WEAK_ATTRIBUTE extern void __tsan_set_in_internal_write_call( | ^ ../rtl/tsan_rtl.cpp:45:13: note: previous definition is here 45 | extern void __tsan_set_in_internal_write_call(bool value) { | ^ ``` This patch guards all changes related to the flag behind `!SANITIZER_GO` to avoid these errors occurring.
There are currently build errors when checking the TSan Go runtime due to the implementation of this flag (as pointed out here):
This patch guards all changes related to the flag behind
!SANITIZER_GO
to avoid these errors occurring.