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

Issue 9684002: [Linux] Force dump_syms to always read public symbols (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by walac
Modified:
9 years, 4 months ago
CC:
google-breakpad-dev_googlegroups.com
Base URL:
http://google-breakpad.googlecode.com/svn/trunk/src/
Visibility:
Public.

Description

If we extract symbols from an object with debug info, but some there is no
.debug_info for some public symbols, we miss them.

We now force dump_syms to read public symbol information after reading debug
info.

To avoid name collisions in the symbol file, we only add public symbols if they
don't have debug info.

Patch Set 1 #

Total comments: 3

Patch Set 2 : #

Patch Set 3 : nit: Breakpad's comments should all be written in third person. #

Total comments: 4

Patch Set 4 : Comment tweaks and logical fix #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M common/linux/dump_symbols.cc View 1 2 3 2 chunks +35 lines, -38 lines 1 comment Download
M common/module.cc View 1 2 3 1 chunk +15 lines, -4 lines 0 comments Download

Messages

Total messages: 10
Ted Mielczarek
https://breakpad.appspot.com/9684002/diff/1/common/linux/dump_symbols.cc File common/linux/dump_symbols.cc (right): https://breakpad.appspot.com/9684002/diff/1/common/linux/dump_symbols.cc#newcode715 common/linux/dump_symbols.cc:715: if (!found_debug_info_section || options.read_public_symbols) { The way you've modified ...
9 years, 7 months ago #1
walac
On 2014/08/29 00:31:31, Ted Mielczarek wrote: > https://breakpad.appspot.com/9684002/diff/1/common/linux/dump_symbols.cc > File common/linux/dump_symbols.cc (right): > > https://breakpad.appspot.com/9684002/diff/1/common/linux/dump_symbols.cc#newcode715 ...
9 years, 7 months ago #2
Ted Mielczarek
This needs a few comment changes and one small logic fix and it should be ...
9 years, 6 months ago #3
walac
On 2014/09/10 18:44:57, Ted Mielczarek wrote: > This needs a few comment changes and one ...
9 years, 6 months ago #4
Ted Mielczarek
LGTM, I'll land this.
9 years, 5 months ago #5
Lei Zhang (chromium)
On 2014/10/29 19:26:02, Ted Mielczarek wrote: > LGTM, I'll land this. walac: A kind reminder ...
9 years, 5 months ago #6
Ted Mielczarek
On 2014/10/29 22:36:22, Lei Zhang (chromium) wrote: > walac: A kind reminder to sign the ...
9 years, 5 months ago #7
Lei Zhang (chromium)
On 2014/10/29 22:47:08, Ted Mielczarek wrote: > On 2014/10/29 22:36:22, Lei Zhang (chromium) wrote: > ...
9 years, 5 months ago #8
Lei Zhang (chromium)
https://breakpad.appspot.com/9684002/diff/160001/common/linux/dump_symbols.cc File common/linux/dump_symbols.cc (right): https://breakpad.appspot.com/9684002/diff/160001/common/linux/dump_symbols.cc#newcode746 common/linux/dump_symbols.cc:746: return found_debug_info_section; This seems wrong. I build dump_syms and ...
9 years, 4 months ago #9
Lei Zhang (chromium)
9 years, 4 months ago #10
Message was sent while issue was closed.
On 2014/11/05 19:22:44, Lei Zhang (chromium) wrote:
> https://breakpad.appspot.com/9684002/diff/160001/common/linux/dump_symbols.cc
> File common/linux/dump_symbols.cc (right):
> 
>
https://breakpad.appspot.com/9684002/diff/160001/common/linux/dump_symbols.cc...
> common/linux/dump_symbols.cc:746: return found_debug_info_section;
> This seems wrong. I build dump_syms and now it no longer follows the
> .gnu_debug_link. See delete comment in the previous version at line 748.

Err, "deleted"
Sign in to reply to this message.

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