Commit Graph

1680 Commits

Author SHA1 Message Date
Mark Mentovai
f6fe7cc55a mac processor: Add mapping for EXC_BAD_ACCESS/KERN_CODESIGN_ERROR
This code indicates termination under “kill” semantics due to a module’s
code signature becoming invalid.

Bug: chromium:1023239
Change-Id: I3d453af9ef6c6925edcf9c08dbd78e563877522f
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1930177
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2019-11-22 17:52:59 +00:00
Sylvain Defresne
792e6b2143 Remove dependency of uploader.mm on GTMLogger
The file GTMLogger shipped with breakpad is a copy of the version
from google_toolbox_for_mac. Having uploader.mm depend on GTMLogger
causes pain to iOS projects that want to integrate both breakpad
and google_toolbox_for_mac.

Since the file uploader.mm mixed uses of fprintf and GTMLogger to
log errors and warning, convert it to only use fprintf to stderr.

Bug: none
Change-Id: I68313ccf6951676a2859f44225281813722096ba
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1911755
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-11-12 16:17:19 +00:00
Christopher Grant
5915ea929c Add fallthrough annotations on new C++ switches
A recent commit converted some source from C to C++. Chrome has
checks against unannotated switch case fallthroughs in C++ code, so the
converted source needs annotations.

Bug: 990190
Change-Id: Ib92435b4877be936f837928a70b552ec4975d42a
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1898429
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2019-11-05 03:39:54 +00:00
Mark Mentovai
8c62281f8b Revert "Remove use of "register" keyword, deprecated in C++17"
This reverts commit 07411862ea.

We were a bit overzealous in removing “register” here. Both clang and
GCC correctly disallow “register” as a storage class specifier in C++17
mode by producing an error in ordinary use. However, they require
“register” to be specified for explicit register variables, and do not
produce an error in this case.

Change-Id: I223f2652c6da4215d6e8788d902e767c94b8c29d
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1894875
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-11-01 03:49:20 +00:00
Joshua Peraza
5085b1d0df linux, client: set module name from DT_SONAME
3e56ef9d changed dump_syms to set the module name from DT_SONAME
expecting that clients were already using DT_SONAME when it was
present. The Breakpad client previously only used DT_SONAME as the name
for a module if it detected that it was likely mapped from a zip file.
This patch updates the Breakpad Linux client to always use the
DT_SONAME in minidumps if it's present.

Also included are changes to address comments that were missed from
that review.

Bug: 1016924
Change-Id: I4aae8c05e6793d4b0598049a8964ddd4cb0c6194
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1889231
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2019-10-30 17:54:00 +00:00
Christopher Grant
d27fd9ae7e linux, dump_syms: Make style consistent in module unittest
Make the following consistent within the file:

- 0x...ULL for unsigned literals.
- On pointers, put the asterisk with the type.

Context: Made a previous change and saw a mix of styles. Make it
consistent for the sake of visual consistency.

Bug: None
Change-Id: I0f80aedc0ca0295be3c70bd88822cbb82d0415ba
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1888434
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2019-10-29 20:30:25 +00:00
Christopher Grant
862c9f47ef linux, dump_syms: Filter module entries outside specified ranges
Partitioned libraries generated with lld and llvm-objcopy currently
contain a superset of debug information, beyond what applies to the
library itself. This is because objcopy cannot split up debug
information by partition - instead, it places a copy of all debug
information into each partition.

In lieu of potential future support for lld or objcopy becoming able to
split up debug information, let dump_syms do the next best thing:

- Find the address ranges of all PT_LOAD segments in the lib.
- Supply these to the Module being generated.
- Filter additions to the Module based on these ranges.

Bug: 990190
Change-Id: Ib5f279f42e3f6ea79eed9665efbcc23c3c5d25dc
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1884699
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2019-10-29 19:03:12 +00:00
Jake Ehrlich
17958ef62c Add options to set OS and filename
This allows Fuchsia to use dump_syms directly without a
postprocessing step.

