Extend grokdump.py with simple BreakPad symbol files support.

Extend grokdump.py with simple BreakPad symbol files support.


R=mstarzinger@chromium.org
BUG=

Committed: https://code.google.com/p/v8/source/detail?r=12620

[Michael Starzinger, 2012-09-19 09:54:32]

LGTM (with a few comments about the printed output and the usual nits).

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py
File tools/grokdump.py (right):

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:36: import disasm
I know you didn't touch, but can we alpha-sort the imports?

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:709: result = re.match(r"^FUNC ([a-f0-9]+) ([a-f0-9]+)
([a-f0-9]+) (.*)$", line)
More than 80 characters.

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:714: bisect.insort_left(self.symbols, FuncSymbol(baseaddr +
start, size, name))
More than 80 characters.

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:1770: print
Can we move this empty print to after the modules are printed?

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:1771: print "  Modules:"
Lowercase "modules:" would be consistent with the rest of the exception info.

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:1789: print reader.FindSymbol(reader.ExceptionIP())
This will print "None" when no symbol is found, can we skip printing in that
case? I think the "None" in the output is confusing.

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:1824: maybe_symbol)
Just use "maybe_symbol or ''" here and drop the if statement above.

[Vyacheslav Egorov (Google), 2012-09-26 12:48:57]

thanks for the review. 

addressed comments. landing.

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py
File tools/grokdump.py (right):

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:36: import disasm
On 2012/09/19 09:54:32, Michael Starzinger wrote:
> I know you didn't touch, but can we alpha-sort the imports?

Done.

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:709: result = re.match(r"^FUNC ([a-f0-9]+) ([a-f0-9]+)
([a-f0-9]+) (.*)$", line)
On 2012/09/19 09:54:32, Michael Starzinger wrote:
> More than 80 characters.

Done.

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:714: bisect.insort_left(self.symbols, FuncSymbol(baseaddr +
start, size, name))
On 2012/09/19 09:54:32, Michael Starzinger wrote:
> More than 80 characters.

Done.

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:1770: print
On 2012/09/19 09:54:32, Michael Starzinger wrote:
> Can we move this empty print to after the modules are printed?

I will add another one after modules. I like some empty lines, they make reading
easier.

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:1771: print "  Modules:"
On 2012/09/19 09:54:32, Michael Starzinger wrote:
> Lowercase "modules:" would be consistent with the rest of the exception info.

Done.

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:1789: print reader.FindSymbol(reader.ExceptionIP())
On 2012/09/19 09:54:32, Michael Starzinger wrote:
> This will print "None" when no symbol is found, can we skip printing in that
> case? I think the "None" in the output is confusing.

Done.

https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newc...
tools/grokdump.py:1824: maybe_symbol)
On 2012/09/19 09:54:32, Michael Starzinger wrote:
> Just use "maybe_symbol or ''" here and drop the if statement above.

Done.