From bf4ed5643ef9c69caf491d7db12523a39b3b0e25 Mon Sep 17 00:00:00 2001
From: Nika Layzell <nika@thelayzells.com>
Date: Mon, 20 Jan 2020 18:23:48 +0000
Subject: [PATCH] Bug 1608158 - Include MainThreadRunnableName in crash
 reports, r=chutten,gsvelto

While this information is often included in crash stacks, there are a few
situations where this probe can give us better information:

* Sometimes we get absolute garbage for part of the stack, and having
  information off to the side could help clarify things.
* Sometimes many different runnables get their Run() method merged through code
  folding, and so the Run() you see in the stack is very much not what you would
  expect to see. Having the runnable name be available could help clarify things.
* This data may be easier to work with in bulk as it is a separate,
  well-defined, field.

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

--HG--
extra : moz-landing-system : lando
---
 .../telemetry/docs/data/crash-ping.rst        |  1 +
 toolkit/crashreporter/CrashAnnotations.yaml   |  6 ++++
 toolkit/crashreporter/nsExceptionHandler.cpp  | 29 +++++++++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/toolkit/components/telemetry/docs/data/crash-ping.rst b/toolkit/components/telemetry/docs/data/crash-ping.rst
index 0902e3b690f0..ccfae48a2fe5 100644
--- a/toolkit/components/telemetry/docs/data/crash-ping.rst
+++ b/toolkit/components/telemetry/docs/data/crash-ping.rst
@@ -74,6 +74,7 @@ Structure:
           TotalVirtualMemory: <size>, // Windows-only, virtual memory in use expressed in bytes
           UptimeTS: <duration>, // Seconds since Firefox was started, this can have a fractional component
           User32BeforeBlocklist: "1", // Windows-only, present only if user32.dll was loaded before the DLL blocklist has been initialized
+          MainThreadRunnableName: <name>, // Optional, Nightly-only, name of the currently executing nsIRunnable on the main thread
         },
         hasCrashEnvironment: bool
       }
diff --git a/toolkit/crashreporter/CrashAnnotations.yaml b/toolkit/crashreporter/CrashAnnotations.yaml
index 412c4a21792e..983012a9843b 100644
--- a/toolkit/crashreporter/CrashAnnotations.yaml
+++ b/toolkit/crashreporter/CrashAnnotations.yaml
@@ -600,6 +600,12 @@ LowCommitSpaceEvents:
   type: integer
   ping: true
 
+MainThreadRunnableName:
+  description: >
+    Name of the currently executing nsIRunnable on the main thread.
+  type: string
+  ping: true
+
 MarshalActCtxManifestPath:
   description: >
     Proxy stream marshalling current activation context manifest path.
diff --git a/toolkit/crashreporter/nsExceptionHandler.cpp b/toolkit/crashreporter/nsExceptionHandler.cpp
index eff43d8d500e..dd4c815bf4df 100644
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -27,6 +27,7 @@
 #include "mozilla/ipc/CrashReporterClient.h"
 
 #include "nsThreadUtils.h"
+#include "nsThread.h"
 #include "jsfriendapi.h"
 #include "ThreadAnnotation.h"
 #include "private/pprio.h"
@@ -133,6 +134,7 @@ typedef std::wstring xpstring;
 #  define CONVERT_XP_CHAR_TO_UTF16(x) x
 #  define XP_STRLEN(x) wcslen(x)
 #  define my_strlen strlen
+#  define my_memchr memchr
 #  define CRASH_REPORTER_FILENAME "crashreporter.exe"
 #  define XP_PATH_SEPARATOR L"\\"
 #  define XP_PATH_SEPARATOR_CHAR L'\\'
@@ -163,6 +165,7 @@ typedef std::string xpstring;
 #    define XP_TTOA(time, buffer) sprintf(buffer, "%ld", time)
 #    define XP_STOA(size, buffer) sprintf(buffer, "%zu", (size_t)size)
 #    define my_strlen strlen
+#    define my_memchr memchr
 #    define sys_close close
 #    define sys_fork fork
 #    define sys_open open
@@ -1276,6 +1279,28 @@ static bool LaunchCrashHandlerService(XP_CHAR* aProgramPath,
 
 #endif
 
+static void WriteMainThreadRunnableName(AnnotationWriter& aWriter) {
+#ifdef MOZ_COLLECTING_RUNNABLE_TELEMETRY
+  // Only try to collect this information if the main thread is crashing.
+  if (!NS_IsMainThread()) {
+    return;
+  }
+
+  // NOTE: Use `my_memchr` over `strlen` to ensure we don't run off the end of
+  // the buffer if it contains no null bytes. This is used instead of `strnlen`,
+  // as breakpad's linux support library doesn't export a `my_strnlen` function.
+  const char* buf = nsThread::sMainThreadRunnableName.begin();
+  size_t len = nsThread::kRunnableNameBufSize;
+  if (const void* end = my_memchr(buf, '\0', len)) {
+    len = static_cast<const char*>(end) - buf;
+  }
+
+  if (len > 0) {
+    aWriter.Write(Annotation::MainThreadRunnableName, buf, len);
+  }
+#endif
+}
+
 static void WriteMozCrashReason(AnnotationWriter& aWriter) {
   if (gMozCrashReason != nullptr) {
     aWriter.Write(Annotation::MozCrashReason, gMozCrashReason);
@@ -1344,6 +1369,8 @@ static void WriteAnnotationsForMainProcessCrash(PlatformWriter& pw,
 
   WriteMozCrashReason(writer);
 
+  WriteMainThreadRunnableName(writer);
+
   char oomAllocationSizeBuffer[32] = "";
   if (gOOMAllocationSize) {
     XP_STOA(gOOMAllocationSize, oomAllocationSizeBuffer);
@@ -1645,6 +1672,8 @@ static void PrepareChildExceptionTimeAnnotations(
 
   WriteMozCrashReason(writer);
 
+  WriteMainThreadRunnableName(writer);
+
 #ifdef MOZ_PHC
   WritePHCAddrInfo(writer, addrInfo);
 #endif