Change-Id: I84507f8bedddfcdcdb237119457c8ddf8ac354d5
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1850718
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2019-10-24 22:05:33 +00:00
Joshua Peraza
3e56ef9d4e linux, dump_syms: set module name from DT_SONAME
The Breakpad and Crashpad clients will use an object's DT_SONAME as
the name for a module if it exists. Previously, linux dump_syms would
assume the basename of an input elf file matches that value, causing
symbol lookups to fail if they were mismatched. This patch updates
dump_syms to use DT_SONAME as the module name, if present.

Bug: 1016924
Change-Id: I5eff0cf06c703841df3fb552cb5a8e1e50a20c64
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1876763
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2019-10-24 18:59:59 +00:00
Mike Frysinger
db1cda2653 convert_UTF: rewrite in C++
This allows us to namespace the symbols properly.

Bug: google-breakpad:725
Change-Id: Iea8052547eef6c0acb299c1995781735c6d8994f
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1769236
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-09-04 20:25:23 +00:00
Mark Mentovai
07411862ea Remove use of "register" keyword, deprecated in C++17
"register" as a storage class specifier has been deprecated since C++11,
and has been removed from C++17 while remaining a reserved word. See
C++17 5.11 and C.4.3.

Change-Id: I2dbab8a7061cb680d902644d39ea1a7fbc930e5c
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1749329
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2019-08-12 18:36:48 +00:00
Mark Mentovai
01dfa81f1b Mac dump_syms: accept __DWARF segment without __debug_info section
A .dSYM may validly contain a __DWARF segment without any __debug_info
section. This can occur for Chromium Framework in a component build of
Chromium, because in that case, all of the code is in other libraries
that Chromium Framework depends on.

This was previously tested by an assertion, but the assertion did not
trigger in NDEBUG (release) builds. In NDEBUG builds, this condition
would lead to an out-of-bounds read, detected by AddressSanitizer.

Instead of an assertion, the check is now always done at runtime.
Instead of being fatal, it's now just a warning, because it's been
established that __DWARF without __debug_info can occur.

(In the Chromium case, it remains pointless to run dump_syms via the
"chrome_dump_syms" target on a component build, as it'll only attempt to
symbolize Chromium Framework, and not any of the libraries that Chromium
Framework depends on that actually contain the code.)

Bug: chromium:991206
Change-Id: I6c9c75f0be7901813e3eaae54aff38c1afe73ca9
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1741610
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2019-08-07 16:43:11 +00:00
Mike Frysinger
ef04c9c028 string_conversion: fix pointer math
Since target_ptr is of type uint16_t* already, we don't need to scale
the byte count as the language does that for us.  If it were void*, we
would need this code, but it's not.

In practice it's probably not a big deal due to how we preallocated
memory: when converting UTF8->UTF16, we'd reserve the same number of
code units, and UTF8 takes more code units per codepoint than UTF16,
so the UTF16 vector is always oversized.

When converting UTF32->UTF16, we also reserve the same number of
code units, but since one UTF32 code unit could require two UTF16
code units (for U+10000 codepoints and higher), we would probably
corrupt memory in the process.  The APIs in this module don't seem
to take into account that range in general, so for now I'm only
fixing the memory corruption.

Bug: google-breakpad:768
Change-Id: Ibfaea4e866733ff8d99b505e72c500bd40d11a74
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1732888
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-08-05 17:38:16 +00:00
Mike Frysinger
4beee493f6 guid_creater: include string.h for memcpy
Bug: google-breakpad:779
Change-Id: If0cfb036ee924178033c89d4dc3e2ce75ddd46f2
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1732887
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-08-04 04:11:55 +00:00
Mike Frysinger
f0d43bc16b minidump_processor: fix typo in BUS_ADRALN
Bug: google-breakpad:787
Change-Id: I9f7144b3456abb750142eae9f989133bf4f27885
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1734586
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-08-04 04:11:34 +00:00
Joshua Peraza
cfad51e954 arm64: strip PACs from link register values
Pointer authentication codes are used to validate pointers against
accidental or malicious modification by storing a hash of the address
and a secret value in the pointer's unused, upper bits. The exact
bits used may vary by implementation and depend on the size of the
virtual address space of the target system, and whether other tagged
pointer features are in use.

