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

Issue 180001: Make Windows dump_syms output a BINARY line with code_file and code_identifier (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 3 months ago by Ted Mielczarek
Modified:
9 years, 2 months ago
Reviewers:
Mark Mentovai, jimb
CC:
google-breakpad-dev_googlegroups.com
Visibility:
Public.

Description

I have two things I'd like to fix, and both of them require knowing the code
file and code identifier of binary files. These are easy to get when you have
the exe file, and outputing them from dump_syms seems like a natural fit. The
only downside is that I can't easily shoehorn them into the MODULE line, since
the "debug filename" field there can contain spaces, so I've invented a new line
that I named BINARY. It's not perfect, since anything that parses symbols needs
to be uploaded to know about it, but I don't think it's that bad.

I needed to use wcstombs, so I added a safe_wcstombs to WindowsStringUtils.
While I was there I refactored safe_mbstowcs to use a vector<wchar_t> instead of
heap allocation.

This patch also fixes basic_source_line_resolver to skip BINARY lines and adds a
BINARY line to one of the test files to ensure that it works.

Specifically, the two things I want to fix using this are:
http://code.google.com/p/google-breakpad/issues/detail?id=389 - Teach symbol
supplier to locate symbols in absence of CodeView record
https://bugzilla.mozilla.org/show_bug.cgi?id=528092 - Supply binaries
(firefox.exe and DLLs) from symbol server

Patch Set 1 #

Total comments: 15

Patch Set 2 : Updated with review comments #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M src/common/windows/pdb_source_line_writer.cc View 1 6 chunks +119 lines, -5 lines 1 comment Download
M src/common/windows/pdb_source_line_writer.h View 1 4 chunks +32 lines, -0 lines 0 comments Download
M src/common/windows/string_utils-inl.h View 1 chunk +3 lines, -0 lines 0 comments Download
M src/common/windows/string_utils.cc View 1 2 chunks +50 lines, -9 lines 0 comments Download
M src/processor/basic_source_line_resolver.cc View 1 1 chunk +4 lines, -0 lines 0 comments Download
M src/processor/testdata/module1.out View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/tools/windows/dump_syms/dump_syms.vcproj View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 8
Ted Mielczarek
9 years, 3 months ago #1
jimb
Be sure to update the symbol file format description on the wiki.
9 years, 3 months ago #2
Mark Mentovai
http://breakpad.appspot.com/180001/diff/1/2 File src/common/windows/pdb_source_line_writer.cc (right): http://breakpad.appspot.com/180001/diff/1/2#newcode511 Line 511: fprintf(output_, "BINARY %ws %ws\n", BINARY isn’t a terribly ...
9 years, 3 months ago #3
jimb
http://breakpad.appspot.com/180001/diff/1/2 File src/common/windows/pdb_source_line_writer.cc (right): http://breakpad.appspot.com/180001/diff/1/2#newcode511 Line 511: fprintf(output_, "BINARY %ws %ws\n", Or "CODE-IDENTIFIER"? It's not ...
9 years, 3 months ago #4
Ted Mielczarek
http://breakpad.appspot.com/180001/diff/1/2 File src/common/windows/pdb_source_line_writer.cc (right): http://breakpad.appspot.com/180001/diff/1/2#newcode511 Line 511: fprintf(output_, "BINARY %ws %ws\n", On 2010/09/01 17:03:47, Mark ...
9 years, 3 months ago #5
Ted Mielczarek
Ping? I think I addressed all your review comments.
9 years, 2 months ago #6
Mark Mentovai
Yes, you did. I was hoping we could land the processor-side stuff first and then ...
9 years, 2 months ago #7
Ted Mielczarek
9 years, 2 months ago #8
http://breakpad.appspot.com/180001/diff/10001/11001
File src/common/windows/pdb_source_line_writer.cc (right):

http://breakpad.appspot.com/180001/diff/10001/11001#newcode805
Line 805: // PrintPEInfo();
This patch has the PrintPEInfo line commented out, so it won't change the output
format, but does change the processor. We can give some time to adapt and then I
can uncomment this line (a patch I have in my local patch queue) later.
Sign in to reply to this message.

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