[llvm-branch-commits] [sanitizer] Add CHECK that static TLS info is ready (PR #108684)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Vitaly Buka (vitalybuka)

<details>
<summary>Changes</summary>

There is possibility of
static_tls_begin is set and static_tls_end is not yet

The test reproduces the case.
Stack trace looks like this:
   `MsanThread::Init`
   `SetThreadStackAndTls`
   `GetThreadStackAndTls`
   `GetThreadStackTopAndBottom`
   `pthread_getattr_np`
   `realloc`
   `__sanitizer_malloc_hook`
   TLS access
   `___interceptor___tls_get_addr`
   `DTLS_on_tls_get_addr`

The issue is that `SetThreadStackAndTls` implementation
stores `tls_begin` before `GetThreadStackTopAndBottom`,
and `tls_end` after. So we have partially initialized
state in `DTLS_on_tls_get_addr`.


---
Full diff: https://github.com/llvm/llvm-project/pull/108684.diff


2 Files Affected:

- (modified) compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp (+1) 
- (added) compiler-rt/test/sanitizer_common/TestCases/Linux/tls_malloc_hook.c (+60) 


``````````diff
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
index 087bd801b6e5ff..a17a14882d0e15 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
@@ -130,6 +130,7 @@ DTLS::DTV *DTLS_on_tls_get_addr(void *arg_void, void *res,
   DTLS::DTV *dtv = DTLS_Find(dso_id);
   if (!dtv || dtv->beg)
     return nullptr;
+  CHECK_LE(static_tls_begin, static_tls_end);
   uptr tls_size = 0;
   uptr tls_beg = reinterpret_cast<uptr>(res) - arg->offset - kDtvOffset;
   VReport(2,
diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/tls_malloc_hook.c b/compiler-rt/test/sanitizer_common/TestCases/Linux/tls_malloc_hook.c
new file mode 100644
index 00000000000000..c582372ab9763d
--- /dev/null
+++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/tls_malloc_hook.c
@@ -0,0 +1,60 @@
+// Test that we don't crash accessing DTLS from malloc hook.
+
+// RUN: %clang %s -o %t
+// RUN: %clang %s -DBUILD_SO -fPIC -o %t-so.so -shared
+// RUN: %run %t 2>&1 | FileCheck %s
+
+// REQUIRES: glibc
+
+// No allocator and hooks.
+// XFAIL: ubsan
+
+// FIXME: Crashes on CHECK.
+// XFAIL: asan && !i386-linux
+// XFAIL: msan && !i386-linux
+
+#ifndef BUILD_SO
+#  include <assert.h>
+#  include <dlfcn.h>
+#  include <pthread.h>
+#  include <stdio.h>
+#  include <stdlib.h>
+
+typedef long *(*get_t)();
+get_t GetTls;
+void *Thread(void *unused) { return GetTls(); }
+
+__thread long recursive_hook;
+
+// CHECK: __sanitizer_malloc_hook:
+void __sanitizer_malloc_hook(const volatile void *ptr, size_t sz)
+    __attribute__((disable_sanitizer_instrumentation)) {
+  ++recursive_hook;
+  if (recursive_hook == 1 && GetTls)
+    fprintf(stderr, "__sanitizer_malloc_hook: %p\n", GetTls());
+  --recursive_hook;
+}
+
+int main(int argc, char *argv[]) {
+  char path[4096];
+  snprintf(path, sizeof(path), "%s-so.so", argv[0]);
+  int i;
+
+  void *handle = dlopen(path, RTLD_LAZY);
+  if (!handle)
+    fprintf(stderr, "%s\n", dlerror());
+  assert(handle != 0);
+  GetTls = (get_t)dlsym(handle, "GetTls");
+  assert(dlerror() == 0);
+
+  pthread_t t;
+  pthread_create(&t, 0, Thread, 0);
+  pthread_join(t, 0);
+  pthread_create(&t, 0, Thread, 0);
+  pthread_join(t, 0);
+  return 0;
+}
+#else // BUILD_SO
+__thread long huge_thread_local_array[1 << 17];
+long *GetTls() { return &huge_thread_local_array[0]; }
+#endif

``````````

</details>


https://github.com/llvm/llvm-project/pull/108684