Commit Graph

1639 Commits

Author SHA1 Message Date
Leonard Mosescu
5f112cb174 A couple of minor fixes
1. Fixing ExceptionHandlerTest.FirstChanceHandlerRuns:
   exit() is not an async-signal-safe function (http://man7.org/linux/man-pages/man7/signal-safety.7.html)

2. Fixing entry point signature in minidump_dump
  Changed "const char* argv[]" to "char* argv[]" to match the standard entry point signature

3. Updating .gitignore to exclude unit test artifacts

Change-Id: I9662898d0bd97769621fb6476a720105821c60f0
Reviewed-on: https://chromium-review.googlesource.com/562356
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-07-11 17:24:47 +00:00
Eric Holk
66856d617b Fix asan builds
When rolling this into Chrome, we got compile failures due to
DoNullPointerDereference being undefined but the new FirstChanceHandlerRuns
tests depends on this and was still defined.

The fix is to only enable the FirstChanceHandlerRuns test on non-asan builds.

Bug:
Change-Id: I5a3da0a21e2d0dd663ffc01137496d16905293a6
Reviewed-on: https://chromium-review.googlesource.com/544186
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-06-22 17:17:43 +00:00
Eric Holk
1628d99f7b Add first chance exception handler API
This change adds the option for Breakpad hosts to register a callback
that gets the first chance to handle an exception. The handler will 
return true if it handled the exception and false otherwise.

The primary use case is V8's trap-based bounds checking support for
WebAssembly.

Bug:
Change-Id: I5aa5b87d1229f1cef905a00404fa2027ee86be56
Reviewed-on: https://chromium-review.googlesource.com/509994
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-06-19 18:23:41 +00:00
Mike Frysinger
c142362a6c roll lss deps
This should hopefully fix the ARMv8/IT fix to work on clang & gcc.

BUG=chromium:686903

Change-Id: Ib99f05a0cd8df2cb2d393e2ff951d3109cdb5f5b
Reviewed-on: https://chromium-review.googlesource.com/538213
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-06-16 04:05:08 +00:00
Peter Collingbourne
94b6309aec Merge exec and non-exec segments while merging executable bit.
The bfd and gold linkers create segments like this: r/x, r/w where
the r/x segment covers the start of the ELF file.

lld's segments look like this: r, r/x, r/w where the r segment covers
the start of the ELF file.

So we cannot rely on the location of the r/x to tell where the start
of the ELF is. But we can still rely on the r and r/x mappings being
adjacent. So what we do is when we see an r segment followed by an r/x,
merge the r into the r/x and claim that it is executable. This way,
the minidump writer will continue to see a single executable segment
covering the entire executable.

Testing: "make check" passes when breakpad is compiled with
lld compiled from trunk (requires bug fix from LLVM r303689).

Also patched change into chromium and tested these builds:

$ cat args.gn
is_chrome_branded = true
is_debug = false
is_official_build = true
use_lld = true
allow_posix_link_time_opt = false
is_cfi = false

$ cat args.gn
target_os = "android"
target_cpu = "arm"
is_debug = false
is_official_build = true
is_chrome_branded = true

With both builds breakpad_unittests passes and
chrome/chrome_modern_public_apk create good minidumps after navigating
to chrome://inducebrowsercrashforrealz (checked that minidump contains
stack trace entry for content::HandleDebugURL).

Bug: chromium:716484
Change-Id: Ib6ed3a8420b83acf4a5962843930fb006734cb95
Reviewed-on: https://chromium-review.googlesource.com/513610
Reviewed-by: Primiano Tucci <primiano@chromium.org>
2017-05-26 16:51:46 +00:00
Peter Collingbourne
08bea455d4 Teach the ELF parser to handle multiple PT_NOTE phdrs.
It is legal for an ELF to contain multiple PT_NOTEs, and that is in
fact what lld's output looks like.

Testing: "make check" and breakpad_unittests when patched into
chromium.

Bug: chromium:716484
Change-Id: I01d3f8679961e2cb7e789d4007de8914c6af357d
Reviewed-on: https://chromium-review.googlesource.com/513512
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Ted Mielczarek <ted@mielczarek.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-05-26 16:43:47 +00:00
Markus Stange
8880afb762 Make the cross-compilation glue for dump_syms Mac handle x86_64h.
x86_64h has a different cpusubtype from x86_64. The h is for Haswell.