Apple has implemented PACs in the Apple A12.
https://developer.apple.com/documentation/security/preparing_your_app_to_work_with_pointer_authentication

The documented method of stripping PACs from a pointer is to call
ptrauth_strip(), which ultimately emits an `xpaci` instruction, but
this option isn't available to the Breakpad processor not running on
the device. Instead, this patch selects likely address bits from
link register values by examining the address range of loaded modules.

Change-Id: I054bd1a03605719937fc85dcc8d8b9fe927f44be
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1713650
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-07-23 20:02:05 +00:00
Ivan Penkov
4a6d7c70cc Improving the support for Fuchsia in Breakpad Processor.
Now decoding the OS name and exception codes for Fuchsia. Still not decoding exception flags (can be added later, if needed).

Change-Id: If66cb000828be18f0c1b35d1b1f52b3ca3e1fd67
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1699049
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-07-12 18:29:23 +00:00
Mark Mentovai
02dd8e0da7 Remove base_logging::LogMessage
This is deprecated in the downstream consumer that it was introduced to
support.

This undoes b41ad66e93.

Change-Id: I3e0fa068b134014700128d1d75cae6a20d591d12
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1687125
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2019-07-12 17:06:06 +00:00
Nelson Billing
c46151db0f Fix Windows symupload after PE-only MD refactor.
- I think I previously fixed this for dump_syms, but must have missed
the symupload one because it didn't have a gn target.

Change-Id: Ibf4daa0dc874f329c2ee7c7b3d4de1ee6bc68d13
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1682717
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-07-01 17:49:32 +00:00
Nelson Billing
9cafb376eb Explicitly LoadLibrary dbghelp.dll in symbol converter.
- This is a workaround to an issue with gn/clang build of Windows
symbol converter where dbghelp.dll is loaded from system32/syswow64
instead of alongside the process exe.
- Why do we care where dbghelp.dll is loaded from? Two considerations:
  1. dbghelp.dll will only load symsrv.dll from the directory where it
  resides.
  2. symsrv.dll requires a file called "symsrv.yes" to be in the
  directory where it resides in order to work with MS symbol stores.
Therefore if we load dbghelp.dll from syswow64, then we must also
ensure there is a symsrv.dll and symsrv.yes file in syswow64.

Change-Id: Ia283a2c11e276c855a48157aa7be77897af4b02e
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1680670
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2019-06-28 20:44:55 +00:00
Nelson Billing
a5a607b0b0 Fix clang build warnings/errors for windows symbol converter.
Change-Id: Ib7f6e37af1466b5bed3e7d2921e0d9774394ad1e
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1680056
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-06-27 20:02:30 +00:00
Nelson Billing
b62101dead Fix windows symbol converter blacklisting.
- Was attempting "full match" when we meant to do "partial match".

Change-Id: Ia748a7fc8707e11f44c205e57f218f5f4bbc5612
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1676936
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2019-06-25 23:24:24 +00:00
Nelson Billing
e1b5fef027 Fix dump_syms clang Windows build.
- Put FileHeaderMachineToCpuString definition in header.
- Remove extra semi-colon.

Change-Id: I726ad0f73c57908576414fd828052dff09f5c51f
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1673142
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-06-25 00:28:04 +00:00
Nelson Billing
c11646e643 Fix breakpad client build.
- Was broken by a rename in http upload util code.

Change-Id: I72c275fe45638c83e535901e79817893b00ee62e
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1673138
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2019-06-24 22:44:01 +00:00
Nelson Billing
6ca3f8bbe5 Add PE-only MD support to Windows symbol converter.
- Only 64-bit PEs supported.
- Re-add some scripts that were missed in initial move of code.
- Change msdia120.dll dependency to msdia140.dll.
- Add tests for Intel, AMD, and NVidia Microsoft Symbol Stores.
- Windows symbol converter now attempts to fall back to PE-only metadata
  when it fails to locate a PDB.
