Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(116)

Issue 1305893010: Don’t trigger EXC_CORPSE_NOTIFY on OS X 10.11 (Closed)

Created:
5 years, 3 months ago by Mark Mentovai
Modified:
5 years, 3 months ago
Reviewers:
Robert Sesek, Greg K
CC:
crashpad-dev_chromium.org
Base URL:
https://chromium.googlesource.com/crashpad/crashpad@master
Target Ref:
refs/heads/master
Project:
crashpad
Visibility:
Public.

Description

Don’t trigger EXC_CORPSE_NOTIFY on OS X 10.11 CrashReportExceptionHandler::CatchMachException() must always set a valid new_state. Failing to do so appears to trigger corpse generation on OS X 10.11. This is addressed by calling ExcServerCopyState(). Previously, this was not done for exceptions forwarded to the user ReportCrash, under the apparent mistaken assumption that ReportCrash would do it. However, ReportCrash is given copies of out-parameters like new_state to explicitly prevent it from influencing Crashpad’s returned state. ExcServerSuccessfulReturnValue() must not return MACH_RCV_PORT_DIED for an EXC_CRASH handler on OS X 10.11. This appears to trigger corpse generation. This is addressed by always returning KERN_SUCCESS from EXC_CRASH handlers on OS X 10.11. This also adds generic EXC_CORPSE_NOTIFY support throughout Crashpad. The crashpad_handler does not listen for this exception type, but it is now possible to work with this exception type using tools like exception_port_tool and catch_exception_tool. BUG=crashpad:48 TEST=Crashes handled by crashpad_handler do not result in the generation of reports in the root /Library/Logs/DiagnosticReports. R=kerrnel@chromium.org, rsesek@chromium.org Committed: https://chromium.googlesource.com/crashpad/crashpad/+/9086d25ce8b0bf121cc2f771797ec15bf3c46c90

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 5

Patch Set 4 : Address review feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+282 lines, -86 lines) Patch
M client/crashpad_client_mac.cc View 1 chunk +3 lines, -5 lines 0 comments Download
M client/simulate_crash_mac_test.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M compat/mac/AvailabilityMacros.h View 1 chunk +6 lines, -0 lines 0 comments Download
M compat/mac/mach/mach.h View 1 chunk +18 lines, -9 lines 0 comments Download
M compat/non_mac/mach/mach.h View 1 chunk +3 lines, -0 lines 0 comments Download
M handler/mac/crash_report_exception_handler.cc View 1 3 chunks +6 lines, -13 lines 0 comments Download
M snapshot/mac/mach_o_image_annotations_reader_test.cc View 1 1 chunk +3 lines, -1 line 0 comments Download
M tools/mac/catch_exception_tool.cc View 1 3 chunks +29 lines, -13 lines 0 comments Download
M tools/mac/exception_port_tool.ad View 1 1 chunk +5 lines, -5 lines 0 comments Download
M tools/mac/exception_port_tool.cc View 1 chunk +1 line, -2 lines 0 comments Download
M util/mach/exc_server_variants.h View 1 2 2 chunks +16 lines, -4 lines 0 comments Download
M util/mach/exc_server_variants.cc View 1 2 2 chunks +12 lines, -1 line 0 comments Download
M util/mach/exc_server_variants_test.cc View 1 2 3 4 chunks +54 lines, -14 lines 0 comments Download
M util/mach/exception_ports_test.cc View 1 4 chunks +6 lines, -7 lines 0 comments Download
M util/mach/mach_extensions.h View 1 chunk +19 lines, -3 lines 0 comments Download
M util/mach/mach_extensions.cc View 1 2 3 3 chunks +28 lines, -8 lines 0 comments Download
M util/mach/mach_extensions_test.cc View 1 2 2 chunks +71 lines, -0 lines 0 comments Download
M util/mach/symbolic_constants_mach.cc View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 11 (5 generated)
Mark Mentovai
https://codereview.chromium.org/1305893010/diff/100001/compat/mac/mach/mach.h File compat/mac/mach/mach.h (left): https://codereview.chromium.org/1305893010/diff/100001/compat/mac/mach/mach.h#oldcode47 compat/mac/mach/mach.h:47: #undef EXC_MASK_VALID The SDK never exposed this, it was ...
5 years, 3 months ago (2015-09-03 15:24:45 UTC) #5
Greg K
On 2015/09/03 15:24:45, Mark Mentovai - August is over wrote: > https://codereview.chromium.org/1305893010/diff/100001/compat/mac/mach/mach.h > File compat/mac/mach/mach.h ...
5 years, 3 months ago (2015-09-03 19:30:54 UTC) #6
Mark Mentovai
Thanks, Greg. I’d like Robert to have a look also.
5 years, 3 months ago (2015-09-03 19:46:19 UTC) #8
Robert Sesek
LGTM w/ nits https://codereview.chromium.org/1305893010/diff/100001/util/mach/exc_server_variants.h File util/mach/exc_server_variants.h (right): https://codereview.chromium.org/1305893010/diff/100001/util/mach/exc_server_variants.h#newcode158 util/mach/exc_server_variants.h:158: //! handler is valid, because it ...
5 years, 3 months ago (2015-09-04 17:29:49 UTC) #9
Mark Mentovai
https://codereview.chromium.org/1305893010/diff/100001/util/mach/exc_server_variants.h File util/mach/exc_server_variants.h (right): https://codereview.chromium.org/1305893010/diff/100001/util/mach/exc_server_variants.h#newcode158 util/mach/exc_server_variants.h:158: //! handler is valid, because it will be passed ...
5 years, 3 months ago (2015-09-04 18:26:01 UTC) #10
Mark Mentovai
5 years, 3 months ago (2015-09-04 18:29:16 UTC) #11
Message was sent while issue was closed.
Committed patchset #4 (id:120001) manually as
9086d25ce8b0bf121cc2f771797ec15bf3c46c90 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698