Bug 1062709 (part 2, attempt 2) - Clean up stack printing and fixing. r=dbaron.

--HG--
extra : rebase_source : 626fd23a14ec90cfc9807c3d555169ec6463d19d

Author: nnethercote

security/sandbox/win/src/warnonlysandbox/wosCallbacks.h

@@ -66,8 +66,9 @@ StackFrameToOStringStream(uint32_t aFrameNumber, void* aPC, void* aSP,
   nsCodeAddressDetails details;
   char buf[1024];
   NS_DescribeCodeAddress(aPC, &details);
-  NS_FormatCodeAddressDetails(aFrameNumber, aPC, &details, buf, sizeof(buf));
-  *stream << "--" << buf;
+  NS_FormatCodeAddressDetails(buf, sizeof(buf), aFrameNumber, aPC, &details);
+  *stream << "--" << buf << '\n';
+  stream->flush();
 }
 #endif
 

toolkit/xre/nsSigHandlers.cpp

@@ -63,8 +63,9 @@ static void PrintStackFrame(uint32_t aFrameNumber, void *aPC, void *aSP,
   nsCodeAddressDetails details;
 
   NS_DescribeCodeAddress(aPC, &details);
-  NS_FormatCodeAddressDetails(aFrameNumber, aPC, &details, buf, sizeof(buf));
-  fputs(buf, stdout);
+  NS_FormatCodeAddressDetails(buf, sizeof(buf), aFrameNumber, aPC, &details);
+  fprintf(stdout, "%s\n", buf);
+  fflush(stdout);
 }
 
 }

tools/rb/fix_linux_stack.py

@@ -4,18 +4,9 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-# This script uses addr2line (part of binutils) to process the output of
-# nsTraceRefcnt's Linux stack walking code.  This is useful for two
-# things:
-#  (1) Getting line number information out of
-#      |nsTraceRefcnt::WalkTheStack|'s output in debug builds.
-#  (2) Getting function names out of |nsTraceRefcnt::WalkTheStack|'s
-#      output on optimized builds (where it mostly prints UNKNOWN
-#      because only a handful of symbols are exported from component
-#      libraries).
-#
-# Use the script by piping output containing stacks (such as raw stacks
-# or make-tree.pl balance trees) through this script.
+# This script uses addr2line (part of binutils) to post-process the entries
+# produced by NS_FormatCodeAddress(), which on Linux often lack a function
+# name, a file name and a line number.
 
 import subprocess
 import sys
@@ -296,25 +287,20 @@ def addressToSymbol(file, address):
     cache[address] = result
     return result
 
-line_re = re.compile("^(.*) ?\[([^ ]*) \+(0x[0-9A-F]{1,8})\](.*)$")
-balance_tree_re = re.compile("^([ \|0-9-]*)(.*)$")
+# Matches lines produced by NS_FormatCodeAddress().
+line_re = re.compile("^(.*#\d+: )(.+)\[(.+) \+(0x[0-9A-Fa-f]+)\](.*)$")
 
 def fixSymbols(line):
     result = line_re.match(line)
     if result is not None:
-        # before allows preservation of balance trees
-        # after allows preservation of counts
-        (before, file, address, after) = result.groups()
+        (before, fn, file, address, after) = result.groups()
 
         if os.path.exists(file) and os.path.isfile(file):
-            # throw away the bad symbol, but keep balance tree structure
-            (before, badsymbol) = balance_tree_re.match(before).groups()
-
             (name, fileline) = addressToSymbol(file, address)
 
             # If addr2line gave us something useless, keep what we had before.
             if name == "??":
-                name = badsymbol
+                name = fn
             if fileline == "??:0" or fileline == "??:?":
                 fileline = file
 

tools/rb/fix_macosx_stack.py

