[compiler-rt][rtsan] Use sanitizer internal allocator during rtsan init to avoid segfault in dlsym

[davidtrevelyan]

Follows #98268 with a fix for a segfault during preinit on ubuntu:20.04 environments. Previously, rtsan was not handling the situation where dlsym calls calloc during the interceptors initialization, resulting in a call to a function at a null address.

@cjappl and I took inspiration from the solution in nsan, but we re-used the sanitizer internal allocator instead of our own static buffer. This PR also re-enables the existing non-instrumented rtsan tests for x86_64 and arm64 architectures.

[llvmbot]

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

Author: None (davidtrevelyan)

Changes

Follows #98268 with a fix for a segfault during preinit on ubuntu:20.04 environments. Previously, rtsan was not handling the situation where dlsym calls calloc during the interceptors initialization, resulting in a call to a function at a null address.

@cjappl and I took inspiration from the solution in nsan, but we re-used the sanitizer internal allocator instead of our own static buffer. This PR also re-enables the existing non-instrumented rtsan tests for x86_64 and arm64 architectures.

---

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

4 Files Affected:

• (modified) compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake (+1-3)
• (modified) compiler-rt/lib/rtsan/rtsan.cpp (+14-1)
• (modified) compiler-rt/lib/rtsan/rtsan.h (+7)
• (modified) compiler-rt/lib/rtsan/rtsan_interceptors.cpp (+17)

diff --git a/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake b/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
index c8bec41db36e9..647f67b66af85 100644
--- a/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
+++ b/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
@@ -32,9 +32,7 @@ set(ALL_ASAN_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${RISCV64}
     ${LOONGARCH64})
 set(ALL_ASAN_ABI_SUPPORTED_ARCH ${X86_64} ${ARM64} ${ARM64_32})
 set(ALL_DFSAN_SUPPORTED_ARCH ${X86_64} ${MIPS64} ${ARM64} ${LOONGARCH64})
-#set(ALL_RTSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${RISCV64}
-#    ${MIPS32} ${MIPS64} ${PPC64} ${S390X} ${SPARC} ${SPARCV9} ${HEXAGON}
-#    ${LOONGARCH64})
+set(ALL_RTSAN_SUPPORTED_ARCH ${X86_64} ${ARM64})
 
 if(ANDROID)
   set(OS_NAME "Android")
diff --git a/compiler-rt/lib/rtsan/rtsan.cpp b/compiler-rt/lib/rtsan/rtsan.cpp
index 43a63b4636f1e..cf7fbddd9eb9c 100644
--- a/compiler-rt/lib/rtsan/rtsan.cpp
+++ b/compiler-rt/lib/rtsan/rtsan.cpp
@@ -12,10 +12,23 @@
 #include <rtsan/rtsan_context.h>
 #include <rtsan/rtsan_interceptors.h>
 
