Add the crashpad_handler executable

Add the crashpad_handler executable.

crashpad_handler is Crashpad’s exception handler server.

Currently, it runs a loop to receive exceptions, and exits when it no
longer has any clients. In the future, this will be extended to write
and potentially upload dumps.

The handler is expected to be started by its initial client via the
CrashpadClient interface.

R=rsesek@chromium.org

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/588d7dba1ce2d18f56a0ad17f0dca636e6b1bff0

[Mark Mentovai, 2014-12-19 22:47:27]

mark@chromium.org changed reviewers:
+ rsesek@chromium.org

[Mark Mentovai, 2014-12-19 22:47:28]

The CrashpadClient interface that starts crashpad_handler is proposed for review
separately at https://codereview.chromium.org/785233011. There is no strict
dependency relationship between these two changes, although neither is fully
usable without the other.

[Robert Sesek, 2014-12-29 16:17:08]

https://codereview.chromium.org/789693005/diff/20001/handler/mac/exception_ha...
File handler/mac/exception_handler_server.cc (right):

https://codereview.chromium.org/789693005/diff/20001/handler/mac/exception_ha...
handler/mac/exception_handler_server.cc:55: DCHECK(running_);
To ensure that this method is only called once, why isn't running_ initialized
to false and then DCHECK(!running_) here before setting it to true?

https://codereview.chromium.org/789693005/diff/20001/handler/mac/exception_ha...
handler/mac/exception_handler_server.cc:107: MACH_CHECK(mr == MACH_MSG_SUCCESS,
mr) << "MachMessageServer::Run";
Won't this abort if an error occurs, contrary to the documentation for this
method?

https://codereview.chromium.org/789693005/diff/20001/handler/mac/exception_ha...
handler/mac/exception_handler_server.cc:131: return MIG_BAD_ID;
MIG_BAD_ARGUMENTS instead?

https://codereview.chromium.org/789693005/diff/20001/handler/mac/main.cc
File handler/mac/main.cc (right):

https://codereview.chromium.org/789693005/diff/20001/handler/mac/main.cc#newc...
handler/mac/main.cc:35: "  -h, --handshake-fd=FD  establish communication with
the client over FD\n"
Since this is never meant to be run by users, I'm not sure you need a short arg.

https://codereview.chromium.org/789693005/diff/20001/handler/mac/main.cc#newc...
handler/mac/main.cc:93: if (options.handshake_fd < 0) {
This is done above on line 74, too. This one is needed for the lack of a -h, but
you could remove the one above.

[Mark Mentovai, 2014-12-29 17:08:07]

If you think any of these things deserves a comment in the source code, just say
so.

https://codereview.chromium.org/789693005/diff/20001/handler/mac/exception_ha...
File handler/mac/exception_handler_server.cc (right):

https://codereview.chromium.org/789693005/diff/20001/handler/mac/exception_ha...
handler/mac/exception_handler_server.cc:55: DCHECK(running_);
Robert Sesek wrote:
> To ensure that this method is only called once, why isn't running_ initialized
> to false and then DCHECK(!running_) here before setting it to true?

This DCHECK actually does ensure that the method is only called once.

When Run() returns, running_ will always be false. The constructor initializes
running_ to true to disambiguate, so that the DCHECK checks that it was not
called previously.

https://codereview.chromium.org/789693005/diff/20001/handler/mac/exception_ha...
handler/mac/exception_handler_server.cc:107: MACH_CHECK(mr == MACH_MSG_SUCCESS,
mr) << "MachMessageServer::Run";
Robert Sesek wrote:
> Won't this abort if an error occurs, contrary to the documentation for this
> method?

The documentation says

  //! If an unexpected condition that prevents this method from functioning is
  //! encountered, it will log a message and terminate execution. Receipt of an
  //! invalid message on \a receive_port_ will cause a message to be logged, but
  //! this method will continue running normally.

MachMessageServer::Run() doesn’t consider a demux failure (invalid message,
etc.) an error, it just reports the return value from mach_msg() send and
receive attempts. It ignores mach_msg() errors like MACH_RCV_TOO_LARGE because
that’s what it was asked to do. It doesn’t time out because it was asked to wait
indefinitely. So if MachMessageServer::Run() returns a non-success result, it’s
probably because it was invoked incorrectly.

https://codereview.chromium.org/789693005/diff/20001/handler/mac/exception_ha...
handler/mac/exception_handler_server.cc:131: return MIG_BAD_ID;
Robert Sesek wrote:
> MIG_BAD_ARGUMENTS instead?

This (exception_port != exception_port_) happens when someone with a send right
to something else (the notify server) in the port set sends a message that isn’t
appropriate for that send right. The fact that there’s a port set is an
implementation detail of this server. Normally, if there wasn’t a port set but
just an individual NotifyServer, any request of the notify subsystem would
result in the MIG_BAD_ID, because the notify subsystem’s only expected to handle
requests from that subsystem.

So MIG_BAD_ID is just saying “I don’t understand that message ID,” but the “I
don’t understand” part is nuanced, more like “the port you sent the message to
doesn’t understand that message ID.”

https://codereview.chromium.org/789693005/diff/20001/handler/mac/main.cc
File handler/mac/main.cc (right):

https://codereview.chromium.org/789693005/diff/20001/handler/mac/main.cc#newc...
handler/mac/main.cc:93: if (options.handshake_fd < 0) {
Robert Sesek wrote:
> This is done above on line 74, too. This one is needed for the lack of a -h,
but
> you could remove the one above.

The first one comes into play when you give a bad argument, and the second when
you don’t give it at all. I think it’s useful to distinguish. The other tools do
when converting a string argument to a number.

[Robert Sesek, 2014-12-30 16:21:52]

LGTM. Don't think any of these warrant comments.

[Mark Mentovai, 2014-12-30 19:23:51]

Committed patchset #3 (id:40001) manually as
588d7dba1ce2d18f56a0ad17f0dca636e6b1bff0 (presubmit successful).