- Remove the 'binary' folder under converter_exe. Need to think more
  about how a deployment should look and what tool(s) to use in creating
  one.

Change-Id: I52e42cbe5e759874a25114c2483e8b50d73fdf77
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1670098
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2019-06-24 18:55:02 +00:00
Nelson Billing
c38725b70e Fix 'debug_file' in PESourceLineWriter.
- Add a #define to testing.gyp to avoid warnings about TR1 deprecation.
- PESourceLineWriter now reads debug_file from CodeView record instead
  of using code_file value.
- Updated PE-only MD reading unit test.

Change-Id: Ib4e6201df3e3fd651e160f310584b5a67b16c842
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1668347
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2019-06-19 23:16:50 +00:00
Nelson Billing
ee65a3c005 Fix symbol converter after PE-only MD refactor.
Change-Id: Ib6d3359f6a8ad7e0d2c2d6a4e92af37bc451db28
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1666353
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2019-06-18 23:22:46 +00:00
Nelson Billing
315ab8a927 Fix common_windows.gyp.
- Rename two files from '.cpp' to '.cc' and insert missing comma in
  between lines in source files list.

Change-Id: I169a738b565f52292cb0292b34c4397e9dc05370
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1666351
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2019-06-18 23:11:27 +00:00
Nelson Billing
92032389a6 Move Windows Symbol Converter to breakpad
- First step, this is just enough to get it generating a msbuild
  project with GYP, which in turn can build the executable.
- Tests need to be redesigned because there isn't an available server.

Change-Id: I45440fd32b3ede29666c127703bcd441f0e4288e
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1661134
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2019-06-18 21:58:05 +00:00
Mark Mentovai
a7d686086c Mac upload_system_symbols: make dump of /Library/QuickTime optional
/Library/QuickTime is gone in 10.15b2 19A487l.

Change-Id: I927350a9cb383b93e8b18aef5f36c77bb67fede1
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1663996
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2019-06-18 15:01:25 +00:00
Mark Mentovai
9575315d8b Mac upload_system_symbols: use log.Fatalf where formatting is desired
Change-Id: I5623da3109feeb4b80137fa67501b8fd7aa5c8ec
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1663995
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2019-06-18 14:48:57 +00:00
Nelson Billing
87bc402210 Enable PE-only metadata dumping for 64bit (aka. PE32+ format) PEs files.
- Implement in common_windows_lib-- added class "PESourceLineWriter".
    - Add command-line switch to tell dump_syms to use PESourceLineWriter.
      Symbol data created this way will contain information to correlate the
      module with ones found in minidumps, along with frame info that allows
      much higher quality stack-walking in those minidumps.
    - Significant refactor of PDBSourceLineWriter-- all code concerned with
      extracting metadata from PE files has been moved into utility
      functions. This is to allow sharing of this functionality with newly-
      added PESourceLineWriter.
    - Added a unit test to dump_syms for the PE-only scenario.

Change-Id: If0855f05d424d32d23f484995be5f34232179a37
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1525325
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2019-06-12 21:20:27 +00:00
Nelson Billing
548ca6e382 Port new symbol upload API to Windows symupload tool.
- CL for Linux change, including new documentation for API, at:
  https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1422400/3

