[rtsan] Make sure rtsan gets initialized on mac

[cjappl]

Intermittently on my mac I was getting the same nullptr crash in dlsym.

We need to make sure rtsan gets initialized on mac between when the binary starts running, and the first intercepted function is called. Until that point we should use the DlsymAllocator.

[cjappl]

CC For review @vitalybuka @MaskRay @davidtrevelyan

[llvmbot]

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

Author: Chris Apple (cjappl)

Changes

Intermittently on my mac I was getting the same nullptr crash in dlsym.

We need to make sure rtsan gets initialized on mac between when the binary starts running, and the first intercepted function is called. Until that point we should use the DlsymAllocator.

---

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

1 Files Affected:

• (modified) compiler-rt/lib/rtsan/rtsan_interceptors.cpp (+3-1)

diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors.cpp
index 4d5423ec629d2..833062c3de41e 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors.cpp
@@ -39,7 +39,6 @@
 
 using namespace __sanitizer;
 
-using __rtsan::rtsan_init_is_running;
 using __rtsan::rtsan_initialized;
 
 namespace {
@@ -49,6 +48,9 @@ struct DlsymAlloc : public DlSymAllocator<DlsymAlloc> {
 } // namespace
 
 void ExpectNotRealtime(const char *intercepted_function_name) {
+  if (!rtsan_initialized)
+    __rtsan_init();
+
   __rtsan::GetContextForThisThread().ExpectNotRealtime(
       intercepted_function_name);
 }

[cjappl]

Pinging reviewers @MaskRay @vitalybuka

Thanks as always for the help.

[cjappl]

Weekly ping of reviewers: @vitalybuka @MaskRay

Current state: All outstanding review comments have been addressed.

[vitalybuka]

why don't you call a static one here?

[cjappl]

Is there a benefit of calling a static function vs the external API function?

[cjappl]

(for the record, the external method must exist because we use it in rtsan_interceptors to see if we need to use the start-up allocator before dlsym is fully settled)

[vitalybuka]

I guess it's cleaner with a mutex, like asan_inited_mutex

There is possibility that 2 racing thread will pass CHECK(!IsInitRunning());

[cjappl]

Good catch. Alright, this now does the following:

Checks if initialized - using an atomic to prevent data races
If not, grabs the mutex
Once the mutex is grabbed, checks the atomic once more to ensure we don't double initialize.

I think this handles all the possible data races in this path.

The separation of rtsan_ensure_inited vs rtsan_init is to make sure the preinit array initialization of rtsan is as lean as possible. This is following some of the patterns I saw in other sanitizers.

[cjappl]

Two very small updates since last review:

1. Matching llvm style "If you use a braced initializer list when initializing a variable, use an equals before the open curly brace:"

- static atomic_uint8_t rtsan_initialized{0};
+ static atomic_uint8_t rtsan_initialized = {0};

2. Got rid of the now-unused IsInitialized in favor of the __rtsan_is_initialized method, this was cruft from an earlier version of this review.

These two changes may be reviewed in the latest two commits, if anyone wants to see them in isolation.

I also rebased on latest main. I will likely merge this early next week

[cjappl]

Thanks for the merge @vitalybuka !

I went ahead and requested commit access to LLVM, so I'll be able to merge things next time I get approvals. I hope that leads to less clerical tasks on your plate.