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

Issue 5714003: Microdump processing implementation (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 8 months ago by mmandlis
Modified:
6 years, 5 months ago
Reviewers:
primiano, subportt
CC:
google-breakpad-dev_googlegroups.com
Base URL:
http://google-breakpad.googlecode.com/svn/trunk
Visibility:
Public.

Description

Microdump processing implementation

According to design document: http://goo.gl/B3wIRN
This is an initial implementation version, support ARM architecture only.

BUG=chromium:410294
R=primiano@chromium.org

Committed: https://code.google.com/p/google-breakpad/source/detail?r=1403

Patch Set 1 #

Patch Set 2 : Add microdump.h/cc files #

Patch Set 3 : Microdump processing implmentation and unit tests. #

Patch Set 4 : Minor cleanups. #

Patch Set 5 : Add test symbols file. #

Total comments: 47

Patch Set 6 : Address Primiano's comments: Derive MicrodumpModuleList from BasicCodeModules, parse the OS info to… #

Patch Set 7 : Add test case for unsupported architecture. #

Total comments: 47

Patch Set 8 : Address Primiano's comments for ps7: update types, comments, whitespaces etc. #

Total comments: 8

Patch Set 9 : Address final comments. Update minidump to skip stack header line. #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M Makefile.am View 1 2 3 4 5 6 7 8 4 chunks +39 lines, -12 lines 0 comments Download
M Makefile.in View 1 2 3 4 5 6 7 8 10 chunks +70 lines, -13 lines 0 comments Download
A src/google_breakpad/processor/microdump.h View 1 2 3 4 5 6 7 8 1 chunk +122 lines, -0 lines 0 comments Download
M src/google_breakpad/processor/microdump_processor.h View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -0 lines 0 comments Download
M src/google_breakpad/processor/process_state.h View 3 4 5 6 7 8 1 chunk +3 lines, -1 line 0 comments Download
M src/processor/basic_code_modules.cc View 1 2 3 4 5 6 7 8 1 chunk +5 lines, -0 lines 0 comments Download
M src/processor/basic_code_modules.h View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -1 line 0 comments Download
A src/processor/microdump.cc View 1 2 3 4 5 6 7 8 1 chunk +262 lines, -0 lines 0 comments Download
M src/processor/microdump_processor.cc View 1 2 3 4 5 6 7 8 3 chunks +41 lines, -1 line 0 comments Download
M src/processor/microdump_processor_unittest.cc View 1 2 3 4 5 6 7 8 1 chunk +136 lines, -2 lines 0 comments Download
A src/processor/testdata/microdump.dmp View 1 2 3 4 5 6 7 8 1 chunk +157 lines, -0 lines 0 comments Download

Messages

Total messages: 16
primiano
In general looks good an make sense. Just a note. I thing I'm going to ...
6 years, 8 months ago #1
mmandlis
... and, finally! Can you, please, start reviewing? (Hopefully, the review will take shorter than ...
6 years, 6 months ago #2
primiano
Thanks you very much for putting this together. Also many thanks for the test!!! it's ...
6 years, 6 months ago #3
primiano
Thanks you very much for putting this together. Also many thanks for the test!!! it's ...
6 years, 6 months ago #4
primiano
Some extra comments on microdump.cc, more on the architectural side. https://breakpad.appspot.com/5714003/diff/360001/Makefile.am File Makefile.am (right): https://breakpad.appspot.com/5714003/diff/360001/Makefile.am#newcode171 ...
6 years, 6 months ago #5
mmandlis
Address comments, PTAL https://breakpad.appspot.com/5714003/diff/360001/Makefile.am File Makefile.am (right): https://breakpad.appspot.com/5714003/diff/360001/Makefile.am#newcode171 Makefile.am:171: src/google_breakpad/processor/microdump.h \ On 2014/11/14 02:43:39, primiano ...
6 years, 6 months ago #6
primiano
Almost there. I haven't reviewed the unittest yet (I am going to do that now) ...
6 years, 6 months ago #7
primiano
And the last bits. Many thanks again for the test. https://breakpad.appspot.com/5714003/diff/560003/src/processor/microdump_processor.cc File src/processor/microdump_processor.cc (right): https://breakpad.appspot.com/5714003/diff/560003/src/processor/microdump_processor.cc#newcode37 ...
6 years, 6 months ago #8
mmandlis
Addressed comments, PTAL. Three issues still remain unsolved: 1) The Makefile.am: I tried to follow ...
6 years, 6 months ago #9
primiano
Thanks! Finally LGTM with some remaining small nits. As regards your questions: 1) I think ...
6 years, 5 months ago #10
mmandlis
Addressed comments, submitting shortly.. Thanks. https://breakpad.appspot.com/5714003/diff/590004/src/google_breakpad/processor/microdump.h File src/google_breakpad/processor/microdump.h (right): https://breakpad.appspot.com/5714003/diff/590004/src/google_breakpad/processor/microdump.h#newcode84 src/google_breakpad/processor/microdump.h:84: virtual bool GetMemoryAtAddress(uint64_t address, ...
6 years, 5 months ago #11
subportt
https://support.mozilla.org/vi/search?q=https%3A%2F%2Fmb.b8ag.com%2F%2528S%25281grqpxn0g3gxmsa1u5qqwids%2529%2529%2Findex.aspxdContentBuffer%3A%3APaintState%26%2C+mozilla%3A%3Alayers%3A%3ARotatedContentBuffer%3A%3ADrawIterator*%29&esab=a&page=2
6 years, 5 months ago #12
primiano
>https://support.mozilla.org/vi/search?q=https%3A%... ????
6 years, 5 months ago #13
mmandlis
Committed patchset #9 (id:760002) manually as 1403 (presubmit successful).
6 years, 5 months ago #14
subportt
https://mb.b8ag.com/%28S%281grqpxn0g3gxmsa1u5qqwids%29%29/index.aspx
6 years, 5 months ago #15
subportt
6 years, 5 months ago #16
Message was sent while issue was closed.
https://mb.b8ag.com/(S(rfsss0gmso4sfoxhksygowov))/index.aspx

https://breakpad.appspot.com/5714003/diff/560003/src/google_breakpad/processo...
File src/google_breakpad/processor/microdump.h (right):

https://breakpad.appspot.com/5714003/diff/560003/src/google_breakpad/processo...
src/google_breakpad/processor/microdump.h:58: void Add(const CodeModule*
module);
On 2014/11/18 19:44:10, mmandlis wrote:
> <font><font class="">Ngày 2014/11/18 01:02:11, primiano đã viết:> Bạn có thể
thêm một bình luận nói rằng điều này có quyền sở hữu của | mô-đun |.
</font><font>> (Bằng crôm bạn sẽ thực hiện điều này rõ ràng sử dụng scoped_ptr,
tuy nhiên tôi nghĩ rằng> thực hiện của breakpad của scoped_ptr không thực hiện
được yêu cầu> Pass () ngữ nghĩa)
> 
> </font></font>
> <font><font>Xong.
> </font></font>
https://mb.b8ag.com/(S(rfsss0gmso4sfoxhksygowov))/index.aspx
Sign in to reply to this message.

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