diff --git a/ddprof-lib/src/main/cpp/common.h b/ddprof-lib/src/main/cpp/common.h index 943c92fb4..13b8ae9ca 100644 --- a/ddprof-lib/src/main/cpp/common.h +++ b/ddprof-lib/src/main/cpp/common.h @@ -4,6 +4,34 @@ #include #include +// Sanitizer detection: define ASAN_ENABLED / TSAN_ENABLED uniformly across +// toolchains. clang exposes sanitizers only via __has_feature(...), while gcc +// defines __SANITIZE_ADDRESS__ / __SANITIZE_THREAD__. Guarding code on the gcc +// macros alone silently drops every sanitizer annotation under clang (the CI +// sanitizer compiler), so always go through these macros instead. +#if defined(__has_feature) +# if __has_feature(address_sanitizer) +# ifndef ASAN_ENABLED +# define ASAN_ENABLED 1 +# endif +# endif +# if __has_feature(thread_sanitizer) +# ifndef TSAN_ENABLED +# define TSAN_ENABLED 1 +# endif +# endif +#endif +#ifdef __SANITIZE_ADDRESS__ +# ifndef ASAN_ENABLED +# define ASAN_ENABLED 1 +# endif +#endif +#ifdef __SANITIZE_THREAD__ +# ifndef TSAN_ENABLED +# define TSAN_ENABLED 1 +# endif +#endif + // Knuth's multiplicative constant (golden ratio * 2^64 for 64-bit) // Used for hash distribution in various components constexpr size_t KNUTH_MULTIPLICATIVE_CONSTANT = 0x9e3779b97f4a7c15ULL; diff --git a/ddprof-lib/src/main/cpp/gtest_crash_handler.h b/ddprof-lib/src/main/cpp/gtest_crash_handler.h index 7afa6c5bd..7bc15bcb1 100644 --- a/ddprof-lib/src/main/cpp/gtest_crash_handler.h +++ b/ddprof-lib/src/main/cpp/gtest_crash_handler.h @@ -13,6 +13,8 @@ #include #include +#include "common.h" // TSAN_ENABLED (toolchain-agnostic sanitizer detection) + // Platform detection for execinfo.h availability #if defined(__GLIBC__) || (defined(__APPLE__) && defined(__MACH__)) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) #define HAVE_EXECINFO_H 1 @@ -123,7 +125,7 @@ void specificCrashHandler(int sig, siginfo_t *info, void *context) { // and overriding them causes TSan to crash before it can write its report. template void installGtestCrashHandler() { -#if !defined(__SANITIZE_THREAD__) +#if !defined(TSAN_ENABLED) struct sigaction sa; sa.sa_flags = SA_SIGINFO; // Get detailed info, keep handler active sigemptyset(&sa.sa_mask); @@ -140,7 +142,7 @@ void installGtestCrashHandler() { // Restore default signal handlers. inline void restoreDefaultSignalHandlers() { -#if !defined(__SANITIZE_THREAD__) +#if !defined(TSAN_ENABLED) signal(SIGSEGV, SIG_DFL); signal(SIGBUS, SIG_DFL); signal(SIGABRT, SIG_DFL); diff --git a/ddprof-lib/src/main/cpp/linearAllocator.cpp b/ddprof-lib/src/main/cpp/linearAllocator.cpp index 4443ed44d..4aea288fe 100644 --- a/ddprof-lib/src/main/cpp/linearAllocator.cpp +++ b/ddprof-lib/src/main/cpp/linearAllocator.cpp @@ -20,23 +20,17 @@ #include "os.h" #include "common.h" #include - -// Enable ASan memory poisoning for better use-after-free detection -#ifdef __has_feature - #if __has_feature(address_sanitizer) - #define ASAN_ENABLED 1 - #endif -#endif - -#ifdef __SANITIZE_ADDRESS__ - #define ASAN_ENABLED 1 +#ifdef TSAN_ENABLED + #include + #include #endif +// ASAN_ENABLED / TSAN_ENABLED are defined in common.h (toolchain-agnostic). #ifdef ASAN_ENABLED #include #endif -#ifdef __SANITIZE_THREAD__ +#ifdef TSAN_ENABLED #include #endif @@ -55,13 +49,13 @@ void LinearAllocator::clear() { // never clears shadow memory on munmap. Add explicit acquire/release around // every plain prev-field read so the happens-before chain from freeChunk's // __tsan_release reaches any thread that later reuses the same VA. - #ifdef __SANITIZE_THREAD__ + #ifdef TSAN_ENABLED __tsan_acquire(_reserve); #endif if (_reserve->prev == _tail) { freeChunk(_reserve); // __tsan_release inside } - #ifdef __SANITIZE_THREAD__ + #ifdef TSAN_ENABLED else { __tsan_release(_reserve); // not freed here; release for future VA-reuse acquirers } @@ -92,14 +86,14 @@ void LinearAllocator::clear() { // chain covers the condition check, not just the assignment inside the body. { Chunk *current = _tail; - #ifdef __SANITIZE_THREAD__ + #ifdef TSAN_ENABLED __tsan_acquire(current); // before the first current->prev read (loop condition) #endif while (current->prev != NULL) { Chunk *next = current->prev; freeChunk(current); // __tsan_release(current) + safeFree inside current = next; - #ifdef __SANITIZE_THREAD__ + #ifdef TSAN_ENABLED __tsan_acquire(current); // before the next iteration's current->prev read #endif } @@ -123,13 +117,13 @@ ChunkList LinearAllocator::detachChunks() { // have been allocated by another thread via reserveChunk() → allocateChunk(), // which released ownership with __tsan_release after writing chunk->prev. if (_reserve != _tail) { - #ifdef __SANITIZE_THREAD__ + #ifdef TSAN_ENABLED __tsan_acquire(_reserve); #endif if (_reserve->prev == _tail) { result.head = _reserve; } - #ifdef __SANITIZE_THREAD__ + #ifdef TSAN_ENABLED __tsan_release(_reserve); #endif } @@ -163,11 +157,11 @@ void LinearAllocator::freeChunks(ChunkList& chunks) { // __tsan_release in allocateChunk() that published the initialized chunk. // Without this, TSan cannot connect the writer's (e.g. reserveChunk thread) // initialization of chunk->prev to this read, and reports a false data race. - #ifdef __SANITIZE_THREAD__ + #ifdef TSAN_ENABLED __tsan_acquire(current); #endif Chunk* prev = current->prev; - #ifdef __SANITIZE_THREAD__ + #ifdef TSAN_ENABLED __tsan_release(current); #endif OS::safeFree(current, chunks.chunk_size); @@ -220,26 +214,47 @@ Chunk *LinearAllocator::allocateChunk(Chunk *current) { Chunk *chunk = (Chunk *)OS::safeAlloc(_chunk_size); if (chunk != NULL) { // OS::safeAlloc uses a raw mmap syscall that bypasses ASan and TSan - // interceptors by design (to avoid profiling self-instrumentation). - // When the OS reuses a VA that had stale sanitizer state from a previous - // allocation at that address, writing to the chunk header triggers: - // ASan: use-after-poison (f7 shadow from prior ASAN_POISON_MEMORY_REGION) - // TSan: data-race (prior access history for the same VA not cleared by munmap) - // Fix: unpoison the entire chunk and acquire TSan ownership BEFORE the first - // write, establishing a clean sanitizer baseline for this logical allocation. + // interceptors by design (to avoid self-instrumentation in the profiler). + // When the OS reuses a VA from a prior munmap, TSan's shadow memory for + // that VA still holds stale access history from previous chunk users, + // causing false-positive data-race reports on user-data addresses. + // + // Fix: re-map the same VA through the libc mmap() wrapper. TSan intercepts + // mmap() and calls MemoryRangeImitateWrite, which sets all 4 shadow slots + // for the entire chunk range to the current thread's write — completely + // overwriting any stale entries. safeAlloc itself is unchanged. + // + // This MUST stay TSan-build-only: the libc mmap() wrapper and TSan's + // interceptor are not async-signal-safe, and allocateChunk() is reachable + // from a signal handler in the full profiler. TSAN_ENABLED is defined for + // any TSan-instrumented translation unit (it tracks the compiler's + // sanitizer detection in common.h, not the build target), but the build + // only applies -fsanitize=thread to the isolated gtest binaries — never to + // the shared library the JVM loads. Those gtest binaries never drive the + // allocator from a signal handler, so the non-async-signal-safe mmap() call + // here is safe in practice. #ifdef ASAN_ENABLED ASAN_UNPOISON_MEMORY_REGION(chunk, _chunk_size); #endif - #ifdef __SANITIZE_THREAD__ - __tsan_acquire(chunk); + #ifdef TSAN_ENABLED + void *remap = mmap(chunk, _chunk_size, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); + // MAP_FIXED unmaps before it maps, so a failure would leave a hole at + // `chunk` and the writes below would fault. Abort unconditionally rather + // than via assert(), which an NDEBUG build would strip — turning a clean, + // diagnosable failure into a stale-shadow or use-after-unmap crash. + if (remap != chunk) { + perror("TSan shadow re-map (mmap MAP_FIXED) failed"); + abort(); + } #endif chunk->prev = current; chunk->offs = sizeof(Chunk); - // Publish the initialized chunk: release TSan ownership so that any thread - // which later acquires this chunk (via __tsan_acquire) will see these writes. - #ifdef __SANITIZE_THREAD__ + // Publish the initialized chunk: any thread that later acquires this chunk + // (via __tsan_acquire in freeChunks/detachChunks) will see these writes. + #ifdef TSAN_ENABLED __tsan_release(chunk); #endif @@ -251,10 +266,10 @@ Chunk *LinearAllocator::allocateChunk(Chunk *current) { void LinearAllocator::freeChunk(Chunk *current) { // Release TSan ownership before munmap so the sanitizer knows this thread is - // done with the memory. The paired __tsan_acquire in allocateChunk() ensures - // the next thread to receive this VA (after OS VA reuse) starts with a clean - // happens-before baseline rather than seeing stale access history. - #ifdef __SANITIZE_THREAD__ + // done with the memory. The mmap(MAP_FIXED) re-map in allocateChunk() resets + // the shadow for whichever thread later reuses this VA (after OS VA reuse), so + // it starts from a clean baseline rather than seeing stale access history. + #ifdef TSAN_ENABLED __tsan_release(current); #endif OS::safeFree(current, _chunk_size); diff --git a/ddprof-lib/src/test/cpp/ddprof_ut.cpp b/ddprof-lib/src/test/cpp/ddprof_ut.cpp index 3a5db92e5..afdb990fe 100644 --- a/ddprof-lib/src/test/cpp/ddprof_ut.cpp +++ b/ddprof-lib/src/test/cpp/ddprof_ut.cpp @@ -375,7 +375,7 @@ static DdprofGlobalSetup ddprof_global_setup; // Without the fix tryEnterCriticalSection() returns false (exit 5). // fork() is unsupported under TSan: the child inherits shadow memory in an // inconsistent state and crashes before any TSan report can be written. - #if !defined(__SANITIZE_THREAD__) + #if !defined(TSAN_ENABLED) TEST(ProfiledThreadTeardown, CriticalSectionExitsEvenAfterTLSCleared) { pid_t pid = fork(); ASSERT_NE(-1, pid); @@ -413,7 +413,7 @@ static DdprofGlobalSetup ddprof_global_setup; ASSERT_TRUE(WIFEXITED(status)) << "child crashed (signal " << WTERMSIG(status) << ")"; ASSERT_EQ(0, WEXITSTATUS(status)) << "child exited with code " << WEXITSTATUS(status); } - #endif // !__SANITIZE_THREAD__ + #endif // !TSAN_ENABLED int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); diff --git a/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp b/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp index 6d81fb0a2..a8706967d 100644 --- a/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp +++ b/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp @@ -7,6 +7,7 @@ #include "callTraceStorage.h" #include "callTraceHashTable.h" #include "guards.h" +#include "common.h" // TSAN_ENABLED (toolchain-agnostic sanitizer detection) #include #include #include @@ -2034,7 +2035,7 @@ TEST_F(StressTestSuite, HashTableSpinWaitEdgeCasesTest) { // correctness across expansion boundaries independently of memory ordering; // it catches logic regressions in all build configurations. TEST_F(StressTestSuite, FindCallTraceAtomicReadRaceTest) { -#if !defined(__SANITIZE_THREAD__) && !(defined(__has_feature) && __has_feature(thread_sanitizer)) +#if !defined(TSAN_ENABLED) GTEST_SKIP() << "TSan-only race regression: re-run with -fsanitize=thread"; #endif