[compiler-rt] d3531fc - [sanitizer] Don't run malloc hooks for stacktraces

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 13 16:34:02 PDT 2022


Author: Vitaly Buka
Date: 2022-04-13T16:33:53-07:00
New Revision: d3531fc7f0053a7fe68317521ee6491da6e36733

URL: https://github.com/llvm/llvm-project/commit/d3531fc7f0053a7fe68317521ee6491da6e36733
DIFF: https://github.com/llvm/llvm-project/commit/d3531fc7f0053a7fe68317521ee6491da6e36733.diff

LOG: [sanitizer] Don't run malloc hooks for stacktraces

Usually when we generated stacktraces the process is in error state, so
running hooks may crash the process and prevent meaningfull error report.

Symbolizer, unwinder and pthread are potential source of mallocs.

https://b.corp.google.com/issues/228110771

Reviewed By: kda

Differential Revision: https://reviews.llvm.org/D123566

Added: 
    compiler-rt/test/sanitizer_common/TestCases/malloc_hook_skip.cpp

Modified: 
    compiler-rt/lib/msan/msan_allocator.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_common.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_common.h
    compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h
    compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/msan/msan_allocator.cpp b/compiler-rt/lib/msan/msan_allocator.cpp
index 0d5e85329850b..7ff1bc99a4486 100644
--- a/compiler-rt/lib/msan/msan_allocator.cpp
+++ b/compiler-rt/lib/msan/msan_allocator.cpp
@@ -194,16 +194,19 @@ static void *MsanAllocate(StackTrace *stack, uptr size, uptr alignment,
       __msan_set_origin(allocated, size, o.raw_id());
     }
   }