@@ -4,16 +4,9 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-# This script uses atos to process the output of nsTraceRefcnt's Mac OS
-# X stack walking code.  This is useful for two things:
-#  (1) Getting line number information out of
-#      |nsTraceRefcnt::WalkTheStack|'s output in debug builds.
-#  (2) Getting function names out of |nsTraceRefcnt::WalkTheStack|'s
-#      output on all builds (where it mostly prints UNKNOWN because only
-#      a handful of symbols are exported from component libraries).
-#
-# Use the script by piping output containing stacks (such as raw stacks
-# or make-tree.pl balance trees) through this script.
+# This script uses |atos| to post-process the entries produced by
+# NS_FormatCodeAddress(), which on Mac often lack a file name and a line
+# number.
 
 import subprocess
 import sys
@@ -99,16 +92,14 @@ def cxxfilt(sym):
     cxxfilt_proc.stdin.write(sym + "\n")
     return cxxfilt_proc.stdout.readline().rstrip("\n")
 
-line_re = re.compile("^(.*) ?\[([^ ]*) \+(0x[0-9a-fA-F]{1,8})\](.*)$")
-balance_tree_re = re.compile("^([ \|0-9-]*)")
+# Matches lines produced by NS_FormatCodeAddress().
+line_re = re.compile("^(.*#\d+: )(.+)\[(.+) \+(0x[0-9A-Fa-f]+)\](.*)$")
 atos_name_re = re.compile("^(.+) \(in ([^)]+)\) \((.+)\)$")
 
 def fixSymbols(line):
     result = line_re.match(line)
     if result is not None:
-        # before allows preservation of balance trees
-        # after allows preservation of counts
-        (before, file, address, after) = result.groups()
+        (before, fn, file, address, after) = result.groups()
         address = int(address, 16)
 
         if os.path.exists(file) and os.path.isfile(file):
@@ -129,9 +120,6 @@ def fixSymbols(line):
                     name = cxxfilt(name)
                 info = "%s (%s, in %s)" % (name, fileline, library)
 
-            # throw away the bad symbol, but keep balance tree structure
-            before = balance_tree_re.match(before).groups()[0]
-
             nl = '\n' if line[-1] == '\n' else ''
             return before + info + after + nl
         else:

tools/rb/fix_stack_using_bpsyms.py

@@ -4,6 +4,10 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
+# This script uses breakpad symbols to post-process the entries produced by
+# NS_FormatCodeAddress(), which on TBPL builds often lack a file name and a
+# line number (and on Linux even the symbol is often bad).
+
 from __future__ import with_statement
 
 import sys
@@ -112,18 +116,14 @@ def addressToSymbol(file, address, symbolsDir):
   else:
     return ""
 
-line_re = re.compile("^(.*) ?\[([^ ]*) \+(0x[0-9A-F]{1,16})\](.*)$")
-balance_tree_re = re.compile("^([ \|0-9-]*)")
+# Matches lines produced by NS_FormatCodeAddress().
+line_re = re.compile("^(.*#\d+: )(.+)\[(.+) \+(0x[0-9A-Fa-f]+)\](.*)$")
 
 def fixSymbols(line, symbolsDir):
   result = line_re.match(line)
   if result is not None:
-    # before allows preservation of balance trees
-    # after allows preservation of counts
-    (before, file, address, after) = result.groups()
+    (before, fn, file, address, after) = result.groups()
     address = int(address, 16)
-    # throw away the bad symbol, but keep balance tree structure
-    before = balance_tree_re.match(before).groups()[0]
     symbol = addressToSymbol(file, address, symbolsDir)
     if not symbol:
       symbol = "%s + 0x%x" % (os.path.basename(file), address)

tools/rb/make-tree.pl

@@ -97,11 +97,16 @@ ($$$)
 
           my $cnt = shift(@fields);
 