BUG=

Change-Id: Icf884e5699fe120c12d13aa57cd62db5b69a2ce6
Reviewed-on: https://chromium-review.googlesource.com/457171
Reviewed-by: Ted Mielczarek <ted@mielczarek.org>
2017-05-26 16:03:39 +00:00
John Budorick
dc3ba60f0f Don't attempt to use PTRACE_GETREGS if it isn't defined.
Follow up to https://chromium-review.googlesource.com/c/484479/, which
does not compile on arm64.

Bug: chromium:725754
Change-Id: Iaa6fbc332564909a10e2602a1026c14fb25625f4
Reviewed-on: https://chromium-review.googlesource.com/515044
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-05-25 14:44:22 +00:00
Peter Collingbourne
fbfd41af5f Simplify ELF parser code.
The layout of Elf32_Nhdr and Elf64_Nhdr is the same, so remove
templating and code that extracts the elfclass from the ELF file.

Testing: "make check" and breakpad_unittests when patched into
chromium.

Bug: chromium:716484
Change-Id: I41442cfff48afc6ae1a5b604d22b67550a910376
Reviewed-on: https://chromium-review.googlesource.com/514450
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-05-25 00:14:08 +00:00
John Budorick
77305c3ae6 Wrap config.h include in HAVE_CONFIG_H.
Bug: breakpad:730
Change-Id: I5a24b96258e1114378061512239d3e18f3f753f0
Reviewed-on: https://chromium-review.googlesource.com/514283
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-05-24 20:44:03 +00:00
John Budorick
16f5b5f512 Use __NR_exit_group in MinidumpWriterTest.MinidumpStacksSkippedIfRequested.
Also adds waits for all child processes spawned in MinidumpWriterTest.

Bug: 725754
Change-Id: I3248925993dede2c113ab1989b322a9d9c8f24bd
Reviewed-on: https://chromium-review.googlesource.com/513480
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-05-24 16:05:24 +00:00
Lars Volker
ffe3e47865 Only use O_CLOEXEC on platforms that support it
Change a9fca58 made use of the O_CLOEXEC flag, which is not supported on
older Linux kernels. This change makes the use contingent on kernel
support.

Testing: I manually compiled breakpad on CentOS 5.8 running kernel
2.6.18-308.8.2.el5.centos.plusxen.

Bug: 730
Change-Id: I21dff928cfba3c156a56708913f65a0c7b5396a6
Reviewed-on: https://chromium-review.googlesource.com/498528
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-05-10 21:32:37 +00:00
Lars Volker
54a54702a1 Fix race in ExceptionHandler::GenerateDump()
When writing a minidump on Linux, we called clone() in
linux/handler/exception_handler.cc with the CLONE_FILES flag. If the
parent process died while the child waited for the continuation signal,
the write side of the pipe 'fdes' stayed open in the child. The child
would not receive a SIGPIPE and would wait forever.

To fix this, we clone without CLONE_FILES and then close the
read-side of fdes in the master before the ptrace call. That way, if the
master dies, the child will receive a SIGPIPE and will die, too.

To test this I added a sleep() call before SendContinueSignalToChild()
and then killed the master, manually observing that the child would die,
too.

Bug: 728
Change-Id: Ifd72de835a34e7d9852ae1a362e707fdc6c96c7e
Reviewed-on: https://chromium-review.googlesource.com/464708
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-05-10 18:02:37 +00:00
Andrew Ermakovich
67649c6185 Fixing breakpad on old linux kernel
Try to read the trace's registers by PTRACE_GETREGS if kernel doesn't support PTRACE_GETREGSET.

Bug:

Change-Id: I881f3a868789747ca217f22a93370c6914881f9a
Reviewed-on: https://chromium-review.googlesource.com/484479
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-04-21 17:28:03 +00:00
Gabriele Svelto
a9fca58305 Make minidump name generation on Linux as random as possible
This patch ensures that two crashes taken within the same second have
different minidump names. The random characters used in the minidump
filename are now read from /dev/urandom where possible or generated via
arc4random(). If neither is available we fall back to regular rand() but
mixing the address of an object to the current time when generating the
random seed to make it slightly less predictable.

BUG=681