-  UnpoisonParam(2);
-  RunMallocHooks(allocated, size);
+  if (!IsInSymbolizerOrUnwider()) {
+    UnpoisonParam(2);
+    RunMallocHooks(allocated, size);
+  }
   return allocated;
 }
 
 void MsanDeallocate(StackTrace *stack, void *p) {
   CHECK(p);
-  UnpoisonParam(1);
-  RunFreeHooks(p);
-
+  if (!IsInSymbolizerOrUnwider()) {
+    UnpoisonParam(1);
+    RunFreeHooks(p);
+  }
   Metadata *meta = reinterpret_cast<Metadata *>(allocator.GetMetaData(p));
   uptr size = meta->requested_size;
   meta->requested_size = 0;

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp
index e30a93da5b598..674d19d2a3916 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp
@@ -310,7 +310,23 @@ struct MallocFreeHook {
 
 static MallocFreeHook MFHooks[kMaxMallocFreeHooks];
 
+#if !SANITIZER_SUPPORTS_THREADLOCAL || SANITIZER_GO || SANITIZER_MAC || \
+    SANITIZER_ANDROID
+// FIXME: Prevent hooks on other platforms.
+static constexpr int disable_malloc_hooks = 0;
+ScopedDisableMallocHooks::ScopedDisableMallocHooks() {}
+ScopedDisableMallocHooks::~ScopedDisableMallocHooks() {}
+#else
+static THREADLOCAL int disable_malloc_hooks = 0;
+ScopedDisableMallocHooks::ScopedDisableMallocHooks() { ++disable_malloc_hooks; }
+ScopedDisableMallocHooks::~ScopedDisableMallocHooks() {
+  --disable_malloc_hooks;
+}
+#endif
+
 void RunMallocHooks(void *ptr, uptr size) {
+  if (disable_malloc_hooks)
+    return;
   __sanitizer_malloc_hook(ptr, size);
   for (int i = 0; i < kMaxMallocFreeHooks; i++) {
     auto hook = MFHooks[i].malloc_hook;
@@ -321,6 +337,8 @@ void RunMallocHooks(void *ptr, uptr size) {
 }
 
 void RunFreeHooks(void *ptr) {
+  if (disable_malloc_hooks)
+    return;
   __sanitizer_free_hook(ptr);
   for (int i = 0; i < kMaxMallocFreeHooks; i++) {
     auto hook = MFHooks[i].free_hook;

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
index 17570d6068856..0245687dd31e2 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -170,9 +170,18 @@ void SetShadowRegionHugePageMode(uptr addr, uptr length);
 bool DontDumpShadowMemory(uptr addr, uptr length);
 // Check if the built VMA size matches the runtime one.
 void CheckVMASize();
+
 void RunMallocHooks(void *ptr, uptr size);
 void RunFreeHooks(void *ptr);
 
+// Prevents RunMallocHooks and RunFreeHooks. Can be used in places where hooks
+// are undesirable, like in symbolizer or unwinder.
+class ScopedDisableMallocHooks {
+ public:
+  ScopedDisableMallocHooks();
+  ~ScopedDisableMallocHooks();
+};
+
 class ReservedAddressRange {
  public:
   uptr Init(uptr size, const char *name = nullptr, uptr fixed_addr = 0);

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp
index 848953a6ab007..97bef87a1165b 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp
@@ -75,6 +75,7 @@ void Abort() { abort(); }
 int Atexit(void (*function)(void)) { return atexit(function); }
 
 void GetThreadStackTopAndBottom(bool, uptr *stack_top, uptr *stack_bottom) {
+  ScopedDisableMallocHooks disable_hooks;  // pthread can malloc.
   pthread_attr_t attr;
   CHECK_EQ(pthread_getattr_np(pthread_self(), &attr), 0);
   void *base;

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
index 25ad825f568bd..b7577314b8bde 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
@@ -103,6 +103,7 @@ void GetThreadStackTopAndBottom(bool at_initialization, uptr *stack_top,
                                 uptr *stack_bottom) {
   CHECK(stack_top);
   CHECK(stack_bottom);
+  ScopedDisableMallocHooks disable_hooks;  // pthread can malloc.
   if (at_initialization) {
     // This is the main thread. Libpthread may not be initialized yet.
     struct rlimit rl;

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index 41b90b9f35802..e8f7af484fb84 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -410,6 +410,7 @@ void GetThreadStackTopAndBottom(bool at_initialization, uptr *stack_top,
                                 uptr *stack_bottom) {
   CHECK(stack_top);
   CHECK(stack_bottom);
+  ScopedDisableMallocHooks disable_hooks;  // pthread can malloc.
   uptr stacksize = pthread_get_stacksize_np(pthread_self());
   // pthread_get_stacksize_np() returns an incorrect stack size for the main
   // thread on Mavericks. See

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h
index bad4761e345fe..96b27c047dc1f 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h
@@ -212,6 +212,8 @@ class Symbolizer final {
     ~SymbolizerScope();
    private:
     const Symbolizer *sym_;
+
+    ScopedDisableMallocHooks disable_hooks_;  // Symbolizer can malloc.
   };
 };
 

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
index 1ec0c5cad7a20..9670dcf829e2e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
@@ -126,6 +126,7 @@ _Unwind_Reason_Code Unwind_Trace(struct _Unwind_Context *ctx, void *param) {
 void BufferedStackTrace::UnwindSlow(uptr pc, u32 max_depth) {
   CHECK_GE(max_depth, 2);
   size = 0;
+  ScopedDisableMallocHooks disable_hooks;  // libunwind can malloc.
   UnwindTraceArg arg = {this, Min(max_depth + 1, kStackTraceMax)};
   _Unwind_Backtrace(Unwind_Trace, &arg);
   CHECK_GT(size, 0);

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cpp
index b2628dcc4dc1f..89db2a3aa14f3 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cpp
@@ -126,6 +126,7 @@ void SanitizerInitializeUnwinder() {
 void BufferedStackTrace::UnwindSlow(uptr pc, u32 max_depth) {
   CHECK_GE(max_depth, 2);
   size = 0;
+  ScopedDisableMallocHooks disable_hooks;  // libunwind can malloc.
   UnwindTraceArg arg = {this, Min(max_depth + 1, kStackTraceMax)};
   _Unwind_Backtrace(Unwind_Trace, &arg);
   // We need to pop a few frames so that pc is on top.
@@ -156,6 +157,7 @@ void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) {
     return;
   }
 
+  ScopedDisableMallocHooks disable_hooks;  // Maybe unneeded, but won't hurt.
   void *map = acquire_my_map_info_list();
   CHECK(map);
   InternalMmapVector<backtrace_frame_t> frames(kStackTraceMax);

diff  --git a/compiler-rt/test/sanitizer_common/TestCases/malloc_hook_skip.cpp b/compiler-rt/test/sanitizer_common/TestCases/malloc_hook_skip.cpp
new file mode 100644
index 0000000000000..80b760a72a379
--- /dev/null
+++ b/compiler-rt/test/sanitizer_common/TestCases/malloc_hook_skip.cpp
@@ -0,0 +1,37 @@
+// RUN: %clangxx -O0 -g %s -o %t && %run %t
+
+// Test requires platform with thread local support with no dependency on malloc.
+// UNSUPPORTED: android
+// UNSUPPORTED: ios
+
+#include <assert.h>
+#include <sanitizer/allocator_interface.h>
+#include <sanitizer/coverage_interface.h>
+#include <stdlib.h>
+
+static int hooks;
+extern "C" {
+
+void __sanitizer_malloc_hook(const volatile void *ptr, size_t sz) { ++hooks; }
+
+void __sanitizer_free_hook(const volatile void *ptr) { ++hooks; }
+
+} // extern "C"
+
+void MallocHook(const volatile void *ptr, size_t sz) { ++hooks; }
+void FreeHook(const volatile void *ptr) { ++hooks; }
+
+int main() {
+  int before;
+
+  before = hooks;
+  __sanitizer_print_stack_trace();
+  assert(before == hooks);
+
+  __sanitizer_install_malloc_and_free_hooks(MallocHook, FreeHook);
+  before = hooks;
+  __sanitizer_print_stack_trace();
+  assert(before == hooks);
+
+  return 0;
+}


        


More information about the llvm-commits mailing list