-          # Collect the remaining lines to create a stack trace.
+          # Collect the remaining lines to create a stack trace. We need to
+          # filter out the frame numbers so that frames that differ only in
+          # their frame number are considered equivalent. However, we need to
+          # keep a frame number on each line so that the fix*.py scripts can
+          # parse the output. So we set the frame number to 0 for every frame.
           my @stack;
           CALLSITE: while (<$INFILE>) {
               chomp;
               last CALLSITE if (/^$/);
+              $_ =~ s/#\d+: /#00: /;    # replace frame number with 0
               $stack[++$#stack] = $_;
           }
 

xpcom/base/CodeAddressService.h

@@ -89,12 +89,9 @@ class CodeAddressService
 
       // Convert "" to nullptr.  Otherwise, make a copy of the name.
       StringAlloc::free(mFunction);
-      mFunction =
-        !aFunction[0] ? nullptr : StringAlloc::copy(aFunction);
+      mFunction = !aFunction[0] ? nullptr : StringAlloc::copy(aFunction);
       StringAlloc::free(mFileName);
-      mFileName =
-        !aFileName[0] ? nullptr : StringAlloc::copy(aFileName);
-
+      mFileName = !aFileName[0] ? nullptr : StringAlloc::copy(aFileName);
 
       mLibrary = aLibrary;
       mLOffset = aLOffset;
@@ -166,26 +163,9 @@ class CodeAddressService
 
     MOZ_ASSERT(entry.mPc == aPc);
 
-    uintptr_t entryPc = (uintptr_t)(entry.mPc);
-    // Sometimes we get nothing useful.  Just print "???" for the entire entry
-    // so that fix_linux_stack.py doesn't complain about an empty filename.
-    if (!entry.mFunction && !entry.mLibrary[0] && entry.mLOffset == 0) {
-      snprintf(aBuf, aBufLen, "??? 0x%" PRIxPTR, entryPc);
-    } else {
-      // Use "???" for unknown functions.
-      const char* entryFunction = entry.mFunction ? entry.mFunction : "???";
-      if (entry.mFileName) {
-        // On Windows we can get the filename and line number at runtime.
-        snprintf(aBuf, aBufLen, "%s (%s:%u) 0x%" PRIxPTR,
-                 entryFunction, entry.mFileName, entry.mLineNo, entryPc);
-      } else {
-        // On Linux and Mac we cannot get the filename and line number at
-        // runtime, so we print the offset in a form that fix_linux_stack.py and
-        // fix_macosx_stack.py can post-process.
-        snprintf(aBuf, aBufLen, "%s[%s +0x%" PRIXPTR "] 0x%" PRIxPTR,
-                 entryFunction, entry.mLibrary, entry.mLOffset, entryPc);
-      }
-    }
+    NS_FormatCodeAddress(aBuf, aBufLen, aFrameNumber, entry.mPc,
+                         entry.mFunction, entry.mLibrary, entry.mLOffset,
+                         entry.mFileName, entry.mLineNo);
   }
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const

xpcom/base/nsStackWalk.cpp

@@ -13,6 +13,10 @@
 
 #include "nsStackWalk.h"
 
+#ifdef XP_WIN
+#define snprintf _snprintf
+#endif
+
 using namespace mozilla;
 
 // The presence of this address is the stack must stop the stack walk. If
@@ -827,39 +831,6 @@ NS_DescribeCodeAddress(void* aPC, nsCodeAddressDetails* aDetails)
   return NS_OK;
 }
 
-EXPORT_XPCOM_API(nsresult)
-NS_FormatCodeAddressDetails(uint32_t aFrameNumber, void* aPC,
-                            const nsCodeAddressDetails* aDetails,
-                            char* aBuffer, uint32_t aBufferSize)
-{
-  if (aDetails->function[0]) {
-    _snprintf(aBuffer, aBufferSize, "%s+0x%08lX [%s +0x%016lX]",
-              aDetails->function, aDetails->foffset,
-              aDetails->library, aDetails->loffset);
-  } else if (aDetails->library[0]) {
-    _snprintf(aBuffer, aBufferSize, "UNKNOWN [%s +0x%016lX]",
-              aDetails->library, aDetails->loffset);
-  } else {
-    _snprintf(aBuffer, aBufferSize, "UNKNOWN 0x%016lX", aPC);
-  }
-
-  aBuffer[aBufferSize - 1] = '\0';
-
-  uint32_t len = strlen(aBuffer);
-  if (aDetails->filename[0]) {
-    _snprintf(aBuffer + len, aBufferSize - len, " (%s, line %d)\n",
-              aDetails->filename, aDetails->lineno);
-  } else {
-    aBuffer[len] = '\n';
-    if (++len != aBufferSize) {
-      aBuffer[len] = '\0';
-    }
-  }
-  aBuffer[aBufferSize - 2] = '\n';
-  aBuffer[aBufferSize - 1] = '\0';
-  return NS_OK;
-}
-
 // i386 or PPC Linux stackwalking code
 #elif HAVE_DLADDR && (HAVE__UNWIND_BACKTRACE || NSSTACKWALK_SUPPORTS_LINUX || NSSTACKWALK_SUPPORTS_MACOSX)
 
