diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h --- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h @@ -21,8 +21,8 @@ SANITIZER_INTERFACE_ATTRIBUTE uptr __sanitizer_get_estimated_allocated_size(uptr size); SANITIZER_INTERFACE_ATTRIBUTE int __sanitizer_get_ownership(const void *p); -SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE const void * -__sanitizer_get_allocated_begin(const void *p); +SANITIZER_INTERFACE_ATTRIBUTE const void *__sanitizer_get_allocated_begin( + const void *p); SANITIZER_INTERFACE_ATTRIBUTE uptr __sanitizer_get_allocated_size(const void *p); SANITIZER_INTERFACE_ATTRIBUTE uptr __sanitizer_get_current_allocated_bytes(); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h --- a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h @@ -12,16 +12,24 @@ // the lack of interface that would tell us about the Dynamic TLS (DTLS). // https://sourceware.org/bugzilla/show_bug.cgi?id=16291 // -// The matters get worse because the glibc implementation changed between -// 2.18 and 2.19: -// https://groups.google.com/forum/#!topic/address-sanitizer/BfwYD8HMxTM -// -// Before 2.19, every DTLS chunk is allocated with __libc_memalign, +// Before 2.25: every DTLS chunk is allocated with __libc_memalign, // which we intercept and thus know where is the DTLS. -// Since 2.19, DTLS chunks are allocated with __signal_safe_memalign, -// which is an internal function that wraps a mmap call, neither of which -// we can intercept. Luckily, __signal_safe_memalign has a simple parseable -// header which we can use. +// +// Since 2.25: DTLS chunks are allocated with malloc. We could co-opt +// the malloc interceptor to keep track of the last allocation, similar +// to how we handle __libc_memalign; however, this adds some overhead +// (since malloc, unlike __libc_memalign, is commonly called), and +// requires care to avoid false negatives for LeakSanitizer. +// Instead, we rely on our internal allocators - which keep track of all +// its allocations - to determine if an address points to a malloc +// allocation. +// +// There exists a since-deprecated version of Google's internal glibc fork +// that used __signal_safe_memalign. DTLS_on_tls_get_addr relied on a +// heuristic check (is the allocation 16 bytes from the start of a page +// boundary?), which was sometimes erroneous: +// https://bugs.chromium.org/p/chromium/issues/detail?id=1275223#c15 +// Since that check has no practical use anymore, we have removed it. // //===----------------------------------------------------------------------===// diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp --- a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp @@ -12,6 +12,7 @@ #include "sanitizer_tls_get_addr.h" +#include "sanitizer_allocator_interface.h" #include "sanitizer_atomic.h" #include "sanitizer_flags.h" #include "sanitizer_platform_interceptors.h" @@ -26,13 +27,6 @@ uptr offset; }; -// Glibc starting from 2.19 allocates tls using __signal_safe_memalign, -// which has such header. -struct Glibc_2_19_tls_header { - uptr size; - uptr start; -}; - // This must be static TLS __attribute__((tls_model("initial-exec"))) static __thread DTLS dtls; @@ -108,6 +102,14 @@ static const uptr kDtvOffset = 0; #endif +extern "C" { +SANITIZER_WEAK_ATTRIBUTE +uptr __sanitizer_get_allocated_size(const void *p); + +SANITIZER_WEAK_ATTRIBUTE +const void *__sanitizer_get_allocated_begin(const void *p); +} + DTLS::DTV *DTLS_on_tls_get_addr(void *arg_void, void *res, uptr static_tls_begin, uptr static_tls_end) { if (!common_flags()->intercept_tls_get_addr) return 0; @@ -125,19 +127,18 @@ atomic_load(&number_of_live_dtls, memory_order_relaxed)); if (dtls.last_memalign_ptr == tls_beg) { tls_size = dtls.last_memalign_size; - VReport(2, "__tls_get_addr: glibc <=2.18 suspected; tls={0x%zx,0x%zx}\n", + VReport(2, "__tls_get_addr: glibc <=2.24 suspected; tls={0x%zx,0x%zx}\n", tls_beg, tls_size); } else if (tls_beg >= static_tls_begin && tls_beg < static_tls_end) { // This is the static TLS block which was initialized / unpoisoned at thread // creation. VReport(2, "__tls_get_addr: static tls: 0x%zx\n", tls_beg); tls_size = 0; - } else if ((tls_beg % 4096) == sizeof(Glibc_2_19_tls_header)) { - // We may want to check gnu_get_libc_version(). - Glibc_2_19_tls_header *header = (Glibc_2_19_tls_header *)tls_beg - 1; - tls_size = header->size; - tls_beg = header->start; - VReport(2, "__tls_get_addr: glibc >=2.19 suspected; tls={0x%zx 0x%zx}\n", + } else if (const void *start = + __sanitizer_get_allocated_begin((void *)tls_beg)) { + tls_beg = (uptr)start; + tls_size = __sanitizer_get_allocated_size(start); + VReport(2, "__tls_get_addr: glibc >=2.25 suspected; tls={0x%zx,0x%zx}\n", tls_beg, tls_size); } else { VReport(2, "__tls_get_addr: Can't guess glibc version\n"); diff --git a/compiler-rt/test/msan/dtls_test.c b/compiler-rt/test/msan/dtls_test.c --- a/compiler-rt/test/msan/dtls_test.c +++ b/compiler-rt/test/msan/dtls_test.c @@ -12,10 +12,6 @@ // Reports use-of-uninitialized-value, not analyzed XFAIL: target={{.*netbsd.*}} - // This is known to be broken with glibc-2.27+ - // https://bugs.llvm.org/show_bug.cgi?id=37804 - XFAIL: glibc-2.27 - */ #ifndef BUILD_SO