Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2405)

Issue 55011: Breakpad Linux dumper: Add support for dumping DWARF CFI as STACK CFI records. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 2 months ago by jimb
Modified:
13 years, 11 months ago
Reviewers:
mochalatte
CC:
google-breakpad-dev_googlegroups.com
Visibility:
Public.

Description

Define a new DWARF parser class, dwarf2reader::CallFrameInfo.

Extend google_breakpad::Module to store and write out 'STACK CFI' records.

Define a new google_breakpad::DwarfCFIToModule class, to accept DWARF
CFI data from the parser and populate a Module with the equivalent
STACK CFI records.

Extend the Linux symbol dumping tool, dump_syms, to use
dwarf2reader::CallFrameInfo, google_breakpad::DwarfCFIToModule, and
google_breakpad::Module to extract DWARF CFI from the executable or
shared library files and write it to the Breakpad symbol file.

Define CFISection, a new class derived from TestAssembler::Section,
for use in creating DWARF CFI data for test cases.

Patch Set 1 #

Patch Set 2 : Breakpad Linux dumper: Add support for dumping DWARF CFI as STACK CFI records. #

Patch Set 3 : Breakpad Linux dumper: Add support for dumping DWARF CFI as STACK CFI records. #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
A src/common/dwarf/cfi_assembler.cc View 1 1 chunk +103 lines, -0 lines 2 comments Download
A src/common/dwarf/cfi_assembler.h View 1 1 chunk +149 lines, -0 lines 2 comments Download
M src/common/dwarf/dwarf2enums.h View 1 2 1 chunk +43 lines, -0 lines 0 comments Download
M src/common/dwarf/dwarf2reader.cc View 1 2 chunks +1227 lines, -2 lines 0 comments Download
M src/common/dwarf/dwarf2reader.h View 1 3 chunks +468 lines, -0 lines 0 comments Download
A src/common/dwarf/dwarf2reader_cfi_unittest.cc View 1 2 1 chunk +1928 lines, -0 lines 0 comments Download
M src/common/linux/dump_symbols.cc View 1 2 2 chunks +133 lines, -0 lines 2 comments Download
A src/common/linux/dwarf_cfi_to_module.cc View 1 2 1 chunk +187 lines, -0 lines 0 comments Download
A src/common/linux/dwarf_cfi_to_module.h View 1 chunk +154 lines, -0 lines 0 comments Download
A src/common/linux/dwarf_cfi_to_module_unittest.cc View 1 2 1 chunk +274 lines, -0 lines 0 comments Download
M src/common/linux/module.cc View 4 chunks +46 lines, -0 lines 2 comments Download
M src/common/linux/module.h View 4 chunks +59 lines, -6 lines 0 comments Download
M src/common/linux/module_unittest.cc View 3 chunks +110 lines, -2 lines 0 comments Download
M src/tools/linux/dump_syms/Makefile View 1 5 chunks +54 lines, -1 line 0 comments Download

Messages

Total messages: 7
jimb
This is the dumper-side support for DWARF CFI. Neal, if you'd like to delegate the ...
14 years, 2 months ago #1
jimb
Updated patch. More tests, some bug fixes, minor prep for .eh_frame support.
14 years, 1 month ago #2
jimb
Patch set 3 has the dumper provide a default rule for the DWARF CFI return ...
14 years, 1 month ago #3
mochalatte
still reviewing - it's a ton of code :-) http://breakpad.appspot.com/55011/diff/3001/4001 File src/common/dwarf/cfi_assembler.cc (right): http://breakpad.appspot.com/55011/diff/3001/4001#newcode55 Line ...
14 years, 1 month ago #4
mochalatte
LGTM
14 years ago #5
jimb
Committed, r550.
14 years ago #6
jimb
13 years, 11 months ago #7
Hi, Neal.  I came across a bunch of your comments on this patch from February
that I hadn't addressed.  I'll post a new patch to address them, but here are my
responses to your comments.

http://breakpad.appspot.com/55011/diff/3001/4001
File src/common/dwarf/cfi_assembler.cc (right):

http://breakpad.appspot.com/55011/diff/3001/4001#newcode55
Line 55: D64(0xffffffffffffffffULL);            // CIE distinguished value
Okay, I've done this.

http://breakpad.appspot.com/55011/diff/3001/4002
File src/common/dwarf/cfi_assembler.h (right):

http://breakpad.appspot.com/55011/diff/3001/4002#newcode34
Line 34: // cfi-assembler.h: Define CFISection, a class for creating properly
On 2010/02/23 00:31:40, mochalatte wrote:
> change to reflect filename

Fixed, thanks.

http://breakpad.appspot.com/55011/diff/3001/4007
File src/common/linux/dump_symbols.cc (right):

http://breakpad.appspot.com/55011/diff/3001/4007#newcode227
Line 227: {
On 2010/02/23 00:31:40, mochalatte wrote:
> move brace onto previous line

Fixed, thanks.

http://breakpad.appspot.com/55011/diff/3001/4011
File src/common/linux/module.cc (right):

http://breakpad.appspot.com/55011/diff/3001/4011#newcode162
Line 162: 0 > putc(' ', stream))
On 2010/02/23 00:31:40, mochalatte wrote:
> should this be fputc?

I don't think so.  According to the GNU C Library manual:

 -- Function: int putc (int C, FILE *STREAM)
     This is just like `fputc', except that most systems implement it as
     a macro, making it faster.  One consequence is that it may
     evaluate the STREAM argument more than once, which is an exception
     to the general rule for macros.  `putc' is usually the best
     function to use for writing a single character.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted