[sanitizer] Remove GetCurrentThread nullness checks from Allocate

[MaskRay]

The nullness check is unreachable.

• For the main thead and pthread_create created threads, the *Allocate functions must be called after *_current_thread is set.
  set.
• For threads created by Linux's clone, static TLS is either reused or
  set to a new value (CLONE_SETTLS).

Make this change for asan/msan and possibly extend the change to other
sanitizers. (asan supports many platforms and I am not 100% certain that
all platforms have the property.)

[llvmbot]

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

Author: Fangrui Song (MaskRay)

Changes

The *Allocate functions must be called after *_current_thread is
set. Delete the nullness checks.

Make this change for asan/msan and possibly extend the change to other
sanitizers. (asan supports many platforms and I am not 100% certain that
all platforms have the property.)

---

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

2 Files Affected:

• (modified) compiler-rt/lib/asan/asan_allocator.cpp (+2-9)
• (modified) compiler-rt/lib/msan/msan_allocator.cpp (+2-9)

diff --git a/compiler-rt/lib/asan/asan_allocator.cpp b/compiler-rt/lib/asan/asan_allocator.cpp
index 9e66f77217ec6b..e041861edaf0b7 100644
--- a/compiler-rt/lib/asan/asan_allocator.cpp
+++ b/compiler-rt/lib/asan/asan_allocator.cpp
@@ -576,15 +576,8 @@ struct Allocator {
     }
 
     AsanThread *t = GetCurrentThread();
-    void *allocated;
-    if (t) {
-      AllocatorCache *cache = GetAllocatorCache(&t->malloc_storage());
-      allocated = allocator.Allocate(cache, needed_size, 8);
-    } else {
-      SpinMutexLock l(&fallback_mutex);
-      AllocatorCache *cache = &fallback_allocator_cache;
-      allocated = allocator.Allocate(cache, needed_size, 8);
-    }
+    void *allocated = allocator.Allocate(
+        GetAllocatorCache(&t->malloc_storage()), needed_size, 8);
     if (UNLIKELY(!allocated)) {
       SetAllocatorOutOfMemory();
       if (AllocatorMayReturnNull())
diff --git a/compiler-rt/lib/msan/msan_allocator.cpp b/compiler-rt/lib/msan/msan_allocator.cpp
index d7d4967c949859..f478b9979f2daa 100644
--- a/compiler-rt/lib/msan/msan_allocator.cpp
+++ b/compiler-rt/lib/msan/msan_allocator.cpp
@@ -199,15 +199,8 @@ static void *MsanAllocate(BufferedStackTrace *stack, uptr size, uptr alignment,
     ReportRssLimitExceeded(stack);
   }
   MsanThread *t = GetCurrentThread();
-  void *allocated;
-  if (t) {
-    AllocatorCache *cache = GetAllocatorCache(&t->malloc_storage());
-    allocated = allocator.Allocate(cache, size, alignment);
-  } else {
-    SpinMutexLock l(&fallback_mutex);
-    AllocatorCache *cache = &fallback_allocator_cache;
-    allocated = allocator.Allocate(cache, size, alignment);
-  }
+  void *allocated = allocator.Allocate(GetAllocatorCache(&t->malloc_storage()),
+                                       size, alignment);
   if (UNLIKELY(!allocated)) {
     SetAllocatorOutOfMemory();
     if (AllocatorMayReturnNull())

[vitalybuka]

The *Allocate functions must be called after *_current_thread is set. Delete the nullness checks.

With static runtime.

With dynamic runtime, with late activation, it's not guaranteed, I guess?

[MaskRay]

The *Allocate functions must be called after *_current_thread is set. Delete the nullness checks.

With static runtime.

With dynamic runtime, with late activation, it's not guaranteed, I guess?

I've checked start-deactivated.cpp. In the ASAN_OPTIONS=start_deactivated=1 mode, interceptors are still initialized.
As long as DlsymAlloc::Use() in the interceptor returns false and __asan::Allocator::Allocate is called, GetCurrentThread() guarantees a non-null value.

[alexander-shaposhnikov]

wondering if it's initialized if threads are created by direct calls of clone ?
(i guess this happens very rarely, but potentially it is possible)

[vitalybuka]

Have you tried to run out large set of internal tests?

[MaskRay]

Have you tried to run out large set of internal tests?

Finished the large set of tests. The results (with a lot of noise) look good

[cachemeifyoucan]

This breaks Darwin bots: https://green.lab.llvm.org/job/llvm.org/job/clang-stage1-RA/1753/

GTEST_OUTPUT=json:/Users/ec2-user/jenkins/workspace/llvm.org/clang-stage1-RA/clang-build/tools/clang/runtime/compiler-rt-bins/lib/asan/tests/X86_64HDarwinConfig/./Asan-x86_64h-calls-Noinst-Test-AddressSanitizer-Unit-90874-10-16.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=16 GTEST_SHARD_INDEX=10 /Users/ec2-user/jenkins/workspace/llvm.org/clang-stage1-RA/clang-build/tools/clang/runtime/compiler-rt-bins/lib/asan/tests/X86_64HDarwinConfig/./Asan-x86_64h-calls-Noinst-Test
--

Note: This is test shard 11 of 16.
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from AddressSanitizer
[ RUN      ] AddressSanitizer.ThreadedMallocStressTest
AddressSanitizerAddressSanitizerAddressSanitizerAddressSanitizer:DEADLYSIGNAL
:DEADLYSIGNAL
:DEADLYSIGNAL
:DEADLYSIGNAL
=================================================================
==92755==ERROR: AddressSanitizer: SEGV on unknown address 0x000000005e40 (pc 0x00010fa7eff8 bp 0x7000043ca630 sp 0x7000043ca600 T-1)
==92755==The signal is caused by a READ memory access.
    <empty stack>

==92755==Register values:
rax = 0x0000000000005d70  rbx = 0x0000000000000017  rcx = 0x0000000000000008  rdx = 0x0000000000000020  
rdi = 0x000000010fbb5eb8  rsi = 0x00000000000000d0  rbp = 0x00007000043ca630  rsp = 0x00007000043ca600  
 r8 = 0x0000000000000002   r9 = 0x0000000000000001  r10 = 0x00007ff7b04d6248  r11 = 0x0000000000000202  
r12 = 0x0000000000000008  r13 = 0x0000000000000080  r14 = 0xfffffffffffffff8  r15 = 0x0000000000005e40  
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV
==92755==ABORTING

--
exit: 1
--
shard JSON output does not exist: /Users/ec2-user/jenkins/workspace/llvm.org/clang-stage1-RA/clang-build/tools/clang/runtime/compiler-rt-bins/lib/asan/tests/X86_64HDarwinConfig/./Asan-x86_64h-calls-Noinst-Test-AddressSanitizer-Unit-90874-10-16.json

Maybe revert?

[cachemeifyoucan]

This also break Darwin ASAN bots for bootstrapping itself. I will revert for now.

[cachemeifyoucan]

Reverted: 16f4e85860efcccbadca5d0a00a3872244efae08

[MaskRay]

macOS uses the dynamic runtime and GetCurrentThread() could return nullptr.