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

Issue 4674002: dwarf: Minimal support for DWARF expressions

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 1 month ago by mdempsky
Modified:
3 years, 1 month ago
CC:
google-breakpad-dev_googlegroups.com, jln
Base URL:
http://google-breakpad.googlecode.com/svn/trunk/
Visibility:
Public.

Description

dwarf: Minimal support for DWARF expressions

Needed for unwinding past signal handler frames.

BUG=476

Patch Set 1 #

Total comments: 14
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M src/common/dwarf/dwarf2reader.cc View 7 chunks +56 lines, -6 lines 7 comments Download
M src/common/dwarf/dwarf2reader.h View 2 chunks +38 lines, -4 lines 4 comments Download
M src/common/dwarf_cfi_to_module.cc View 3 chunks +41 lines, -11 lines 1 comment Download
M src/common/dwarf_cfi_to_module.h View 4 chunks +10 lines, -3 lines 2 comments Download

Messages

Total messages: 5
mdempsky
Still needs unit tests, but I wanted to check what you think of the general ...
3 years, 1 month ago #1
Lei Zhang (chromium)
I defer to Jim, who wrote most of this code. I've never played real life ...
3 years, 1 month ago #2
jimb
Yes, the general approach looks great. Looking forward to the tests. https://breakpad.appspot.com/4674002/diff/1/src/common/dwarf/dwarf2reader.cc File src/common/dwarf/dwarf2reader.cc (right): ...
3 years, 1 month ago #3
mdempsky
Thanks for the quick review! Some quick answers while I work on tests and fixing ...
3 years, 1 month ago #4
jimb
3 years, 1 month ago #5
https://breakpad.appspot.com/4674002/diff/1/src/common/dwarf/dwarf2reader.cc
File src/common/dwarf/dwarf2reader.cc (right):

https://breakpad.appspot.com/4674002/diff/1/src/common/dwarf/dwarf2reader.cc#...
src/common/dwarf/dwarf2reader.cc:1382: if (4 > left) return ReportIncomplete();
On 2014/09/22 23:30:37, mdempsky wrote:
> I wrote "4 > left" to mimic "case '4'" within ParseOperands, but it makes no
> difference to me.

Oh. Serves me right for indulging in nit-picking. It's up to you.
Sign in to reply to this message.

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