Build ForkWithFlags and part of NamespaceSandbox under nonsfi newlib.

Build ForkWithFlags and part of NamespaceSandbox under nonsfi newlib.

These functions are needed for enabling separate PID namespaces for
nonsfi newlib NaCl processes.

This change is an adaptation of hidehiko@'s
https://codereview.chromium.org/1161933003/ with some additional
functions from NamespaceSandbox added.

BUG=460972
Committed: https://crrev.com/179aeb7b6d1d9bb4c58a1abdc3929df197116056
Cr-Commit-Position: refs/heads/master@{#335175}

[rickyz (no longer on Chrome), 2015-06-16 00:20:20]

Patchset #1 (id:1) has been deleted

[rickyz (no longer on Chrome), 2015-06-16 00:23:06]

rickyz@chromium.org changed reviewers:
+ hidehiko@chromium.org, jln@chromium.org

[rickyz (no longer on Chrome), 2015-06-16 00:23:06]

Here's a change to enable ForkWithFlags and most of NamespaceSandbox for nonsfi
newlib NaCl. This should let us land support for newlib and non-newlib nonsfi
NaCl in one shot.

It's mostly similar to https://codereview.chromium.org/1161933003/ with some
additional functions in NamespaceSandbox poked through.

[hidehiko, 2015-06-16 05:41:53]

https://codereview.chromium.org/1176413003/diff/20001/sandbox/linux/services/...
File sandbox/linux/services/namespace_sandbox.cc (right):

https://codereview.chromium.org/1176413003/diff/20001/sandbox/linux/services/...
sandbox/linux/services/namespace_sandbox.cc:158: SIGHUP, SIGINT, SIGABRT,
SIGQUIT, SIGPIPE, SIGTERM, SIGUSR1, SIGUSR2,
These values are incompatible with Linux ABIs under OS_NACL_NONSFI build. Could
you take a look at linux_signal.h?

https://codereview.chromium.org/1176413003/diff/20001/sandbox/linux/services/...
sandbox/linux/services/namespace_sandbox.cc:171: PCHECK(sigaction(sig, nullptr,
&old_action) == 0);
Youl'll need to call sys_sigaction instead. Ditto for below.

https://codereview.chromium.org/1176413003/diff/20001/sandbox/linux/services/...
sandbox/linux/services/namespace_sandbox.cc:177: } else
nit: you don't need "else"

https://codereview.chromium.org/1176413003/diff/20001/sandbox/linux/services/...
sandbox/linux/services/namespace_sandbox.cc:179: if (old_action.sa_handler !=
SIG_DFL) {
SIG_DFL needs to be LINUX_SIG_DFL.

[rickyz (no longer on Chrome), 2015-06-16 20:57:07]

https://codereview.chromium.org/1176413003/diff/20001/sandbox/linux/services/...
File sandbox/linux/services/namespace_sandbox.cc (right):

https://codereview.chromium.org/1176413003/diff/20001/sandbox/linux/services/...
sandbox/linux/services/namespace_sandbox.cc:158: SIGHUP, SIGINT, SIGABRT,
SIGQUIT, SIGPIPE, SIGTERM, SIGUSR1, SIGUSR2,
On 2015/06/16 05:41:53, hidehiko wrote:
> These values are incompatible with Linux ABIs under OS_NACL_NONSFI build.
Could
> you take a look at linux_signal.h?

Oops, thanks!

https://codereview.chromium.org/1176413003/diff/20001/sandbox/linux/services/...
sandbox/linux/services/namespace_sandbox.cc:171: PCHECK(sigaction(sig, nullptr,
&old_action) == 0);
On 2015/06/16 05:41:53, hidehiko wrote:
> Youl'll need to call sys_sigaction instead. Ditto for below.

Oops, done - do you know whether we need our own definition of struct sigaction
as well then? I remember you saying something about sigset_t having a different
size before.

I wonder how much work it'd be to make our unit tests compile under newlib
nonsfi NaCl. Seems like there's a decent risk of bugs in these things slipping
through :-

https://codereview.chromium.org/1176413003/diff/20001/sandbox/linux/services/...
sandbox/linux/services/namespace_sandbox.cc:177: } else
On 2015/06/16 05:41:53, hidehiko wrote:
> nit: you don't need "else"

Done.

https://codereview.chromium.org/1176413003/diff/20001/sandbox/linux/services/...
sandbox/linux/services/namespace_sandbox.cc:179: if (old_action.sa_handler !=
SIG_DFL) {
On 2015/06/16 05:41:53, hidehiko wrote:
> SIG_DFL needs to be LINUX_SIG_DFL.

Done.

https://codereview.chromium.org/1176413003/diff/40001/sandbox/linux/system_he...
File sandbox/linux/system_headers/linux_signal.h (right):

https://codereview.chromium.org/1176413003/diff/40001/sandbox/linux/system_he...
sandbox/linux/system_headers/linux_signal.h:20: #define LINUX_SIGHUP 1    // 1
in PNaCl toolchain. Defined for consistency.
I got these from
native_client/toolchain/linux_x86/pnacl_newlib/le32-nacl/include/sys/signal.h -
is that correct?

[hidehiko, 2015-06-18 16:16:53]

LGTM. I think you'll need base OWNER review, too.

[jln (very slow on Chromium), 2015-06-18 18:23:20]

lgtm

https://chromiumcodereview.appspot.com/1176413003/diff/40001/sandbox/linux/sy...
File sandbox/linux/system_headers/linux_signal.h (right):

https://chromiumcodereview.appspot.com/1176413003/diff/40001/sandbox/linux/sy...
sandbox/linux/system_headers/linux_signal.h:59: #define LINUX_SIGHUP SIGHUP
Wouldn't it be better to:

1. Always use the #define above for LINUX_* always
2. Add a COMPILE_ASSERT for each to check that they match when we do have the
system headers

[rickyz (no longer on Chrome), 2015-06-18 22:50:57]

rickyz@chromium.org changed reviewers:
+ thakis@chromium.org

[rickyz (no longer on Chrome), 2015-06-18 22:50:58]

Thanks, adding thakis@ for base.

https://codereview.chromium.org/1176413003/diff/40001/sandbox/linux/system_he...
File sandbox/linux/system_headers/linux_signal.h (right):

https://codereview.chromium.org/1176413003/diff/40001/sandbox/linux/system_he...
sandbox/linux/system_headers/linux_signal.h:59: #define LINUX_SIGHUP SIGHUP
On 2015/06/18 18:23:20, jln wrote:
> Wouldn't it be better to:
> 
> 1. Always use the #define above for LINUX_* always
> 2. Add a COMPILE_ASSERT for each to check that they match when we do have the
> system headers

Done.

[Nico, 2015-06-18 23:01:41]

base lgtm

[rickyz (no longer on Chrome), 2015-06-18 23:02:23]

The CQ bit was checked by rickyz@chromium.org

[rickyz (no longer on Chrome), 2015-06-18 23:02:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
hidehiko@chromium.org, jln@chromium.org
Link to the patchset: https://codereview.chromium.org/1176413003/#ps60001
(title: "More comments.")

[commit-bot: I haz the power, 2015-06-18 23:05:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1176413003/60001

[commit-bot: I haz the power, 2015-06-19 00:18:55]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-06-19 00:19:38]

Patchset 3 (id:??) landed as
https://crrev.com/179aeb7b6d1d9bb4c58a1abdc3929df197116056
Cr-Commit-Position: refs/heads/master@{#335175}