Most of this is simple, "Read the form, get the data, then
call ProcessAttribute."
Handling DW_FORM_implcit_const is a little trickier, as it
is the only form that stores its value inline in the abbrev
table itself. Add a test for that.
Print errors for supplementary object files.
Change-Id: I0999b039848bded1891998a866e5059acd538a09
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2446627
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
This is a big change. dwarf5 range lists are quite a bit more complicated
than dwarf 4 range lists, both in the contextual information required, and
in their own representation and interpretation.
The big design choice here is how to pass the CU information all the
way down to the reader. I chose a structure, because otherwise the
parameter list gets very long and error prone (and has to be passed
down several levels). This structure could be made a parto of the CU
context itself, or the range handler, so it wouldn't have to be
separately assembled at range-list read time, but both of those
solutions get even more invasive, and harder to follow.
I've tried to figure out how to break this into smaller changes, but it
affects nearly everything that has to do with a compilation unit's
own addresses and when decisions must be made about how to read them.
Dependency injection will do that to you.
It does add tests for range list reading, which did not exist before.
Change-Id: I923b9a2c3379a0f52609bc05310097de5cbb7227
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2446635
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
CFI might compute invalid rsp/rip values if the values in the callee
frame were corrupted, as in stack overflow. Rejecting the frame
computed by CFI allows Breakpad to fall-back to scanning.
Bug: b/169611285
Change-Id: Ifeb08ab5639932c0e23722a161d9d15403738019
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2456037
Reviewed-by: Mark Mentovai <mark@chromium.org>
This reverts commit 2b936b06c1.
After getting deep into the dwarf5 range reader, I realized that this
should be done a somewhat different way. So reverting in favor or
a better design, coming in a few minutes.
Change-Id: Ie0b2846e70b3df1e637831e96ea69fe093f4e712
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2446011
Reviewed-by: Mark Mentovai <mark@chromium.org>
Dwarf5 introduces a new .debug_rngslist section, to take the place
of the Dwarf4 .debug_ranges. However, the dwarf version is CU-based,
and not file-based, so there can be both sections, and which section
the CU needs isn't known until the dwarf parser encounters either
DW_AT_ranges (dwarf 4 and lower) or DW_AT_rnglists_base (dwarf 5).
This change refactors the code around range lists and range list
readers to defer the decision of what section to parse until
the relevant attribute is found. It moves the range list section
reader from the range-list handler itself (which doesn't know which
section it will use) to the CU context, and then lets the handler
know when it encounters DW_AT_ranges.
I will add a reader for the new dwarf5 section, along with the code to
interpret the new section, and its forms and such in a subsequent patch.
Change-Id: Ie92e4c9daa3f0acb98d7ef74f6b9c2065db849b1
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2433684
Reviewed-by: Mark Mentovai <mark@chromium.org>
- Mac OS symupload used to check for errors in JSON serialization by
inspecting the "error" out parameter of the serialization function. Now
it checks the returned data for "nil".
- Similar change for the HTTP request that's made in the same function.
Change-Id: I86f50ef44e60ee119c302e0614b115a8d35e9b5b
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2390753
Reviewed-by: Mark Mentovai <mark@chromium.org>
We have API's for copying files & changing file modes, so there's
no sense in using system() to run programs to do that.
For the strip call, do the minimal spawn+wait dance. This avoids
weird quoting string issues at least.
Change-Id: Ibda117f243e886c0c7fcf8076fb8602b8d3ba42d
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2396558
Reviewed-by: Mark Mentovai <mark@chromium.org>
Breakpad should only map the file content after the offset instead of
the whole file. Mapping the whole file while only unmap 'file_len - offset'
would leak 'offset' bytes of mapping in virtual memory.
Change-Id: I10be4f929d969703a6a02c1f709d2a4ca86a6e9e
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2393468
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Some vendor library doesn't contain the section header. It causes
segmentation fault in `FindElfClassSection`.
> e_shoff:
> This member holds the section header table's file offset in bytes.
> If the file has no section header table this member holds zero.
Change-Id: Id98d6ff3bd16af4541deb5a55a8fad2fa74eda23
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2354427
Reviewed-by: Mark Mentovai <mark@chromium.org>
Each stackwalker subtracts the size of an instruction
from a frame's instruction pointer to determine which
instruction it was executing. This should also be done
for pointers examined while scanning for likely return
addresses to ensure that those pointers don't point
past the end of functions.
Bug: b/118634446
Change-Id: I043e3f1e51a2c0a3d99ed14bf18ea64dc98add44
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2356649
Reviewed-by: Mark Mentovai <mark@chromium.org>
- Attempt to escape all characters which must be escaped in a URL or JSON string, for debug_file, since almost all of these are legal filename characters.
Change-Id: Ic7a9c1aef00093d164683be7db84f4f282f45f7a
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2339706
Reviewed-by: Mark Mentovai <mark@chromium.org>
- This change should also be made for other platforms.
- This allows users to tell the difference between upload succeeding, failing, and being skipped because the file already exists on server.
Change-Id: I0b404da7aac29e0a16346bbd816ad1c815985bce
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2341373
Reviewed-by: Mark Mentovai <mark@chromium.org>
- Checking symbol status would fail for files whose names contained spaces, because the file name was being put in the URL unescaped.
- Now the filename is escaped before being put in URL when checking symbol status.
Change-Id: I3b989d877e0fd9aef57ec13bdbbb6c3dacb6a9e2
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2339782
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
These fields are only used when NDEBUG is not defined. We get build
failures with the current code like:
src/common/dwarf/dwarf2reader.h:181:12: error: private field 'string_buffer_length_' is not used [-Werror,-Wunused-private-field]
uint64_t string_buffer_length_;
Change-Id: I01d6e29d31d50e29ed3736e7197e70455724ae48
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2299022
Reviewed-by: Sterling Augustine <saugustine@google.com>
We do this in a lot of places, but we're inconsistent.
Normalize the code to the Google C++ style guide.
Change-Id: Ic2aceab661ce8f6b993dda21b1cdf5d2198dcbbf
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2262932
Reviewed-by: Sterling Augustine <saugustine@google.com>
Reviewed-by: Mark Mentovai <mark@chromium.org>
For the upcoming Dwarf5 LineInfo changes, we will need to get
several additional sections by name, but without a the Compilation
Unit. This change prepares for that.
Change-Id: I566855abb339a856110a2f7d243d3848fe2b3c18
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2268861
Reviewed-by: Mark Mentovai <mark@chromium.org>
Given the almost nonexistent direct dwarfreader tests, I think
the best way to test these dwarf5 additions will be to add a full
dwarf5 compilation unit similar to the ones used incidentally in
the other tests. But I can't do that until enough dwarf5 is
correctly implemented.
Change-Id: I3418bda7212ae85c4b67232a2ab8fea9b9ca5d42
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2258838
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Adding the new forms by type and processing should avoid
the problems with 0c0e24f709,
where new forms weren't handled in switch statements, breaking
the build.
Testing this should follow the testing for DW_FORM_GNU_str_index,
very closely, but there doesn't appear to be any tests for that,
or even DW_FORM_strp.
Change-Id: I609d56b1dc879971bfef1070f063f8457fec6017
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2233839
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
This should address the issue where some Chrome builds were failing to
upload symbols due to a bad command-line flag, but there was no
indication of a problem, and no build failure, because symupload was
exiting with a success code.
BUG=1091387
R=nbilling@google.com, wuwang@google.com
Change-Id: I0d7f1a6d689ca5fd37be3abad4c5ebc97f108e50
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2231574
Reviewed-by: Nelson Billing <nbilling@google.com>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Bogus demangler warnings should be suppressed on both Mac and Linux
platforms, so there is no reason to keep this filter behind __APPLE__ gate.
Bug: chromium:1062556
Change-Id: Idf28db0b527c3cd6dd91510fcf7d9040aaa64694
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2210684
Reviewed-by: Mark Mentovai <mark@chromium.org>
On Mac a C++ symbol has 1-4 underscore characters followed by a 'Z'.
Symbols that do not have this format (such as plain C symbols)
causes a lot of warnings to be printed.
Bug: chromium:1062556
Change-Id: I55977f756c7e20cc5e7b1cb8e38316d7bf1f748c
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2179482
Reviewed-by: Mark Mentovai <mark@chromium.org>
These are left over from when this file was written in C.
In C++, true and false are keywords, so this is no longer necessary.
In fact, redefining a keyword is not actually permitted in ISO C++
(https://eel.is/c++draft/macro.names#2), and is rejected by MSVC.
Change-Id: I2f4a6eae7f8dd4d91b8ee6588c7ae5e8f20bd0d0
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2105831
Reviewed-by: Mike Frysinger <vapier@chromium.org>
__WORDSIZE is an internal libc definition. Instead, we can use
ULONG_MAX from limits.h, whose value corresponds to the machine's
native word size.
This allows us to remove the fallback definition of __WORDSIZE in
the Android compatibility headers.
Bug: google-breakpad:631
Change-Id: I7b9e6f3b2121f78ccad9e32bf26acac518aefd8f
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2107100
Reviewed-by: Mike Frysinger <vapier@chromium.org>
getcontext is also not available on musl libc, so generalize
breakpad_getcontext so it can be used as a fallback for non-Android
platforms as well.
On x86_64 and i386, ucontext_t uses an Android-specific offset for
storage of FP registers, since its sigset_t differs in size. So,
make the definition of MCONTEXT_FPREGS_MEM and UCONTEXT_FPREGS_MEM_OFFSET
conditional on whether we are building for Android.
On glibc and musl, signal.h and asm/sigcontext.h can't be included
together, so in breakpad_context_unittest.cc, only compare the libc
and kernel _fpstate when on Android.
Bug: google-breakpad:631
Change-Id: If81d73c4101bae946e9a3655b8d1c40a34ab6c38
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2102135
Reviewed-by: Mike Frysinger <vapier@chromium.org>
The crash address from the microdump was never checked against
anything. Instead, the test was checking the value of a constant.
On 32-bit systems, an intptr_t cannot represent kCrashAddress
(0xDEADDEAD), causing a failure when the crash address is parsed
from the microdump. Instead, use uintptr_t, which matches the type
of kCrashAddress.
Change-Id: Ib5612743803609f7801dcfb98deaa8779e362025
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2100816
Reviewed-by: Mike Frysinger <vapier@chromium.org>
The _libc_fpstate struct tag is an implementation detail of glibc,
and musl uses a different struct tag, _fpstate.
Unfortunately, the public type fpregset_t is a pointer type, so is
not suitable for allocating storage, or referring to constant
storage. Instead, we can use std::remove_pointer<fpregset_t>::type
to refer to the pointed-to type, regardless of the struct tag.
Bug: google-breakpad:631
Change-Id: Iaf47f15b2d834dd8de839431f65a481e9b0c7f9e
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2096171
Reviewed-by: Mike Frysinger <vapier@chromium.org>
__WORDSIZE is an internal libc definition. Instead, we can use
ULONG_MAX from limits.h, whose value corresponds to the machine's
native word size.
Bug: google-breakpad:631
Change-Id: If69caf578286d678585d1510c01562b969b5061f
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2097352
Reviewed-by: Mike Frysinger <vapier@chromium.org>
sys/poll.h and sys/signal.h just redirect to the standardized
location for these headers, poll.h and signal.h.
On musl libc, including the incorrect header path results in a
warning, and since breakpad is built with -Werror, this is an error.
In exception_handler.cc, signal.h is already included earlier, so
we can drop the sys/signal.h include.
Bug: google-breakpad:631
Change-Id: If36d326453e3267d38a5b92ed1301f828e46befe
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2097344
Reviewed-by: Mike Frysinger <vapier@chromium.org>