Change-Id: I2e97454859ed386e199b2628d6b7e87e16481b75
Reviewed-on: https://chromium-review.googlesource.com/445784
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-04-12 18:51:24 +00:00
Robert Sesek
aa7115cfde Dump MH_DYLINKER images in upload_system_symbols.
Change-Id: I18291efe211f88ae0607a9055d027b520ef13291
Reviewed-on: https://chromium-review.googlesource.com/462676
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-03-30 03:31:20 +00:00
Tobias Sargeant
846b6335c5 Treat the process stack as the top of memory for free space histograms.
Because we can't determine the top of userspace mappable memory
directly, we rely on the fact that the process stack is allocated at the
top of the address space (minus some randomization). Anything after that
should not count as free space.

BUG=695382

Change-Id: I68453aac9732c2bd4b87236b234518068dec6640
Reviewed-on: https://chromium-review.googlesource.com/446100
Reviewed-by: Primiano Tucci <primiano@chromium.org>
2017-03-29 10:42:31 +00:00
Lars Volker
219d0b15c4 Increase maximum number of threads for minidump_stackwalk.
BUG=

Change-Id: I361d8812df7b2977fe2630289059d31c3c9a4cc3
Reviewed-on: https://chromium-review.googlesource.com/459010
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-03-28 19:41:00 +00:00
Mike Frysinger
7a8374fc4c minidump_dump: fix up init paths
Fix some build & test failures in the previous minidump_dump code.

BUG=chromium:598947

