From 98b8be29161c842345fe8ab1271d1833d07c2f79 Mon Sep 17 00:00:00 2001 From: Fei Peng Date: Thu, 17 Jul 2025 16:29:56 -0700 Subject: [PATCH 1/5] [TSan] Add support for Android --- .../lib/tsan/rtl/tsan_interceptors_posix.cpp | 8 ++- .../lib/tsan/rtl/tsan_platform_linux.cpp | 52 +++++++++++++++---- compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp | 6 ++- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp index b46a81031258c..28e35229bd5e4 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp @@ -2412,7 +2412,11 @@ TSAN_INTERCEPTOR(int, vfork, int fake) { } #endif -#if SANITIZER_LINUX +#if SANITIZER_LINUX && !SANITIZER_ANDROID +// Bionic's pthread_create internally calls clone. When the CLONE_THREAD flag is +// set, clone does not create a new process but a new thread. This is a +// workaround for Android. Disabling the interception of clone solves the +// problem in most scenarios. TSAN_INTERCEPTOR(int, clone, int (*fn)(void *), void *stack, int flags, void *arg, int *parent_tid, void *tls, pid_t *child_tid) { SCOPED_INTERCEPTOR_RAW(clone, fn, stack, flags, arg, parent_tid, tls, @@ -3135,7 +3139,7 @@ void InitializeInterceptors() { TSAN_INTERCEPT(fork); TSAN_INTERCEPT(vfork); -#if SANITIZER_LINUX +#if SANITIZER_LINUX && !SANITIZER_ANDROID TSAN_INTERCEPT(clone); #endif #if !SANITIZER_ANDROID diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp index 4b55aab49a2b9..65d6fcaaa2fa3 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp @@ -486,8 +486,20 @@ int ExtractRecvmsgFDs(void *msgp, int *fds, int nfd) { // Reverse operation of libc stack pointer mangling static uptr UnmangleLongJmpSp(uptr mangled_sp) { -#if defined(__x86_64__) -# if SANITIZER_LINUX +# if SANITIZER_ANDROID + if (longjmp_xor_key == 0) { + // bionic libc initialization process: __libc_init_globals -> + // __libc_init_vdso (calls strcmp) -> __libc_init_setjmp_cookie. strcmp is + // intercepted by TSan, so during TSan initialization the setjmp_cookie + // remains uninitialized. On Android, longjmp_xor_key must be set on first + // use. + InitializeLongjmpXorKey(); + CHECK_NE(longjmp_xor_key, 0); + } +# endif + +# if defined(__x86_64__) +# if SANITIZER_LINUX // Reverse of: // xor %fs:0x30, %rsi // rol $0x11, %rsi @@ -542,13 +554,13 @@ static uptr UnmangleLongJmpSp(uptr mangled_sp) { # else # define LONG_JMP_SP_ENV_SLOT 2 # endif -#elif SANITIZER_LINUX -# ifdef __aarch64__ -# define LONG_JMP_SP_ENV_SLOT 13 -# elif defined(__loongarch__) -# define LONG_JMP_SP_ENV_SLOT 1 -# elif defined(__mips64) -# define LONG_JMP_SP_ENV_SLOT 1 +# elif SANITIZER_LINUX && !SANITIZER_ANDROID +# ifdef __aarch64__ +# define LONG_JMP_SP_ENV_SLOT 13 +# elif defined(__loongarch__) +# define LONG_JMP_SP_ENV_SLOT 1 +# elif defined(__mips64) +# define LONG_JMP_SP_ENV_SLOT 1 # elif SANITIZER_RISCV64 # define LONG_JMP_SP_ENV_SLOT 13 # elif defined(__s390x__) @@ -556,7 +568,17 @@ static uptr UnmangleLongJmpSp(uptr mangled_sp) { # else # define LONG_JMP_SP_ENV_SLOT 6 # endif -#endif +# elif SANITIZER_ANDROID +# ifdef __aarch64__ +# define LONG_JMP_SP_ENV_SLOT 3 +# elif SANITIZER_RISCV64 +# define LONG_JMP_SP_ENV_SLOT 3 +# elif defined(__x86_64__) +# define LONG_JMP_SP_ENV_SLOT 6 +# else +# error unsupported +# endif +# endif uptr ExtractLongJmpSp(uptr *env) { uptr mangled_sp = env[LONG_JMP_SP_ENV_SLOT]; @@ -653,6 +675,16 @@ ThreadState *cur_thread() { } CHECK_EQ(0, internal_sigprocmask(SIG_SETMASK, &oldset, nullptr)); } + + // https://android.googlesource.com/platform/external/skia/+/refs/tags/android-15.0.0_r36/src/ports/SkMemory_malloc.cpp#105 + // https://android.googlesource.com/platform/external/scudo/+/refs/tags/android-15.0.0_r36/standalone/tsd_shared.h#198 + // Workaround to get the correct ThreadState pointer. Scudo allocator uses the + // last bit of TLS_SLOT_SANITIZER as a flag of disable memory initialization. + // getPlatformAllocatorTlsSlot should not use TLS_SLOT_SANITIZER slot. + uptr addr = reinterpret_cast(thr); + if (addr % 2 != 0) { + return reinterpret_cast(addr & ~1ULL); + } return thr; } diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp index b1464ccfddeb4..7226962f6fe90 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp @@ -206,10 +206,14 @@ void ThreadStart(ThreadState *thr, Tid tid, ThreadID os_id, } #endif -#if !SANITIZER_GO +#if !SANITIZER_GO && !SANITIZER_ANDROID // Don't imitate stack/TLS writes for the main thread, // because its initialization is synchronized with all // subsequent threads anyway. + // Because thr is created by MmapOrDie, the thr object + // is not in tls, the pointer of thr object is in + // TLS_SLOT_SANITIZER slot. So skip this check on + // Android platform. if (tid != kMainTid) { if (stk_addr && stk_size) { const uptr pc = StackTrace::GetNextInstructionPc( From e0e866643a60b5af4100e5c35bbf0f7294d46eb8 Mon Sep 17 00:00:00 2001 From: Fei Peng Date: Mon, 29 Sep 2025 12:52:46 -0700 Subject: [PATCH 2/5] Rearrange LONG_JMP_SP_ENV_SLOT defines --- .../lib/tsan/rtl/tsan_platform_linux.cpp | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp index 65d6fcaaa2fa3..7368af96ba7cd 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp @@ -554,7 +554,17 @@ static uptr UnmangleLongJmpSp(uptr mangled_sp) { # else # define LONG_JMP_SP_ENV_SLOT 2 # endif -# elif SANITIZER_LINUX && !SANITIZER_ANDROID +# elif SANITIZER_ANDROID +# ifdef __aarch64__ +# define LONG_JMP_SP_ENV_SLOT 3 +# elif SANITIZER_RISCV64 +# define LONG_JMP_SP_ENV_SLOT 3 +# elif defined(__x86_64__) +# define LONG_JMP_SP_ENV_SLOT 6 +# else +# error unsupported +# endif +# elif SANITIZER_LINUX # ifdef __aarch64__ # define LONG_JMP_SP_ENV_SLOT 13 # elif defined(__loongarch__) @@ -568,16 +578,6 @@ static uptr UnmangleLongJmpSp(uptr mangled_sp) { # else # define LONG_JMP_SP_ENV_SLOT 6 # endif -# elif SANITIZER_ANDROID -# ifdef __aarch64__ -# define LONG_JMP_SP_ENV_SLOT 3 -# elif SANITIZER_RISCV64 -# define LONG_JMP_SP_ENV_SLOT 3 -# elif defined(__x86_64__) -# define LONG_JMP_SP_ENV_SLOT 6 -# else -# error unsupported -# endif # endif uptr ExtractLongJmpSp(uptr *env) { From e316d29ad3b68d92f8aafb5cd84dbc9f2058efc4 Mon Sep 17 00:00:00 2001 From: Fei Peng Date: Mon, 29 Sep 2025 14:24:41 -0700 Subject: [PATCH 3/5] Fix issues raised in comments --- .../lib/tsan/rtl/tsan_platform_linux.cpp | 19 ++++++++++--------- compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp index 7368af96ba7cd..458799921013e 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp @@ -677,15 +677,16 @@ ThreadState *cur_thread() { } // https://android.googlesource.com/platform/external/skia/+/refs/tags/android-15.0.0_r36/src/ports/SkMemory_malloc.cpp#105 - // https://android.googlesource.com/platform/external/scudo/+/refs/tags/android-15.0.0_r36/standalone/tsd_shared.h#198 - // Workaround to get the correct ThreadState pointer. Scudo allocator uses the - // last bit of TLS_SLOT_SANITIZER as a flag of disable memory initialization. - // getPlatformAllocatorTlsSlot should not use TLS_SLOT_SANITIZER slot. - uptr addr = reinterpret_cast(thr); - if (addr % 2 != 0) { - return reinterpret_cast(addr & ~1ULL); - } - return thr; + // https://github.com/llvm/llvm-project/blob/release/21.x/compiler-rt/lib/scudo/standalone/wrappers_c.inc#L247 + // https://github.com/llvm/llvm-project/blob/release/21.x/compiler-rt/lib/scudo/standalone/tsd_shared.h#L106 + // https://github.com/llvm/llvm-project/blob/release/21.x/compiler-rt/lib/scudo/standalone/tsd_shared.h#L198 + // https://github.com/llvm/llvm-project/blob/release/21.x/compiler-rt/lib/scudo/standalone/tsd_shared.h#L154 + // https://android.googlesource.com/platform/bionic/+/refs/tags/android-15.0.0_r36/libc/platform/scudo_platform_tls_slot.h#34 + // Skia changes the malloc flags to M_THREAD_DISABLE_MEM_INIT, which sets the + // least significant bit of TLS_SLOT_SANITIZER to 1. Scudo allocator uses this + // bit as a flag to disable memory initialization. This is a workaround to get + // the correct ThreadState pointer. + reinterpret_cast(addr & ~1ULL); } void set_cur_thread(ThreadState *thr) { diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp index 7226962f6fe90..978d853b0bc7e 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp @@ -211,7 +211,7 @@ void ThreadStart(ThreadState *thr, Tid tid, ThreadID os_id, // because its initialization is synchronized with all // subsequent threads anyway. // Because thr is created by MmapOrDie, the thr object - // is not in tls, the pointer of thr object is in + // is not in tls, the pointer to the thr object is in // TLS_SLOT_SANITIZER slot. So skip this check on // Android platform. if (tid != kMainTid) { From 98929e05930e3bd5bd02f3e8f01b5c2259a8e42b Mon Sep 17 00:00:00 2001 From: Fei Peng Date: Mon, 29 Sep 2025 14:38:22 -0700 Subject: [PATCH 4/5] fix code format --- compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp index 458799921013e..4aa3ed9745382 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp @@ -686,7 +686,7 @@ ThreadState *cur_thread() { // least significant bit of TLS_SLOT_SANITIZER to 1. Scudo allocator uses this // bit as a flag to disable memory initialization. This is a workaround to get // the correct ThreadState pointer. - reinterpret_cast(addr & ~1ULL); + reinterpret_cast(addr & ~1ULL); } void set_cur_thread(ThreadState *thr) { From 0abdf7df43c7ba441fef5ea58274aa3e2f3678e9 Mon Sep 17 00:00:00 2001 From: Fei Peng Date: Tue, 30 Sep 2025 10:28:31 -0700 Subject: [PATCH 5/5] update comment --- compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp index 4aa3ed9745382..6b6538735beb0 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp @@ -676,16 +676,10 @@ ThreadState *cur_thread() { CHECK_EQ(0, internal_sigprocmask(SIG_SETMASK, &oldset, nullptr)); } - // https://android.googlesource.com/platform/external/skia/+/refs/tags/android-15.0.0_r36/src/ports/SkMemory_malloc.cpp#105 - // https://github.com/llvm/llvm-project/blob/release/21.x/compiler-rt/lib/scudo/standalone/wrappers_c.inc#L247 - // https://github.com/llvm/llvm-project/blob/release/21.x/compiler-rt/lib/scudo/standalone/tsd_shared.h#L106 - // https://github.com/llvm/llvm-project/blob/release/21.x/compiler-rt/lib/scudo/standalone/tsd_shared.h#L198 - // https://github.com/llvm/llvm-project/blob/release/21.x/compiler-rt/lib/scudo/standalone/tsd_shared.h#L154 - // https://android.googlesource.com/platform/bionic/+/refs/tags/android-15.0.0_r36/libc/platform/scudo_platform_tls_slot.h#34 - // Skia changes the malloc flags to M_THREAD_DISABLE_MEM_INIT, which sets the - // least significant bit of TLS_SLOT_SANITIZER to 1. Scudo allocator uses this - // bit as a flag to disable memory initialization. This is a workaround to get - // the correct ThreadState pointer. + // Skia calls mallopt(M_THREAD_DISABLE_MEM_INIT, 1), which sets the least + // significant bit of TLS_SLOT_SANITIZER to 1. Scudo allocator uses this bit + // as a flag to disable memory initialization. This is a workaround to get the + // correct ThreadState pointer. reinterpret_cast(addr & ~1ULL); }