Bug 1829128 - Crashes in jemalloc don't have useful PHC stacks r=gsvelto

Differential Revision: https://phabricator.services.mozilla.com/D180196

memory/build/malloc_decls.h

@@ -112,6 +112,12 @@ MALLOC_DECL(jemalloc_thread_local_arena, void, bool)
 
 // Provide information about any allocation enclosing the given address.
 MALLOC_DECL(jemalloc_ptr_info, void, const void*, jemalloc_ptr_info_t*)
+
+// If jemalloc is currently doing something on this thread then this will return
+// true. This is for signal handlers, if jemalloc causes a segfault during free
+// (or realloc etc) then this can tell the signal handler that the crashing
+// address isn't useful for something like PHC.
+MALLOC_DECL(jemalloc_is_working, bool)
 #  endif
 
 #  if MALLOC_FUNCS & MALLOC_FUNCS_ARENA_BASE

memory/build/mozjemalloc.cpp

@@ -3651,6 +3651,27 @@ MOZ_NEVER_INLINE jemalloc_ptr_info_t* jemalloc_ptr_info(const void* aPtr) {
 }
 }  // namespace Debug
 
+// Used in crash reporting to query what pointer jemalloc was working on when it
+// crashed.
+static MOZ_THREAD_LOCAL(unsigned) gWorkingCount;
+
+struct MOZ_RAII AutoSetWorking {
+  explicit AutoSetWorking() { gWorkingCount.set(gWorkingCount.get() + 1); }
+
+  AutoSetWorking(const AutoSetWorking&) = delete;
+  AutoSetWorking(AutoSetWorking&&) = delete;
+
+  ~AutoSetWorking() {
+    MOZ_ASSERT(gWorkingCount.get());
+    gWorkingCount.set(gWorkingCount.get() - 1);
+  }
+};
+
+template <>
+inline bool MozJemalloc::jemalloc_is_working() {
+  return !!gWorkingCount.get();
+}
+
 arena_chunk_t* arena_t::DallocSmall(arena_chunk_t* aChunk, void* aPtr,
                                     arena_chunk_map_t* aMapElm) {
   arena_run_t* run;
@@ -3736,6 +3757,8 @@ static inline void arena_dalloc(void* aPtr, size_t aOffset, arena_t* aArena) {
   MOZ_ASSERT(aOffset != 0);
   MOZ_ASSERT(GetChunkOffsetForPtr(aPtr) == aOffset);
 
+  AutoSetWorking auto_working;
+
   auto chunk = (arena_chunk_t*)((uintptr_t)aPtr - aOffset);
   auto arena = chunk->arena;
   MOZ_ASSERT(arena);
@@ -4279,6 +4302,7 @@ static bool malloc_init_hard() {
   if (!thread_arena.init()) {
     return true;
   }
+  MOZ_ASSERT(gWorkingCount.init());
 
   // Get page size and number of CPUs
   const size_t result = GetKernelPageSize();
@@ -4543,6 +4567,7 @@ inline void* BaseAllocator::realloc(void* aPtr, size_t aSize) {
 
   if (aPtr) {
     MOZ_RELEASE_ASSERT(malloc_initialized);
+    AutoSetWorking auto_working;
 
     auto info = AllocInfo::Get(aPtr);
     auto arena = info.Arena();
@@ -4649,6 +4674,7 @@ inline size_t MozJemalloc::malloc_good_size(size_t aSize) {
 
 template <>
 inline size_t MozJemalloc::malloc_usable_size(usable_ptr_t aPtr) {
+  AutoSetWorking auto_working;
   return AllocInfo::GetValidated(aPtr).Size();
 }
 

toolkit/crashreporter/breakpad-client/linux/handler/exception_handler.cc

@@ -102,6 +102,7 @@
 #endif
 
 #ifdef MOZ_PHC
+#include "mozmemory.h"
 #include "replace_malloc_bridge.h"
 #endif
 
@@ -467,9 +468,14 @@ static void GetPHCAddrInfo(siginfo_t* siginfo,
 // Runs on the crashing thread.
 bool ExceptionHandler::HandleSignal(int /*sig*/, siginfo_t* info, void* uc) {
   mozilla::phc::AddrInfo* addr_info = nullptr;
+
 #ifdef MOZ_PHC
-  addr_info = &mozilla::phc::gAddrInfo;
-  GetPHCAddrInfo(info, addr_info);
+  if (!jemalloc_is_working()) {
+    // If jemalloc crashed then the PHC information is invalid. Only retrive
+    // it when we know its valid.
+    addr_info = &mozilla::phc::gAddrInfo;
+    GetPHCAddrInfo(info, addr_info);
+  }
 #endif
 
   if (filter_ && !filter_(callback_context_)) {

toolkit/crashreporter/breakpad-client/mac/handler/exception_handler.cc

@@ -43,6 +43,7 @@
 #include "mozilla/Assertions.h"
 
 #ifdef MOZ_PHC
+#include "mozmemory.h"
 #include "replace_malloc_bridge.h"
 #endif
 
@@ -444,8 +445,12 @@ bool ExceptionHandler::WriteMinidumpWithException(
 
   mozilla::phc::AddrInfo* addr_info = nullptr;
 #ifdef MOZ_PHC
-  addr_info = &mozilla::phc::gAddrInfo;
-  GetPHCAddrInfo(exception_type, exception_subcode, addr_info);
+  if (!jemalloc_is_working()) {
+    // If jemalloc crashed then the PHC information is invalid. Only retrive
+    // it when we know its valid.
+    addr_info = &mozilla::phc::gAddrInfo;
+    GetPHCAddrInfo(exception_type, exception_subcode, addr_info);
+  }
 #endif
 
   if (directCallback_) {

toolkit/crashreporter/breakpad-client/windows/handler/exception_handler.cc

@@ -40,6 +40,7 @@
 #include "common/windows/guid_string.h"
 
 #ifdef MOZ_PHC
+#include "mozmemory.h"
 #include "replace_malloc_bridge.h"
 #endif
 
@@ -915,8 +916,12 @@ ExceptionHandler::MinidumpResult ExceptionHandler::WriteMinidumpWithException(
 {
   mozilla::phc::AddrInfo* addr_info = nullptr;
 #ifdef MOZ_PHC
-  addr_info = &mozilla::phc::gAddrInfo;
-  GetPHCAddrInfo(exinfo, addr_info);
+  if (!jemalloc_is_working()) {
+    // If jemalloc crashed then the PHC information is invalid. Only retrive
+    // it when we know its valid.
+    addr_info = &mozilla::phc::gAddrInfo;
+    GetPHCAddrInfo(exinfo, addr_info);
+  }
 #endif
 
   // Give user code a chance to approve or prevent writing a minidump.  If the