Change-Id: Ia8fce453265167368de96747a8a92af930e78245
Reviewed-on: https://chromium-review.googlesource.com/458881
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-03-25 01:37:18 +00:00
Mike Frysinger
e1b3620ec7 minidump_dump: dump stack memory like hexdump
The current stack output is one line byte string which is not easy for
humans to parse.  Extend the print mode to support a hexdump-like view
and switch to that by default.  Now we get something like:
Stack
00000000  20 67 7b 53 94 7f 00 00  01 00 00 00 00 00 00 00  | g{S...........|
00000010  00 70 c4 44 9a 25 00 00  08 65 7a 53 94 7f 00 00  |.p.D.%...ezS...|

BUG=chromium:598947

Change-Id: I868e1cf4faa435a14c5f1c35f94a5db4a49b6a6d
Reviewed-on: https://chromium-review.googlesource.com/404008
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-03-24 16:22:21 +00:00
Mike Frysinger
117aa25107 minidump_dump: add proper cli processing
In preparation for adding more flexibility to this tool, add a
proper parser for the command line flags.  This uses the style
as seen in other breakpad tools.

BUG=chromium:598947

Change-Id: I95495e6ca7093be34d0d426f98a6c22880ff24a3
Reviewed-on: https://chromium-review.googlesource.com/457019
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-03-24 16:21:48 +00:00
Tobias Sargeant
b37547bc70 add -Wunused-local-typedefs to the set of warning flags
This helps catch issues when rolling in Chromium.

Change-Id: I2e3f33df4a69b501d8c04799ee0ac03a4bed9ac9
Reviewed-on: https://chromium-review.googlesource.com/459477
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-03-24 16:13:31 +00:00
Gordana Cmiljanovic
eb6c296472 Remove unused typedef
This removes unused typedef left in change:
https://chromium-review.googlesource.com/c/447697/
and fixes error:
dump_symbols.cc:613:35: error: unused typedef 'Word' [-Werror,-Wunused-local-typedef]

Change-Id: Ib5a82cd8af9a58ebf173b0f338fa9ad341819ef3
Reviewed-on: https://chromium-review.googlesource.com/459518
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-03-24 14:02:02 +00:00
Tobias Sargeant
0b7c2ea698 Fix ASSERT_EQUAL that should have been ASSERT_EQ.
BUG=703599

Change-Id: I5623705edc41644495aa4f2389056d255e22da8e
Reviewed-on: https://chromium-review.googlesource.com/459617
Reviewed-by: Primiano Tucci <primiano@chromium.org>
2017-03-24 12:05:43 +00:00
Tobias Sargeant
97483928cc Don't generate minidump if crash thread doesn't ref principal mapping.
If the crashing thread doesn't reference the principal mapping we can
assume that not only is that thread uninteresting from a debugging
perspective, the whole crash is uninteresting. In that case we should
not generate a minidump at all.

BUG=703599

Change-Id: Ia25bbb8adb79d04dcaf3992c3d2474f3b9b1f796
Reviewed-on: https://chromium-review.googlesource.com/457338
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-03-23 15:16:01 +00:00
Joshua Peraza
6cfdde4b91 Sanity check frame pointer while stackwalking
BUG=

Change-Id: Ib9b0fd5ba7f829f8be8cf856ab371c6540279ee5
Reviewed-on: https://chromium-review.googlesource.com/458526
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2017-03-22 23:23:25 +00:00
Joshua Peraza
124035f4fb Use string instead of std::string
BUG=

Change-Id: I06d1a836f8ff59a6abb7e420cd35fe52610ce091
Reviewed-on: https://chromium-review.googlesource.com/457872
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2017-03-22 23:22:40 +00:00
Gordana Cmiljanovic
77411db895 [MIPS] Get (ptrace) value of $pc for a thread
This change is fixing LinuxPtraceDumperTest.SanitizeStackCopy
test case.

Change-Id: I1eb3becfd4b3660bc5529b5d2a5e35db0b6eb6e0
Reviewed-on: https://chromium-review.googlesource.com/458277
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-03-22 12:47:06 +00:00
Mike Frysinger
0a57d7a32b dump_context: arm: name the registers in the dump output
The current ARM minidump_dump output makes people remember or look up
how registers are mapped in the ISA.  Let's use human friendly names
instead so they don't have to.

Currently it looks like:
MDRawContextARM
  context_flags       = 0x40000006
  iregs[ 0]            = 0x3c48b000
  iregs[ 1]            = 0x3
  iregs[ 2]            = 0x20
  iregs[ 3]            = 0x0
  iregs[ 4]            = 0x1c
  iregs[ 5]            = 0x3c48b000
  iregs[ 6]            = 0x20
  iregs[ 7]            = 0x3c48b04c
  iregs[ 8]            = 0x39100611
  iregs[ 9]            = 0x1c
  iregs[10]            = 0x0
  iregs[11]            = 0xbe61c200
  iregs[12]            = 0xfb9c1fec
  iregs[13]            = 0xbe61bd28
  iregs[14]            = 0x39e19b1c
  iregs[15]            = 0x357dd74c
  cpsr                = 0x680b0010
  float_save.fpscr     = 0x0

Now it looks like:
MDRawContextARM
  context_flags        = 0x40000006
  r0                   = 0x3c48b000
  r1                   = 0x3
  r2                   = 0x20
  r3                   = 0x0
  r4                   = 0x1c
  r5                   = 0x3c48b000
  r6                   = 0x20
  r7                   = 0x3c48b04c
  r8                   = 0x39100611
  r9                   = 0x1c
  r10                  = 0x0
  r11                  = 0xbe61c200
  r12                  = 0xfb9c1fec
  sp                   = 0xbe61bd28
  lr                   = 0x39e19b1c
  pc                   = 0x357dd74c
  cpsr                 = 0x680b0010
  float_save.fpscr     = 0x0

BUG=chromium:665083

Change-Id: I46d87c4ff7303a7efcd60da1d0b67ae7a5465c8f
Reviewed-on: https://chromium-review.googlesource.com/457197
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2017-03-18 03:04:26 +00:00
Jon Turney
0c3b559653 Fix sporadic failure of InstructionPointerMemory test on Windows
If another memory region of interest (e.g. a thread stack) randomly happens
to lie immediately before the page allocated by this test, the memory
regions can be coalesced in the minidump generated.  Relax this test so it
correctly handles the case where the expected 256 bytes around the IP aren't
at the start of the minidump memory region.

Alternatively, that could be avoided by reserving the page before the page
used for this test, in which case this test is degenerate with
InstructionPointerMemoryMinBound and can be removed.

BUG=

Change-Id: Ib1bfb242b2c0acaa090df68334a02ac434ad880c
Reviewed-on: https://chromium-review.googlesource.com/456702
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-03-17 19:15:06 +00:00
Jon Turney
4bb0cd4743 Fix issues with Windows dump_syms_unittest
* Turn DumpSymsRegressionTest into a parameterized test so it's easier to
see which test file is failing

* Convert dump_syms_regtest.sym to DOS line endings, being careful to
preserve the required spaces at the end of 'STACK WIN' lines

* In test #4 (omap_reorder_bbs), since the .exe corresponding to the .pdb is
not present, no INFO line is generated in the .sym file.  Update .sym file.

* Stop collecting stderr from dump_syms.  Future work: perhaps it's worth
collecting stderr to compare with a different file to verify that "Couldn't
locate EXE or DLL file" is output when expected?

* Regenerate testdata for test #5 (dump_syms_regtest64), which currently
does not pass, seemingly due a mis-match in the PDB age between the .pdb
file and the .sym file.  Also add the .exe corresponding to the .pdb
present, to provide CFI

BUG=

Change-Id: I54fab866437c9e1bad3a5534cef4fe4b6ae47cd2
Reviewed-on: https://chromium-review.googlesource.com/453178
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-03-17 17:49:55 +00:00
Dragan Mladjenovic
796a6c9baf [MIPS]: Don't terminate stackwalk when $sp value doesn't change between frames
Currently on MIPS we accidentally terminate stackwalk if $sp value doesn't change between frames 
which results in incomplete callchain terminated at the point of first tailcall encountered.

Change-Id: I8f1ed1df958d8f0a9eb11fd7800062184d8f1ee2
Reviewed-on: https://chromium-review.googlesource.com/449755
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-03-16 15:59:05 +00:00
Jon Turney
7ec3caf6c7 Fix Windows client ExceptionHandlerTest tests
ExceptionHandlerTest.InvalidParameterMiniDumpTest and
ExceptionHandlerTest.PureVirtualCallMiniDumpTest both also exercise a
feature that if the MiniDumpWithFullMemory MINIDUMP_TYPE is used, both
UUID.dmp and UUID-full.dmp files are written.

This is currently broken, and requesting a minidump with
MiniDumpWithFullMemory MINIDUMP_TYPE fails, as the file handle for the full
dump is not set.

Call GenerateFullDumpFile() if MiniDumpWithFullMemory is requested, to
generate a filename for the full dump file and set the file handle.

Currently GenerateFullDumpFile() also generates another UUID for the full
dump filename, so also make the private method
MinidumpGenerator::GenerateDumpFilePath() idempotent (so the same UUID is
reused)

(Note that calling Generate(|Full)DumpFile() more than once is not
permitted, so there's no behaviour where this changed the UUID to preserve)

BUG=

Change-Id: I74304f38b398f53da1c24f368dedfba8463da9e5
Reviewed-on: https://chromium-review.googlesource.com/452978
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-03-11 20:58:29 +00:00
Roman Margold
dac2223398 iOS client identifies itself via URL params
For iOS apps, product and version information is 
now automatically provided as part of the crash 
report upload URL to allow for early rejections.

Change-Id: Ia19c490c38023f9e23ec8a537f7a203ff1e642d7
Reviewed-on: https://chromium-review.googlesource.com/436164
Reviewed-by: Roman Margold <rmargold@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-03-10 18:30:14 +00:00
George Kola
a784e84497 Use NSURLSession if the min version we support is iOS 7+.
Because many apps still support iOS 8, they were defaulting to
deprecated NSURLConnection even if the code ran on iOS 10.
NSURLConnection requires a run loop and hence the code did not
always upload if the queue ran on a thread without a Run Loop.
This should improve break pad uploads

BUG=

Change-Id: I7bff80ea977fd1ab13c8812ed933ef842dab417f
Reviewed-on: https://chromium-review.googlesource.com/451880
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-03-09 16:11:07 +00:00
Joshua Peraza
5dbd93a0f8 Fix segfault when interpreting bad debug_file
BUG=chromium:661037

Change-Id: Ia4da0bd9787c232a6a199cfdfccfbed60c2515c2
Reviewed-on: https://chromium-review.googlesource.com/450090
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-03-08 21:31:16 +00:00
Dragan Mladjenovic
32a9e03835 [MIPS]: Relax dwarf section loading to accept both PROGBITS and MIPS_DWARF section types
Change-Id: I0862d930d92687dee47daa8d4dc3a21524c1c893
Reviewed-on: https://chromium-review.googlesource.com/447697
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-03-07 16:09:40 +00:00
Maciej Pawlowski
d61d49b385 Fix dump_syms clang compilation on Windows
Clang complains about bad format strings (DWORD is an unsigned long, not
unsigned int) and signed/unsigned comparison.
This change is necessary for https://codereview.chromium.org/2712423002/

BUG=245456

Change-Id: I58da92d43d90ac535c165fca346ee6866dfce22e
Reviewed-on: https://chromium-review.googlesource.com/448037
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-03-02 13:32:02 +00:00
Mike Frysinger
8349b97378 roll lss deps
We want the updated ARMv8/IT fix in the clone code path that newer
versions of clang warn about.

Change-Id: Ibd2d301d94cd09affc4225d013547ee1e24fac82
Reviewed-on: https://chromium-review.googlesource.com/448001
Reviewed-by: Mark Seaborn <mseaborn@chromium.org>
2017-03-01 21:40:42 +00:00
Scott Graham
88e5b2c880 win: Set LargeAddressAware on symupload
This was set manually on Chrome's built binary before
https://codereview.chromium.org/2173533002 but wasn't added to the build
file.

After this change:

c:\src\breakpad\src\src>dumpbin /headers tools\windows\symupload\Release\symupload.exe | grep large
                   Application can handle large (>2GB) addresses

This change only affects x86 builds.

R=mark@chromium.org
BUG=chromium:696911

Change-Id: I8f1bd5535af242edde51e70c60cf33b6170855ea
Reviewed-on: https://chromium-review.googlesource.com/447780
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-02-28 19:27:24 +00:00
Tobias Sargeant
ccf03c13eb Improve stack sanitization unittests.
Rather than relying on the process stack having all the things that
should/shouldn't be sanitized, create synthetic stacks to test all of
the important cases.

BUG=664460

Change-Id: I959266390e94d6fb83ca8ef11ac19fac89e68c31
Reviewed-on: https://chromium-review.googlesource.com/446108
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-02-24 19:59:03 +00:00
Scott Graham
19af23e3c0 Handle ntdll only emitting PUBLIC at func entry
This handles a case encountered in ntdll.dll symbols for Windows 7,
where a PUBLIC would be emitted only for the entry point to the
function. The body of the function, however, is split in a PGO-ish
fashion to another remote location in the binary. Because of this, there
were large gaps in the RVA space that would be attributed to the "last"
function that happened to have an entry point before the gap. In
practice, something like this:

0x100 Func1
0x110 Func2
0x120 Func3
0x130 Func4
...
0x800 LaterFuncs

The bodies of Func1/2/3 tend to be implemented as a fast-path check,
followed by a jmp to somewhere in the range between 0x130 and 0x800.
Because no symbols are emitted for this range, everything is attributed
to Func4, causing crash misattribution.

In this CL, the change is: after emitting the entry point symbol, also
walk in the original OMAP entries through the untranslated binary, and
for each block until we resolve to a new symbol (via the same mechanism
as we found the entry point) emit another PUBLIC indicating that there's
another block that belongs to that symbol. This effectively breaks up
the "0x130 - 0x800" range above.

R=mark@chromium.org
BUG=chromium:678874

Change-Id: Ib3741abab2e7158c81e3e34bca4340ce4d3153a1
Reviewed-on: https://chromium-review.googlesource.com/446717
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-02-23 18:28:02 +00:00
Tobias Sargeant
4a7e088a27 Make stack sanitization elide pointers to non-executable mappings.
The address space of every Android Java process is approximately 50%
mapped, which means that sanitization tends to be ineffective because
most string fragments are plausibly pointers into some mapping.

For example, the zygote on 32 bit devices has the following mappings
made by dalvik and this covers all 4 byte strings starting with a
character between 0x13 and 0x52 (which includes all uppercase characters
up to and including 'R').

12c00000-12d16000
12d16000-32c00000
32c00000-32c01000
32c01000-52c00000

In order to perform stack unwinding we only need pointers into the stack
of the thread in question, and pointers to executable mappings. If we
reduce the set of considered mappings to those mappings alone, then only
~2% of the address space is left unelided.

BUG=664460

Change-Id: I1cc27821659acfb91d658f42a83a24c176505a88
Reviewed-on: https://chromium-review.googlesource.com/446500
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-02-23 17:47:47 +00:00
Tobias Sargeant
4af8174278 Use the correct PC when determining whether to skip storing a stack.
This addresses a bug in commit 049a1532 that meant that the PC of the
crashing thread was always used to determine whether to include a stack,
instead of using the PC of the thread in question.

BUG=664460

Change-Id: Idcbd5db751e5c00941a1be28607389961c0c75d7
Reviewed-on: https://chromium-review.googlesource.com/446499
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-02-23 17:47:07 +00:00
Mike Frysinger
4e82b6fa11 minidump: mark Read as override in derived classes
The base class here declares Read as virtual, so make sure it's
marked as override in the derived classes.  This fixes some build
errors with clang.

src/google_breakpad/processor/minidump.h:853:8: error:
  'Read' overrides a member function but is not marked 'override'
  [-Werror,-Winconsistent-missing-override]
  bool Read(uint32_t expected_size_);
       ^
src/google_breakpad/processor/minidump.h:153:16: note:
  overridden virtual function is here
  virtual bool Read(uint32_t expected_size) = 0;
               ^

Change-Id: Ie4e5fec097b7f37739433a9deb39e7ed60471461
Reviewed-on: https://chromium-review.googlesource.com/444385
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
2017-02-20 18:03:14 +00:00
Mike Frysinger
644e71591b travis: add a clang build
We rework the matrix a bit to avoid the implicit explosion of
duplicated results.

Change-Id: I9a2d91b3a6a55bf2843e0e90d59fe5710bd639c7
Reviewed-on: https://chromium-review.googlesource.com/444544
Reviewed-by: Ted Mielczarek <ted@mielczarek.org>
2017-02-17 16:53:16 +00:00
Tobias Sargeant
fd28a5bbe9 Fix compile errors arising from compiling breakpad with clang.
These compile errors occur when building the check target with:
CXX=clang++-3.8
CXXFLAGS="-Werror -Wconstant-conversion -g -O2 -std=c++11"

src/processor/stackwalker_mips.cc:60:9: error: comparison of constant
  18446744073709551615 with expression of type 'bool' is always false
  [Werror,-Wtautological-constant-out-of-range-compare]
        > 0xffffffffffffffff) {
        ^ ~~~~~~~~~~~~~~~~~~
src/processor/stackwalker_mips.cc:68:66: error: comparison of constant
  4294967295 with expression of type 'bool' is always false
  [-Werror,-Wtautological-constant-out-of-range-compare]
    if ((memory_ && memory_->GetBase() + memory_->GetSize() - 1) > 0xffffffff) {
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~

Change-Id: I29eed8f4a67b9feeb274aa1fc6c79a019135e8d6
Reviewed-on: https://chromium-review.googlesource.com/438445
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2017-02-17 03:18:05 +00:00
Mike Frysinger
6cc037526e autotools: move -W flags to configure detection
This lets us use the flags with clang, and to add more flags easily.

Change-Id: I51bb53ffd5ab6da769cdfb422a2c88442f1ff9ad
Reviewed-on: https://chromium-review.googlesource.com/441864
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2017-02-16 22:48:56 +00:00
Mike Frysinger
8ba59492ab libdisasm: add upstream/license details
The license file comes from the upstream libdisasm tarball/repo.

Change-Id: I04a4002db72f778dd67dbcd71d3b5d1205a8c21d
Reviewed-on: https://chromium-review.googlesource.com/441884
Reviewed-by: Ted Mielczarek <ted@mielczarek.org>
2017-02-14 11:07:42 +00:00
George Kola
96b79e9bf8 There is no need to use the main queue just for perform selector.
We were using the main queue to queue up a perform selector and then the code
[self sendStoredCrashReports] was immediately doing a dispatch_async.
This unnecessary thread switching is not needed.

We simplify the above logic and use dispatch_after to queue the block on
the
internal queue after a delay

Note that main queue is typically more loaded and it is better for
non-UI code
to not use the main queue. This may also help improve crash log upload.

This change also switches from @synchronized to dispatch_once as that is
faster
Reference:
http://googlemac.blogspot.com/2006/10/synchronized-swimming.html

BUG=

Change-Id: I81035149cbbf13a3058ca3a11e6efd23980f19ad
Reviewed-on: https://chromium-review.googlesource.com/441364
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-02-13 21:45:42 +00:00