[TSan] Fix determining static TLS blocks#183106
Conversation
Running gcc test c-c++-common/tsan/tls_race.c on s390 we get:
ThreadSanitizer: CHECK failed: tsan_platform_linux.cpp:618 "((thr_beg)) >= ((tls_addr))" (0x3ffaa35e140, 0x3ffaa35e250) (tid=2419930)
#0 __tsan::CheckUnwind() /devel/src/libsanitizer/tsan/tsan_rtl.cpp:696 (libtsan.so.2+0x91b57)
llvm#1 __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /devel/src/libsanitizer/sanitizer_common/sanitizer_termination.cpp:86 (libtsan.so.2+0xd211b)
llvm#2 __tsan::ImitateTlsWrite(__tsan::ThreadState*, unsigned long, unsigned long) /devel/src/libsanitizer/tsan/tsan_platform_linux.cpp:618 (libtsan.so.2+0x8faa3)
llvm#3 __tsan::ThreadStart(__tsan::ThreadState*, unsigned int, unsigned long long, __sanitizer::ThreadType) /devel/src/libsanitizer/tsan/tsan_rtl_thread.cpp:225 (libtsan.so.2+0xaadb5)
llvm#4 __tsan_thread_start_func /devel/src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1065 (libtsan.so.2+0x3d34d)
llvm#5 start_thread <null> (libc.so.6+0xae70d) (BuildId: d3b08de1b543c2d15d419bf861b3c2e4c01ac75b)
llvm#6 thread_start <null> (libc.so.6+0x12d2ff) (BuildId: d3b08de1b543c2d15d419bf861b3c2e4c01ac75b)
In order to determine the static TLS blocks in GetStaticTlsBoundary we
iterate over the modules and try to find the largest range without a
gap. Here we might have that modules are spaced exactly by the
alignment. For example, for the failing test we have:
(gdb) p/x ranges.data_[0]
$1 = {begin = 0x3fff7f9e6b8, end = 0x3fff7f9e740, align = 0x8, tls_modid = 0x3}
(gdb) p/x ranges.data_[1]
$2 = {begin = 0x3fff7f9e740, end = 0x3fff7f9eed0, align = 0x40, tls_modid = 0x2}
(gdb) p/x ranges.data_[2]
$3 = {begin = 0x3fff7f9eed8, end = 0x3fff7f9eef8, align = 0x8, tls_modid = 0x4}
(gdb) p/x ranges.data_[3]
$4 = {begin = 0x3fff7f9eefc, end = 0x3fff7f9ef00, align = 0x4, tls_modid = 0x1}
where ranges[3].begin == ranges[2].end + ranges[3].align holds. Since
in the loop a strict inequality test is used we compute the wrong address
(gdb) p/x *addr
$5 = 0x3fff7f9eefc
whereas 0x3fff7f9e6b8 is expected which is why we bail out in the
subsequent.
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Stefan Schulze Frielinghaus (stefan-sf-ibm) ChangesRunning gcc test c-c++-common/tsan/tls_race.c on s390 we get: ThreadSanitizer: CHECK failed: tsan_platform_linux.cpp:618 "((thr_beg)) >= ((tls_addr))" (0x3ffaa35e140, 0x3ffaa35e250) (tid=2419930) In order to determine the static TLS blocks in GetStaticTlsBoundary we iterate over the modules and try to find the largest range without a gap. Here we might have that modules are spaced exactly by the alignment. For example, for the failing test we have: (gdb) p/x ranges.data_[0] where ranges[3].begin == ranges[2].end + ranges[3].align holds. Since in the loop a strict inequality test is used we compute the wrong address (gdb) p/x *addr whereas 0x3fff7f9e6b8 is expected which is why we bail out in the subsequent. Full diff: https://github.com/llvm/llvm-project/pull/183106.diff 1 Files Affected:
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
index 530ff90c4cd16..2d398476738e0 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
@@ -503,10 +503,10 @@ __attribute__((unused)) static void GetStaticTlsBoundary(uptr *addr, uptr *size,
// loader places static TLS blocks this way not to waste space.
uptr l = one;
*align = ranges[l].align;
- while (l != 0 && ranges[l].begin < ranges[l - 1].end + ranges[l].align)
+ while (l != 0 && ranges[l].begin <= ranges[l - 1].end + ranges[l].align)
*align = Max(*align, ranges[--l].align);
uptr r = one + 1;
- while (r != len && ranges[r].begin < ranges[r - 1].end + ranges[r].align)
+ while (r != len && ranges[r].begin <= ranges[r - 1].end + ranges[r].align)
*align = Max(*align, ranges[r++].align);
*addr = ranges[l].begin;
*size = ranges[r - 1].end - ranges[l].begin;
|
|
/cc @vitalybuka could you have a look at this? |
3 participants
Running gcc test c-c++-common/tsan/tls_race.c on s390 we get:
ThreadSanitizer: CHECK failed: tsan_platform_linux.cpp:618 "((thr_beg)) >= ((tls_addr))" (0x3ffaa35e140, 0x3ffaa35e250) (tid=2419930)
#0 __tsan::CheckUnwind() /devel/src/libsanitizer/tsan/tsan_rtl.cpp:696 (libtsan.so.2+0x91b57)
#1 __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /devel/src/libsanitizer/sanitizer_common/sanitizer_termination.cpp:86 (libtsan.so.2+0xd211b)
#2 __tsan::ImitateTlsWrite(__tsan::ThreadState*, unsigned long, unsigned long) /devel/src/libsanitizer/tsan/tsan_platform_linux.cpp:618 (libtsan.so.2+0x8faa3)
#3 __tsan::ThreadStart(__tsan::ThreadState*, unsigned int, unsigned long long, __sanitizer::ThreadType) /devel/src/libsanitizer/tsan/tsan_rtl_thread.cpp:225 (libtsan.so.2+0xaadb5)
#4 __tsan_thread_start_func /devel/src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1065 (libtsan.so.2+0x3d34d)
#5 start_thread (libc.so.6+0xae70d) (BuildId: d3b08de1b543c2d15d419bf861b3c2e4c01ac75b)
#6 thread_start (libc.so.6+0x12d2ff) (BuildId: d3b08de1b543c2d15d419bf861b3c2e4c01ac75b)
In order to determine the static TLS blocks in GetStaticTlsBoundary we iterate over the modules and try to find the largest range without a gap. Here we might have that modules are spaced exactly by the alignment. For example, for the failing test we have:
(gdb) p/x ranges.data_[0]
$1 = {begin = 0x3fff7f9e6b8, end = 0x3fff7f9e740, align = 0x8, tls_modid = 0x3} (gdb) p/x ranges.data_[1]
$2 = {begin = 0x3fff7f9e740, end = 0x3fff7f9eed0, align = 0x40, tls_modid = 0x2} (gdb) p/x ranges.data_[2]
$3 = {begin = 0x3fff7f9eed8, end = 0x3fff7f9eef8, align = 0x8, tls_modid = 0x4} (gdb) p/x ranges.data_[3]
$4 = {begin = 0x3fff7f9eefc, end = 0x3fff7f9ef00, align = 0x4, tls_modid = 0x1}
where ranges[3].begin == ranges[2].end + ranges[3].align holds. Since in the loop a strict inequality test is used we compute the wrong address
(gdb) p/x *addr
$5 = 0x3fff7f9eefc
whereas 0x3fff7f9e6b8 is expected which is why we bail out in the subsequent.