Provide chrome the option of learning the debug stub port chosen by nacl.

Provide chrome the option of learning the debug stub port chosen by nacl.

On mac/linux the debug stub port is allocated chrome side and provided to
Native Client. On Windows (where socket handles cannot be passed between
processes), the debug port must be allocated by the nacl process (which can
only happen when the outer sandbox is disabled).

Propagate the debug port selected by the nacl side to the task manager to make
it available to the user and debugging extensions.

BUG=crbug.com/328714
TEST=None
R=mseaborn@chromium.org

Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=12967

[bradn, 2014-03-25 17:52:35]

ping?

[Mark Seaborn, 2014-03-26 23:17:35]

LGTM

https://codereview.chromium.org/206493005/diff/1/src/public/chrome_main.h
File src/public/chrome_main.h (right):

https://codereview.chromium.org/206493005/diff/1/src/public/chrome_main.h#new...
src/public/chrome_main.h:103: * Callback called when debug stub port is know. 
Optional; may be NULL.
"known"

https://codereview.chromium.org/206493005/diff/1/src/trusted/debug_stub/nacl_...
File src/trusted/debug_stub/nacl_debug.cc (right):

https://codereview.chromium.org/206493005/diff/1/src/trusted/debug_stub/nacl_...
src/trusted/debug_stub/nacl_debug.cc:55: // If port 4014 is not available, try
any port.
This comment belongs before the previous if().

https://codereview.chromium.org/206493005/diff/1/src/trusted/debug_stub/nacl_...
src/trusted/debug_stub/nacl_debug.cc:58: "NaClDebugStubBindSocke: Failed to bind
any TCP port\n");
"NaClDebugBindSocket".  No "Stub", missing "t".

https://codereview.chromium.org/206493005/diff/1/src/trusted/debug_stub/nacl_...
src/trusted/debug_stub/nacl_debug.cc:131: if (!NaClDebugBindSocket()) {
This belongs before "new NaClThread".  "new NaClThread" is paired with
NaClThreadCtor().

https://codereview.chromium.org/206493005/diff/1/src/trusted/debug_stub/nacl_...
src/trusted/debug_stub/nacl_debug.cc:132: NaClLog(LOG_ERROR, "NaClDebugInit:
Failed to bind debug socket\n");
NaClDebugBindSocket() already prints an error, so you can omit this.

[bradn, 2014-04-01 18:31:20]

https://codereview.chromium.org/206493005/diff/1/src/public/chrome_main.h
File src/public/chrome_main.h (right):

https://codereview.chromium.org/206493005/diff/1/src/public/chrome_main.h#new...
src/public/chrome_main.h:103: * Callback called when debug stub port is know. 
Optional; may be NULL.
On 2014/03/26 23:17:35, Mark Seaborn wrote:
> "known"

Done.

https://codereview.chromium.org/206493005/diff/1/src/trusted/debug_stub/nacl_...
File src/trusted/debug_stub/nacl_debug.cc (right):

https://codereview.chromium.org/206493005/diff/1/src/trusted/debug_stub/nacl_...
src/trusted/debug_stub/nacl_debug.cc:55: // If port 4014 is not available, try
any port.
On 2014/03/26 23:17:35, Mark Seaborn wrote:
> This comment belongs before the previous if().

Done.

https://codereview.chromium.org/206493005/diff/1/src/trusted/debug_stub/nacl_...
src/trusted/debug_stub/nacl_debug.cc:58: "NaClDebugStubBindSocke: Failed to bind
any TCP port\n");
On 2014/03/26 23:17:35, Mark Seaborn wrote:
> "NaClDebugBindSocket".  No "Stub", missing "t".

Done.

https://codereview.chromium.org/206493005/diff/1/src/trusted/debug_stub/nacl_...
src/trusted/debug_stub/nacl_debug.cc:131: if (!NaClDebugBindSocket()) {
On 2014/03/26 23:17:35, Mark Seaborn wrote:
> This belongs before "new NaClThread".  "new NaClThread" is paired with
> NaClThreadCtor().

Done.

https://codereview.chromium.org/206493005/diff/1/src/trusted/debug_stub/nacl_...
src/trusted/debug_stub/nacl_debug.cc:132: NaClLog(LOG_ERROR, "NaClDebugInit:
Failed to bind debug socket\n");
On 2014/03/26 23:17:35, Mark Seaborn wrote:
> NaClDebugBindSocket() already prints an error, so you can omit this.

Done.

[bradn, 2014-04-01 20:34:31]

Committed patchset #2 manually as r12967 (presubmit successful).

[Mark Seaborn, 2014-04-04 00:22:44]

This breaks NaCl's debug stub tests when rolled into Chromium:

** Signal 31 from trusted code: pc=7f5ca21b42a7
../../sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc:**CRASHING**:seccomp-bpf
failure in syscall 0051

See:
http://build.chromium.org/p/tryserver.chromium/builders/linux_rel/builds/2388...
https://codereview.chromium.org/221803002/

Syscall 51 is getsockname().

I think you'll need to change the sandbox policy for the NaCl process on the
Chromium side to allow getsockname().  Please test your change against the
Chromium trybots next time!

[Mark Seaborn, 2014-04-04 00:33:21]

On 2014/04/04 00:22:44, Mark Seaborn wrote:
> I think you'll need to change the sandbox policy for the NaCl process
> on the Chromium side to allow getsockname().

Actually, no, you just need to make sure the following is only done on Windows:

  nap->debug_stub_port = g_socket_binding->GetBoundPort();