Change-Id: I579744fec74c64757b8bc31de63d7a07ef9a0f1f
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1487982
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
2019-06-12 20:56:37 +00:00
Joshua Peraza
756daa536a Removed unused includes/declarations
Change-Id: I2ada9f96efe97c25c885eef561af725418a46bec
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1652667
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-06-11 23:36:23 +00:00
Joshua Peraza
d7676cd696 fix build failures
Change-Id: I9e745d2a4b3f780941ba286bbafa5d4169d172f7
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1653863
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-06-11 22:27:59 +00:00
Joshua Peraza
c2467077b9 Enable truncation of module ranges
ELF modules are loaded in memory in several, possibly discontiguous,
segments. If the holes between segments are large enough, other things,
possibly other ELF modules may be mapped in that space. Crashpad
records the range of modules as the base address of the lowest mapped
segment to the high address of the highest mapped segment. This means
that when one module is mapped into a hole in another, it appears to
the Breakpad processor as overlapping modules. Module ranges are
relevant to the Breakpad processor during stackwalking for identifying
which module a particular program counter belongs to (i.e. mapping the
address to a module's text segment). This patch addresses this issue of
overlapping modules by truncating the range of the module with the
lower base address. A typical module's text segment is the first loaded
segment which would leave the text segment range unaffected. Module
producers can restrict the size of holes in their ELF modules with the
flag "-Wl,-z,max-page-size=4096", preventing other modules from being
mapped in their address range.

Properly contemplating ELF module address ranges would require
extensions to the minidump format to encode any holes.
crbug.com/crashpad/298

This patch also renames the concept of "shrinking down" (which
truncated the upper of two overlapping ranges) to "truncate upper".

Change-Id: I4599201f1e43918db036c390961f8b39e3af1849
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1646932
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-06-11 19:04:02 +00:00
Joshua Peraza
6f287a5f57 linux: fix broken tests
Fixes errors for comparisons between signed and unsigned ints.

Change-Id: I7ef151ba84a48a0c8cc449cfaf12b9fef775d5a7
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1642361
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-06-04 00:19:16 +00:00
Ivan Penkov
e436b07330 Fix a build break with SDK 10.14.
* Updated the minimal target version from 10.5 to 10.7.
* Reference for the similar issue:
  https://github.com/uglide/RedisDesktopManager/issues/4284

Fix provided by Hiroyuki Komatsu.

Change-Id: Ie08f8f6084e66d439d8b6282f1f4734b2dfe778f
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1633390
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-05-29 16:53:09 +00:00
John Z Wu
d930308bbb Fix orphaned crash reports.
The path NSCachesDirectory may change across app updates and sometimes
even across app launches. As a result, the Config-XXX files may end up
with an outdated path to the associated minidump file.

Change-Id: I0befde26b2ac406c154ce7c7e9be0063ee99892d
Bug:850379
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1592561
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-05-15 03:47:37 +00:00
John Z Wu
b2831dbed1 Increase timeout of HTTPMultipartUpload from 10s to 60s.
This, hopefully, will reduce occurrences of UnfinishedReportUploads. It
will not eliminate it completely because uploads are never retried if
they fail.

60s is actually the default value. Before iOS6, 240s was the default and
minimum applied to requests with a body. Reference:
https://developer.apple.com/documentation/foundation/nsmutableurlrequest/1414063-timeoutinterval?language=objc

Bug:850379
Change-Id: I2f16fda7d7e8cbb8b8a6fc917111d9f646fbdad0
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1609876
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-05-13 20:24:15 +00:00
Mark Mentovai
21b48a72aa Linux breakpad_unittests: fix ThreadSanitizer problems
Some tests were failing because they had expectations about the number
of threads in a process, but TSan, and in some cases, ASan, introduce
their own threads. Where a sanitizer affects this, the expectations are
now used as minimum thread counts, not exact thread counts. See
https://www.brooklinen.com/blogs/brookliving/best-thread-count-for-sheets.

These problems were detected by ThreadSanitizer at
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8915151099544583616/+/steps/breakpad_unittests__with_patch_/0/stdout

Bug: chromium:949098
Change-Id: Ie40f1766bea27e9bcb112bf9e0b8b846fb343012
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1585948
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2019-04-26 18:44:54 +00:00
Mark Mentovai
9f90ceb904 Linux MinidumpWriter: fix stack-use-after-scope violation
One form of google_breakpad::WriteMinidump() passed MappingList and
AppMemoryList objects by reference to a MinidumpWriter object,
instantiating them directly as constructor parameters. The
MinidumpWriter stored these objects internally as references, and the
underlying objects went out of scope after MinidumpWriter construction.
The MinidumpWriter outlived them, causing a violation on any attempt to
access them following construction.

This bug was detected by AddressSanitizer at
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8915150848087289472/+/steps/breakpad_unittests__with_patch_/0/stdout

Bug: chromium:949098
Change-Id: I072ea9f1b64e1eae3e89d4a2b158764ff7970db5
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1585946
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2019-04-26 18:44:49 +00:00
Olivier Robin
1fc9cc0d0e [Breakpad iOS] Add a callback on report upload completion.
This CL adds a result callback on report upload completion.
On failure, Breakpad deletes the configuration file and does retry to
upload a report.
Using this callback, the client will be able to log some metrics and to
act on upload failure.

Bug: 954175
Change-Id: I95a3264b65d4c06ba5d8dde8377440d23f1e2081
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1572661
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-04-23 13:07:06 +00:00
Mark Mentovai
8c70c504b2 Make breakpad_unittests work with Chrome's test runner instead of gtest's
Chrome's test runner on Linux installs its own StackDumpSignalHandler
which swallows signals and doesn't re-raise them. This is sloppy, but
apparently there are reasons (https://crbug.com/551681). For
breakpad_unittests, it causes problems where a test process expects (via
waitpid()) to observe a child crash.  Deal with those cases by
explicitly restoring the default signal handler.

In another case, Chrome's test runner seems to have been arriving at the
conclusion that it was to expect output from a child. Transitioning from
exit() to _exit() fixes this problem, and it's not necessarily a bad
idea to do this in post-fork() children without an execve() anyway.

Bug: chromium:949098
Change-Id: I5a6af0c2a09cd8eac9998358f6d5ea665288236f
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1575670
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-04-19 21:42:27 +00:00
John Z Wu
ebab5970b0 Use correct c++ standard library in Xcode project.
This makes it build again for Xcode 10.

Change-Id: Ia28b8c0b131ac2862a8c17e223d1d7b1f0f59134
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1574177
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-04-19 14:19:05 +00:00
Mark Mentovai
232c45abee linux_core_dumper: add missing newline to Android test-disabled message
The [       OK ] LinuxCoreDumperTest.VerifyExceptionDetails line does
not appear at the beginning of a line, hiding it from Chromium’s test
infrastructure. This causes the test to have an unknown result, which is
treated as a failure.

https://ci.chromium.org/p/chromium/builders/try/android-kitkat-arm-rel/233129

Bug: google-breakpad:791
Change-Id: I0eb646a219fa40347db884fd28ace647328c5e49
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1558893
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2019-04-09 00:09:28 +00:00
Mike Frysinger
4d550cceca linux_core_dumper: partially disable test on Android
Running this test on android-kitkat-arm-rel fails for some reason:
[ RUN      ] LinuxCoreDumperTest.VerifyExceptionDetails
linux_core_dumper_unittest.cc:170: Failure
Expected: (0U) != (dumper.crash_address()), actual: 0 vs 0
linux_core_dumper_unittest.cc:178: Failure
Expected equality of these values:
  2U
    Which is: 2
  info.size()
    Which is: 0
[  FAILED  ] LinuxCoreDumperTest.VerifyExceptionDetails (7 ms)

Disable it for now on Android until someone can look into it.

Bug: google-breakpad:791
Change-Id: I40a5e3dbeeb44e5eb0df187e61d55e07d8ad3613
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1546778
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-04-07 14:08:29 +00:00
Mike Frysinger
9e9e236c82 mac dump_syms: remove unused vars to fix -Werror build failures
Bug: chromium:940823, chromium:946404
Change-Id: Ib8af7f0845b0488f36cd1b91326aba11bf283b0a
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1546779
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-04-05 00:25:48 +00:00
Mike Frysinger
e2b3b80e43 linux_core_dumper: add explicit casts for exception fields
Some of the fields we save might have signed types depending on the
system (e.g. `typedef int pid_t`).  Depending on the toolchain, we
can trip -Werror=narrowing failures like:
src/client/linux/minidump_writer/linux_core_dumper.cc:248:66: error:
  narrowing conversion of ‘(__pid_t)info->siginfo_t::_sifields.siginfo_t::<anonymous union>::_kill.siginfo_t::<anonymous union>::<anonymous struct>::si_pid’
  from ‘__pid_t {aka int}’ to ‘long unsigned int’ inside { } [-Werror=narrowing]
             set_crash_exception_info({info->si_pid, info->si_uid});
                                             ^^^^^^
src/client/linux/minidump_writer/linux_core_dumper.cc:252:71: error:
  narrowing conversion of ‘(int)info->siginfo_t::_sifields.siginfo_t::<anonymous union>::_sigsys.siginfo_t::<anonymous union>::<anonymous struct>::_syscall’
  from ‘int’ to ‘long unsigned int’ inside { } [-Werror=narrowing]
             set_crash_exception_info({info->si_syscall, info->si_arch});
                                             ^^^^^^^^^^

Since the exception info fields are all uint64_t which should be large
enough to handle all the fields in the siginfo_t structure, add casts
for all the assignments to avoid these errors.  We have implicit casts
even without them, so we aren't changing behavior.

Bug: google-breakpad:791
Bug: chromium:945653
Change-Id: Ib04e015998f08b857159ac13e9a065a66d228d49
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1544862
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-03-30 03:52:15 +00:00
Mark Mentovai
b4a0eb2d06 mac dump_syms: Support .dSYMs > 4GB (partially)
Even 64-bit Mach-O (MH_MAGIC_64 = 0xfeedfacf) is not a fully 64-bit file
format. File offsets in sections are stored in 32-bit fields, with
Mach-O writers typically truncating offsets too large to fit to just
their low 32 bits. When a section begins at a file offset >= 4GB,
dump_syms would produce an error such as:

Google Chrome Framework.dSYM/Contents/Resources/DWARF/Google Chrome Framework: the section '__apple_names' in segment '__DWARF' claims its contents lie outside the segment's contents

As a workaround, this implements the strategy I first described in
https://crbug.com/940823#c22.

Segment file offsets are stored in 64-bit fields. Because segments
contain sections and must load contiguously, it’s possible to infer a
section’s actual offset by computing its load address relative to its
containing segment’s load address, and treating this as an offset into
the containing segment’s file offset. For safety, this is only done for
64-bit segments (LC_SEGMENT_64) where the 32-bit section offset stored
in the Mach-O file is equal to the low (truncated) 32 bits of the
section offset recomputed per the above strategy.

Beware that this does not provide full “large file” support for 64-bit
Mach-O files. There are other file offsets within Mach-O files aside
from section file offsets that are stored in 32-bit fields even in the
64-bit format, including offsets to symbol table data (LC_SYMTAB and
LC_DYSYMTAB). No attempt is made to recover correct file offsets for
such data because, at present, such data is always stored by dsymutil
near the beginning of .dSYM files, within the first 4GB. If it becomes
necessary to address these other offsets, it should be possible to
recover these offsets by reference to the __LINKEDIT segment that
normally contains them, provided that __LINKEDIT doesn’t span more than
4GB, according to the strategy discussed at the bottom of
https://crbug.com/940823#c22.

Although this is sufficient to allow dump_syms to interpret Chromium
.dSYM files that exceed 4GB, be warned that these Mach-O files are still
technically malformed, and most other tools that consume Mach-O files
will continue to have difficulties interpreting these large files.

As further warning, note that should any individual DWARF section exceed
4GB, internal section offsets will be truncated irrecoverably, unless
and until the toolchain implements support for DWARF64.
https://bugs.llvm.org/show_bug.cgi?id=14969

With this change, dump_syms is able to correctly recover file offsets
from and continue processing a .dSYM file with length 4530593528
(4321MB), whose largest section (__DWARF,__debug_info = .debug_info) has
size 0x8d64c0b8 (2262MB), and which contains four sections (starting
with __DWARF,__apple_names) beginning at file offsets >= 4GB.

Bug: chromium:940823, chromium:946404
Change-Id: I23f5f3b07773fa2f010204d5bb53b6fb1d4926f7
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1541830
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
2019-03-28 20:43:54 +00:00