Bug 1286005 - Don't include the PID in the NS_DebugBreak crash annotation. r=froydnj
authorAndrew McCreight <continuation@gmail.com>
Mon, 25 Jul 2016 07:29:13 -0700
changeset 331624 e50eb5912d50b6e9cf6cb09f7137699218e68240
parent 331623 537f4d1d837aa24f1aa08614b0e1e59c72ce41cd
child 331625 9c839b1d1397c2f43a66aeb008f230b068f325ed
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1286005
milestone50.0a1
Bug 1286005 - Don't include the PID in the NS_DebugBreak crash annotation. r=froydnj Including the PID makes it impossible to aggregate crash reports on crash-stats. This also reduces buffer size to ensure that having two buffers does not increase total stack size, though that is unlikely to matter. 1000 characters is likely excessive in any event.
xpcom/base/nsDebugImpl.cpp
--- a/xpcom/base/nsDebugImpl.cpp
+++ b/xpcom/base/nsDebugImpl.cpp
@@ -266,17 +266,17 @@ GetAssertBehavior()
 
 struct FixedBuffer
 {
   FixedBuffer() : curlen(0)
   {
     buffer[0] = '\0';
   }
 
-  char buffer[1000];
+  char buffer[500];
   uint32_t curlen;
 };
 
 static int
 StuffFixedBuffer(void* aClosure, const char* aBuf, uint32_t aLen)
 {
   if (!aLen) {
     return 0;
@@ -301,16 +301,17 @@ StuffFixedBuffer(void* aClosure, const c
 
   return aLen;
 }
 
 EXPORT_XPCOM_API(void)
 NS_DebugBreak(uint32_t aSeverity, const char* aStr, const char* aExpr,
               const char* aFile, int32_t aLine)
 {
+  FixedBuffer nonPIDBuf;
   FixedBuffer buf;
   const char* sevString = "WARNING";
 
   switch (aSeverity) {
     case NS_DEBUG_ASSERTION:
       sevString = "###!!! ASSERTION";
       break;
 
@@ -321,42 +322,41 @@ NS_DebugBreak(uint32_t aSeverity, const 
     case NS_DEBUG_ABORT:
       sevString = "###!!! ABORT";
       break;
 
     default:
       aSeverity = NS_DEBUG_WARNING;
   }
 
-#  define PrintToBuffer(...) PR_sxprintf(StuffFixedBuffer, &buf, __VA_ARGS__)
+#define PRINT_TO_NONPID_BUFFER(...) PR_sxprintf(StuffFixedBuffer, &nonPIDBuf, __VA_ARGS__)
+  PRINT_TO_NONPID_BUFFER("%s: ", sevString);
+  if (aStr) {
+    PRINT_TO_NONPID_BUFFER("%s: ", aStr);
+  }
+  if (aExpr) {
+    PRINT_TO_NONPID_BUFFER("'%s', ", aExpr);
+  }
+  if (aFile) {
+    PRINT_TO_NONPID_BUFFER("file %s, ", aFile);
+  }
+  if (aLine != -1) {
+    PRINT_TO_NONPID_BUFFER("line %d", aLine);
+  }
+#undef PRINT_TO_NONPID_BUFFER
 
   // Print "[PID]" or "[Desc PID]" at the beginning of the message.
-  PrintToBuffer("[");
+#define PRINT_TO_BUFFER(...) PR_sxprintf(StuffFixedBuffer, &buf, __VA_ARGS__)
+  PRINT_TO_BUFFER("[");
   if (sMultiprocessDescription) {
-    PrintToBuffer("%s ", sMultiprocessDescription);
-  }
-
-  PrintToBuffer("%d] ", base::GetCurrentProcId());
-
-  PrintToBuffer("%s: ", sevString);
-
-  if (aStr) {
-    PrintToBuffer("%s: ", aStr);
+    PRINT_TO_BUFFER("%s ", sMultiprocessDescription);
   }
-  if (aExpr) {
-    PrintToBuffer("'%s', ", aExpr);
-  }
-  if (aFile) {
-    PrintToBuffer("file %s, ", aFile);
-  }
-  if (aLine != -1) {
-    PrintToBuffer("line %d", aLine);
-  }
+  PRINT_TO_BUFFER("%d] %s", base::GetCurrentProcId(), nonPIDBuf.buffer);
+#undef PRINT_TO_BUFFER
 
-#  undef PrintToBuffer
 
   // errors on platforms without a debugdlg ring a bell on stderr
 #if !defined(XP_WIN)
   if (aSeverity != NS_DEBUG_WARNING) {
     fprintf(stderr, "\07");
   }
 #endif
 
@@ -380,22 +380,24 @@ NS_DebugBreak(uint32_t aSeverity, const 
       return;
 
     case NS_DEBUG_ABORT: {
 #if defined(MOZ_CRASHREPORTER)
       // Updating crash annotations in the child causes us to do IPC. This can
       // really cause trouble if we're asserting from within IPC code. So we
       // have to do without the annotations in that case.
       if (XRE_IsParentProcess()) {
+        // Don't include the PID in the crash report annotation to
+        // allow faceting on crash-stats.mozilla.org.
         nsCString note("xpcom_runtime_abort(");
-        note += buf.buffer;
+        note += nonPIDBuf.buffer;
         note += ")";
         CrashReporter::AppendAppNotesToCrashReport(note);
         CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("AbortMessage"),
-                                           nsDependentCString(buf.buffer));
+                                           nsDependentCString(nonPIDBuf.buffer));
       }
 #endif  // MOZ_CRASHREPORTER
 
 #if defined(DEBUG) && defined(_WIN32)
       RealBreak();
 #endif
 #if defined(DEBUG)
       nsTraceRefcnt::WalkTheStack(stderr);