@@ -1102,25 +1073,6 @@ NS_DescribeCodeAddress(void* aPC, nsCodeAddressDetails* aDetails)
   return NS_OK;
 }
 
-EXPORT_XPCOM_API(nsresult)
-NS_FormatCodeAddressDetails(uint32_t aFrameNumber, void* aPC,
-                            const nsCodeAddressDetails* aDetails,
-                            char* aBuffer, uint32_t aBufferSize)
-{
-  if (!aDetails->library[0]) {
-    snprintf(aBuffer, aBufferSize, "UNKNOWN %p\n", aPC);
-  } else if (!aDetails->function[0]) {
-    snprintf(aBuffer, aBufferSize, "UNKNOWN [%s +0x%08" PRIXPTR "]\n",
-             aDetails->library, aDetails->loffset);
-  } else {
-    snprintf(aBuffer, aBufferSize, "%s+0x%08" PRIXPTR
-             " [%s +0x%08" PRIXPTR "]\n",
-             aDetails->function, aDetails->foffset,
-             aDetails->library, aDetails->loffset);
-  }
-  return NS_OK;
-}
-
 #else // unsupported platform.
 
 EXPORT_XPCOM_API(nsresult)
@@ -1154,13 +1106,44 @@ NS_DescribeCodeAddress(void* aPC, nsCodeAddressDetails* aDetails)
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
-EXPORT_XPCOM_API(nsresult)
-NS_FormatCodeAddressDetails(uint32_t aFrameNumber, void* aPC,
-                            const nsCodeAddressDetails* aDetails,
-                            char* aBuffer, uint32_t aBufferSize)
+#endif
+
+EXPORT_XPCOM_API(void)
+NS_FormatCodeAddressDetails(char* aBuffer, uint32_t aBufferSize,
+                            uint32_t aFrameNumber, void* aPC,
+                            const nsCodeAddressDetails* aDetails)
 {
-  aBuffer[0] = '\0';
-  return NS_ERROR_NOT_IMPLEMENTED;
+  NS_FormatCodeAddress(aBuffer, aBufferSize,
+                       aFrameNumber, aPC, aDetails->function,
+                       aDetails->library, aDetails->loffset,
+                       aDetails->filename, aDetails->lineno);
+}
+
+EXPORT_XPCOM_API(void)
+NS_FormatCodeAddress(char* aBuffer, uint32_t aBufferSize, uint32_t aFrameNumber,
+                     const void* aPC, const char* aFunction,
+                     const char* aLibrary, ptrdiff_t aLOffset,
+                     const char* aFileName, uint32_t aLineNo)
+{
+  const char* function = aFunction && aFunction[0] ? aFunction : "???";
+  if (aFileName && aFileName[0]) {
+    // We have a filename and (presumably) a line number. Use them.
+    snprintf(aBuffer, aBufferSize,
+             "#%02u: %s (%s:%u)",
+             aFrameNumber, function, aFileName, aLineNo);
+  } else if (aLibrary && aLibrary[0]) {
+    // We have no filename, but we do have a library name. Use it and the
+    // library offset, and print them in a way that scripts like
+    // fix_{linux,macosx}_stacks.py can easily post-process.
+    snprintf(aBuffer, aBufferSize,
+             "#%02u: %s[%s +0x%" PRIxPTR "]",
+             aFrameNumber, function, aLibrary, aLOffset);
+  } else {
+    // We have nothing useful to go on. (The format string is split because
+    // '??)' is a trigraph and causes a warning, sigh.)
+    snprintf(aBuffer, aBufferSize,
+             "#%02u: ??? (???:???" ")",
+             aFrameNumber);
+  }
 }
 
-#endif