+using namespace __rtsan;
+
+bool __rtsan::rtsan_initialized;
+bool __rtsan::rtsan_init_is_running;
+
 extern "C" {
 
 SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_init() {
-  __rtsan::InitializeInterceptors();
+  CHECK(!rtsan_init_is_running);
+  if (rtsan_initialized)
+    return;
+  rtsan_init_is_running = true;
+
+  InitializeInterceptors();
+
+  rtsan_init_is_running = false;
+  rtsan_initialized = true;
 }
 
 SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_realtime_enter() {
diff --git a/compiler-rt/lib/rtsan/rtsan.h b/compiler-rt/lib/rtsan/rtsan.h
index 8b1219c13cbd3..ccddaf2c893ef 100644
--- a/compiler-rt/lib/rtsan/rtsan.h
+++ b/compiler-rt/lib/rtsan/rtsan.h
@@ -14,6 +14,13 @@
 
 extern "C" {
 
+namespace __rtsan {
+
+extern bool rtsan_initialized;
+extern bool rtsan_init_is_running;
+
+} // namespace __rtsan
+
 // Initialise rtsan interceptors.
 // A call to this method is added to the preinit array on Linux systems.
 SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_init();
diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors.cpp
index 3a65f9d3f779d..f7e75b15ca33e 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors.cpp
@@ -10,10 +10,12 @@
 
 #include "rtsan/rtsan_interceptors.h"
 
+#include "sanitizer_common/sanitizer_allocator_internal.h"
 #include "sanitizer_common/sanitizer_platform.h"
 #include "sanitizer_common/sanitizer_platform_interceptors.h"
 
 #include "interception/interception.h"
+#include "rtsan/rtsan.h"
 #include "rtsan/rtsan_context.h"
 
 #if SANITIZER_APPLE
@@ -35,6 +37,9 @@
 
 using namespace __sanitizer;
 
+using __rtsan::rtsan_init_is_running;
+using __rtsan::rtsan_initialized;
+
 void ExpectNotRealtime(const char *intercepted_function_name) {
   __rtsan::GetContextForThisThread().ExpectNotRealtime(
       intercepted_function_name);
@@ -238,11 +243,17 @@ INTERCEPTOR(int, nanosleep, const struct timespec *rqtp,
 // Memory
 
 INTERCEPTOR(void *, calloc, SIZE_T num, SIZE_T size) {
+  if (rtsan_init_is_running && REAL(calloc) == nullptr)
+    return __sanitizer::InternalCalloc(num, size);
+
   ExpectNotRealtime("calloc");
   return REAL(calloc)(num, size);
 }
 
 INTERCEPTOR(void, free, void *ptr) {
+  if (__sanitizer::internal_allocator()->PointerIsMine(ptr))
+    return __sanitizer::InternalFree(ptr);
+
   if (ptr != NULL) {
     ExpectNotRealtime("free");
   }
@@ -250,11 +261,17 @@ INTERCEPTOR(void, free, void *ptr) {
 }
 
 INTERCEPTOR(void *, malloc, SIZE_T size) {
+  if (rtsan_init_is_running && REAL(malloc) == nullptr)
+    return __sanitizer::InternalAlloc(size);
+
   ExpectNotRealtime("malloc");
   return REAL(malloc)(size);
 }
 
 INTERCEPTOR(void *, realloc, void *ptr, SIZE_T size) {
+  if (rtsan_init_is_running && REAL(realloc) == nullptr)
+    return __sanitizer::InternalRealloc(ptr, size);
+
   ExpectNotRealtime("realloc");
   return REAL(realloc)(ptr, size);
 }

[davidtrevelyan]

Tagging interested parties: @vitalybuka @cjappl @chapuni - many thanks in advance for any reviews.

[chapuni]

This works for me.
Please wait for other reviews.

[davidtrevelyan]

Polite ping FAO @vitalybuka and @MaskRay - your oversight on this PR would be much appreciated, @cjappl and I are just waiting for this and #99437 (also approved) to go in before we submit our next PR for rtsan's frontend in clang.

Just wanted to also leave a reminder that I don't have commit access, so I will need someone to do the merge for me if there are no further requested changes.

Many thanks in advance for your time!

[MaskRay]

We should define UseImpl and use DlsymAlloc::Use. See #98415

[cjappl]

Fixed! Thanks

[cjappl]

Also, neither of the main authors of this patch have review access. If someone sees this after it has an acceptable number of approvals and wants to merge it, that would be great! Thanks

[MaskRay]

I was waiting for possible response from regular compiler-rt sanitizers folks:) I think this is good to merge in the absence of their response.

[cjappl]

Thanks @MaskRay ! We were unsure of what to do when it looked like there was some approval, but we didn't have merge rights. Thanks for your help as always

[cjappl]

Having an error on android, which I thought was previously removed but may have been caught in a revert conflict.

OSError: [Errno 8] Exec format error: '/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_aarch64/lib/rtsan/tests/./Rtsan-aarch64-NoInstTest'

#99964