From 94adb30f77b545ea7831f9592f9b3b30a63fb558 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 1 Sep 2014 22:56:05 -0700 Subject: [PATCH] Bug 1062709 (part 2, attempt 2) - Clean up stack printing and fixing. r=dbaron. --HG-- extra : rebase_source : 626fd23a14ec90cfc9807c3d555169ec6463d19d --- .../win/src/warnonlysandbox/wosCallbacks.h | 5 +- toolkit/xre/nsSigHandlers.cpp | 5 +- tools/rb/fix_linux_stack.py | 28 ++--- tools/rb/fix_macosx_stack.py | 24 ++--- tools/rb/fix_stack_using_bpsyms.py | 14 +-- tools/rb/make-tree.pl | 7 +- xpcom/base/CodeAddressService.h | 30 +----- xpcom/base/nsStackWalk.cpp | 101 ++++++++---------- xpcom/base/nsStackWalk.h | 41 +++++-- xpcom/base/nsTraceRefcnt.cpp | 6 +- xpcom/glue/BlockingResourceBase.cpp | 2 +- 11 files changed, 119 insertions(+), 144 deletions(-) diff --git a/security/sandbox/win/src/warnonlysandbox/wosCallbacks.h b/security/sandbox/win/src/warnonlysandbox/wosCallbacks.h index 35df233d8921d..adbce0ba56222 100644 --- a/security/sandbox/win/src/warnonlysandbox/wosCallbacks.h +++ b/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 diff --git a/toolkit/xre/nsSigHandlers.cpp b/toolkit/xre/nsSigHandlers.cpp index 8d28b17293f98..149e5d1536954 100644 --- a/toolkit/xre/nsSigHandlers.cpp +++ b/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); } } diff --git a/tools/rb/fix_linux_stack.py b/tools/rb/fix_linux_stack.py index 2380c05fabc25..c330966f3b12e 100755 --- a/tools/rb/fix_linux_stack.py +++ b/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 diff --git a/tools/rb/fix_macosx_stack.py b/tools/rb/fix_macosx_stack.py index 74502f8ce8ec0..7d076d9b627ae 100755 --- a/tools/rb/fix_macosx_stack.py +++ b/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: diff --git a/tools/rb/fix_stack_using_bpsyms.py b/tools/rb/fix_stack_using_bpsyms.py index 65d3d9c7626dd..83715095efe15 100755 --- a/tools/rb/fix_stack_using_bpsyms.py +++ b/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) diff --git a/tools/rb/make-tree.pl b/tools/rb/make-tree.pl index f983dc90ca5c6..04f0d85341061 100755 --- a/tools/rb/make-tree.pl +++ b/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] = $_; } diff --git a/xpcom/base/CodeAddressService.h b/xpcom/base/CodeAddressService.h index db8e38bad6e08..8716db66e1998 100644 --- a/xpcom/base/CodeAddressService.h +++ b/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 diff --git a/xpcom/base/nsStackWalk.cpp b/xpcom/base/nsStackWalk.cpp index c372b84c226cb..d0e16ce381767 100644 --- a/xpcom/base/nsStackWalk.cpp +++ b/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 diff --git a/xpcom/base/nsStackWalk.h b/xpcom/base/nsStackWalk.h index ad47d4ffcba8e..addc032d7a790 100644 --- a/xpcom/base/nsStackWalk.h +++ b/xpcom/base/nsStackWalk.h @@ -118,9 +118,35 @@ NS_DescribeCodeAddress(void* aPC, nsCodeAddressDetails* aDetails); * these are not available, library and offset should be reported, if * possible. * - * @param aFrameNumber The frame number (starts at 1, not 0). + * Note that this output is parsed by several scripts including the fix*.py and + * make-tree.pl scripts in tools/rb/. It should only be change with care, and + * in conjunction with those scripts. + * + * @param aBuffer A string to be filled in with the description. + * The string will always be null-terminated. + * @param aBufferSize The size, in bytes, of aBuffer, including + * room for the terminating null. If the information + * to be printed would be larger than aBuffer, it + * will be truncated so that aBuffer[aBufferSize-1] + * is the terminating null. + * @param aFrameNumber The frame number. * @param aPC The code address. - * @param aDetails The value filled in by NS_DescribeCodeAddress(aPC). + * @param aFunction The function name. Possibly null or the empty string. + * @param aLibrary The library name. Possibly null or the empty string. + * @param aLOffset The library offset. + * @param aFileName The filename. Possibly null or the empty string. + * @param aLineNo The line number. Possibly zero. + */ +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); + +/** + * Format the information about a code address in the same fashion as + * NS_FormatCodeAddress. + * * @param aBuffer A string to be filled in with the description. * The string will always be null-terminated. * @param aBufferSize The size, in bytes, of aBuffer, including @@ -128,11 +154,14 @@ NS_DescribeCodeAddress(void* aPC, nsCodeAddressDetails* aDetails); * to be printed would be larger than aBuffer, it * will be truncated so that aBuffer[aBufferSize-1] * is the terminating null. + * @param aFrameNumber The frame number. + * @param aPC The code address. + * @param aDetails The value filled in by NS_DescribeCodeAddress(aPC). */ -XPCOM_API(nsresult) -NS_FormatCodeAddressDetails(uint32_t aFrameNumber, void* aPC, - const nsCodeAddressDetails* aDetails, - char* aBuffer, uint32_t aBufferSize); +XPCOM_API(void) +NS_FormatCodeAddressDetails(char* aBuffer, uint32_t aBufferSize, + uint32_t aFrameNumber, void* aPC, + const nsCodeAddressDetails* aDetails); #ifdef __cplusplus } diff --git a/xpcom/base/nsTraceRefcnt.cpp b/xpcom/base/nsTraceRefcnt.cpp index 44b62c3f8727e..c933f4600265d 100644 --- a/xpcom/base/nsTraceRefcnt.cpp +++ b/xpcom/base/nsTraceRefcnt.cpp @@ -936,8 +936,9 @@ PrintStackFrame(uint32_t aFrameNumber, void* aPC, void* aSP, void* aClosure) char buf[1024]; NS_DescribeCodeAddress(aPC, &details); - NS_FormatCodeAddressDetails(aFrameNumber, aPC, &details, buf, sizeof(buf)); - fputs(buf, stream); + NS_FormatCodeAddressDetails(buf, sizeof(buf), aFrameNumber, aPC, &details); + fprintf(stream, "%s\n", buf); + fflush(stream); } static void @@ -949,6 +950,7 @@ PrintStackFrameCached(uint32_t aFrameNumber, void* aPC, void* aSP, char buf[buflen]; gCodeAddressService->GetLocation(aFrameNumber, aPC, buf, buflen); fprintf(stream, " %s\n", buf); + fflush(stream); } #endif diff --git a/xpcom/glue/BlockingResourceBase.cpp b/xpcom/glue/BlockingResourceBase.cpp index 0c27c58c1629a..10a2aad7ff04c 100644 --- a/xpcom/glue/BlockingResourceBase.cpp +++ b/xpcom/glue/BlockingResourceBase.cpp @@ -185,7 +185,7 @@ BlockingResourceBase::Print(nsACString& aOut) const WalkTheStackCodeAddressService addressService; for (uint32_t i = 0; i < state.Length(); i++) { - const size_t kMaxLength = 4096; + const size_t kMaxLength = 1024; char buffer[kMaxLength]; addressService.GetLocation(state[i], buffer, kMaxLength); const char* fmt = " %s\n";