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

Issue 584223002: ExceptionPorts test: accept a port of MACH_PORT_NULL as “no handler.” (Closed)

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

Description

ExceptionPorts::GetExceptionPorts(): don’t return ExceptionHandler elements whose handler port would be MACH_PORT_NULL. For most exception targets, *_get_exception_ports() will normally return an exception port of MACH_PORT_NULL when no handler is registered. However, as of Mac OS X 10.9, thread_get_exception_ports() will return an empty list when no handler is registered for any exception type on a thread. Consequently, a caller would have to do additional processing to determine whether a specific exception port is registered: an unregistered port will either appear but have a handler port of MACH_PORT_NULL, or it will not appear at all. This is confusing for callers. The behaviors are unified, and when a handler port of MACH_PORT_NULL is found, it will not be returned to the caller. This is expected to be the simpler of the two possible behaviors for callers to make use of. The change in the kernel can be seen by comparing 10.8.5 xnu-2050.48.11/osfmk/kern/ipc_tt.c thread_get_exception_ports() to the same function in 10.9.4 xnu-2422.110.17. The 10.9 version has a special check for thread->exc_actions being NULL, which short-circuits the rest of the function without returning any exception ports. In 10.8.5, thread->exc_actions can never be NULL. This new check is only present for thread targets, presumably because it’s very common for threads to not have any exception ports set, and not having to initialize this data is an optimization. Typical user-level tasks in Mac OS X always have at least some exception ports set at the task level. TEST=util_test ExceptionPorts.TaskAndThreadExceptionPorts R=rsesek@chromium.org Committed: https://chromium.googlesource.com/crashpad/crashpad/+/51e696ade94dd2b71fae487afe8a02c109fdcf2b

Patch Set 1 #

Total comments: 1

Patch Set 2 : Address review feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -7 lines) Patch
M util/mach/exception_ports.h View 1 1 chunk +5 lines, -1 line 0 comments Download
M util/mach/exception_ports.cc View 1 1 chunk +8 lines, -6 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
Mark Mentovai
6 years, 3 months ago (2014-09-19 20:29:24 UTC) #2
Robert Sesek
https://codereview.chromium.org/584223002/diff/1/util/mach/exception_ports.h File util/mach/exception_ports.h (right): https://codereview.chromium.org/584223002/diff/1/util/mach/exception_ports.h#newcode125 util/mach/exception_ports.h:125: //! is set to `MACH_PORT_NULL`. The precise behavior depends ...
6 years, 3 months ago (2014-09-22 15:08:23 UTC) #3
Mark Mentovai
Feedback addressed. I’m standardizing on the “remove MACH_PORT_NULL from the vector” behavior because I expect ...
6 years, 3 months ago (2014-09-22 16:18:23 UTC) #4
Robert Sesek
LGTM
6 years, 3 months ago (2014-09-22 16:52:50 UTC) #5
Mark Mentovai
6 years, 3 months ago (2014-09-22 17:07:48 UTC) #6
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 51e696a (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698