[sanitizer] Don't run malloc hooks for stacktraces

[vitalybuka]

Summary

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

[kda]

compiler-rt/test/msan/Linux/malloc_hooks.cpp

25 ↗

(On Diff #422111)

While this shows that the hooks are not called, I'm not sure that it demonstrates that they would usually be called. Perhaps call a non-symbolizer method and show them increasing?

[kstoimenov]

compiler-rt/test/msan/Linux/malloc_hooks.cpp

8 ↗

(On Diff #422111)

Make this static?

21 ↗

(On Diff #422111)

Should we allocate memory to make sure that 'hooks' is non-zero? Also maybe add assert to check that is it is > 0.?

[vitalybuka]

extend on other sanitizers

[vitalybuka]

compiler-rt/test/msan/Linux/malloc_hooks.cpp

21 ↗

(On Diff #422111)

No, we have multiple tests which check that callbacks works..

25 ↗

(On Diff #422111)

No, we have multiple tests which check that callbacks works..
E.g. llvm-project/compiler-rt/test/sanitizer_common/TestCases/malloc_hook.cpp

[kstoimenov]

compiler-rt/lib/sanitizer_common/sanitizer_common.cpp

313

Maybe this can be a static class member of ScopedDisableMallocHooks? You could add a static ScopedDisableMallocHooks::DisableMallocHooks accessor. Your call.

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_common.cpp

313

It makes it visible for linker

[ahatanak]

It looks like this is causing a test to fail:
https://green.lab.llvm.org/green/job/clang-san-iossim/5229/consoleFull#711521816a1ca8a51-895e-46c6-af87-ce24fa4cd561

[vitalybuka]

Reverting

[vitalybuka]

rebase

[vitalybuka]

I probably going to abandon this, as I don't see good solution for all platforms.
Seems having sanitizer-unclean hooks is not good idea anyway.

compiler-rt/lib/msan/msan_allocator.cpp

static void *MsanAllocate(StackTrace *stack, uptr size, uptr alignment,
   } else if (flags()->poison_in_malloc) {
     __msan_poison(allocated, size);
     if (__msan_get_track_origins()) {
       stack->tag = StackTrace::TAG_ALLOC;
       Origin o = Origin::CreateHeapOrigin(stack);
       __msan_set_origin(allocated, size, o.raw_id());
     }
   }
+  if (!IsInSymbolizerOrUnwider()) {
     UnpoisonParam(2);
     RunMallocHooks(allocated, size);
+  }
   return allocated;
 }
 
 void MsanDeallocate(StackTrace *stack, void *p) {
   CHECK(p);
+  if (!IsInSymbolizerOrUnwider()) {
     UnpoisonParam(1);
     RunFreeHooks(p);
-
+  }
   Metadata *meta = reinterpret_cast<Metadata *>(allocator.GetMetaData(p));
   uptr size = meta->requested_size;
   meta->requested_size = 0;
   // This memory will not be reused by anyone else, so we are free to keep it
   // poisoned.
   if (flags()->poison_in_free) {
     __msan_poison(p, size);
     if (__msan_get_track_origins()) {

compiler-rt/lib/sanitizer_common/sanitizer_common.h

 void ReleaseMemoryPagesToOS(uptr beg, uptr end);
 void IncreaseTotalMmap(uptr size);
 void DecreaseTotalMmap(uptr size);
 uptr GetRSS();
 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);
   uptr InitAligned(uptr size, uptr align, const char *name = nullptr);
   uptr Map(uptr fixed_addr, uptr size, const char *name = nullptr);
   uptr MapOrDie(uptr fixed_addr, uptr size, const char *name = nullptr);
   void Unmap(uptr addr, uptr size);
   void *base() const { return base_; }

compiler-rt/lib/sanitizer_common/sanitizer_common.cpp

 static const int kMaxMallocFreeHooks = 5;
 struct MallocFreeHook {
   void (*malloc_hook)(const void *, uptr);
   void (*free_hook)(const void *);
 };
 
 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;
     if (!hook)
       break;
     hook(ptr, 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;
     if (!hook)
       break;
     hook(ptr);
   }
 }

compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp

 
 tid_t GetTid() { return GetThreadSelf(); }
 
 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;
   size_t size;
   CHECK_EQ(pthread_attr_getstack(&attr, &base, &size), 0);
   CHECK_EQ(pthread_attr_destroy(&attr), 0);
 
   *stack_bottom = reinterpret_cast<uptr>(base);

compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp

#endif
   return sigaction(signum, (const struct sigaction *)act,
                    (struct sigaction *)oldact);
 }
 
 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;
     CHECK_EQ(getrlimit(RLIMIT_STACK, &rl), 0);
 
     // Find the mapping that contains a stack variable.
     MemoryMappingLayout proc_maps(/*cache_enabled*/true);
     if (proc_maps.Error()) {

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

tid_t GetTid() {
   pthread_threadid_np(nullptr, &tid);
   return tid;
 }
 
 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
   // https://github.com/google/sanitizers/issues/261
   if ((GetMacosAlignedVersion() >= MacosVersion(10, 9)) && at_initialization &&
       stacksize == (1 << 19))  {
     struct rlimit rl;
     CHECK_EQ(getrlimit(RLIMIT_STACK, &rl), 0);

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h

private:
   StartSymbolizationHook start_hook_;
   EndSymbolizationHook end_hook_;
   class SymbolizerScope {
    public:
     explicit SymbolizerScope(const Symbolizer *sym);
     ~SymbolizerScope();
    private:
     const Symbolizer *sym_;
+
+    ScopedDisableMallocHooks disable_hooks_;  // Symbolizer can malloc.
   };
 };
 
 #ifdef SANITIZER_WINDOWS
 void InitializeDbgHelpIfNeeded();
 #endif
 
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_SYMBOLIZER_H

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp

_Unwind_Reason_Code Unwind_Trace(struct _Unwind_Context *ctx, void *param) {
   arg->stack->trace_buffer[arg->stack->size++] = pc;
   return (arg->stack->size == arg->max_depth ? _URC_NORMAL_STOP
                                              : _URC_NO_REASON);
 }
 
 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);
   // We need to pop a few frames so that pc is on top.
   uptr to_pop = LocatePcInTrace(pc);
   // trace_buffer[0] belongs to the current function so we always pop it,
   // unless there is only 1 frame in the stack trace (1 frame is always better
   // than 0!).

compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cpp

if (!acquire_my_map_info_list || !release_my_map_info_list ||
     release_my_map_info_list = 0;
   }
 }
 #endif
 
 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.
   uptr to_pop = LocatePcInTrace(pc);
   // trace_buffer[0] belongs to the current function so we always pop it,
   // unless there is only 1 frame in the stack trace (1 frame is always better
   // than 0!).
   // 1-frame stacks don't normally happen, but this depends on the actual

 void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) {
   CHECK(context);
   CHECK_GE(max_depth, 2);
   if (!unwind_backtrace_signal_arch) {
     UnwindSlow(pc, 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);
   // siginfo argument appears to be unused.
   sptr res = unwind_backtrace_signal_arch(/* siginfo */ 0, context, map,
                                           frames.data(),
                                           /* ignore_depth */ 0, max_depth);
   release_my_map_info_list(map);

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

+// 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
+// UNSUPPORTED: darwin
+
+#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;
+}