Make libnosys function stubs part of libnacl

Make libnosys function stubs part of libnacl

These functions are already a part of glibc, but in case of newlib
developers have to explicitly link against nosys. This change moves all
the function stubs to libnacl eliminating this issue. The follow up
change should plumb these functions to IRT interfaces to allow
overriding these using IRT overlays.

The libnosys will be kept around for now as an empty library
for backwards compatibility.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3647
TEST= small_tests
R=bsy@google.com, mseaborn@chromium.org, sbc@chromium.org

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

[Mark Seaborn, 2013-08-19 23:42:08]

Hmm, I hadn't considered how many files were involved when we discussed this
earlier.  I wonder if we should split these into subdirectories to make the
untrusted/nacl/ directory a bit more manageable?  Increasing its size from ~75
to ~150 files seems a bit unwieldy.

How about something like untrusted/nacl/file/ and untrusted/nacl/process/?  Or
is that too arbitrary?

[Sam Clegg, 2013-08-20 00:02:48]

On 2013/08/19 23:42:08, Mark Seaborn wrote:
> Hmm, I hadn't considered how many files were involved when we discussed this
> earlier.  I wonder if we should split these into subdirectories to make the
> untrusted/nacl/ directory a bit more manageable?  Increasing its size from ~75
> to ~150 files seems a bit unwieldy.
> 
> How about something like untrusted/nacl/file/ and untrusted/nacl/process/?  Or
> is that too arbitrary?

We could also merge these into a single file?   The ability to selectively
override
them is needed but this can also be achieved using "weak_reference" when
defining the
actual symbol name.

However, it seems more common (e.g. newlib and glibc both do it) to split the
system
calls into their own files....  so I think we should leave it as is.

[bsy, 2013-08-20 23:07:57]

separate files are desirable for static linkage, so it's traditional to do it
that way.

i don't care wrt many small files.

lgtm.

[Sam Clegg, 2013-08-21 00:57:33]

lgtm

[Mark Seaborn, 2013-08-23 15:45:22]

Is there an issue you can link to in BUG=?  LGTM with changes below.

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nacl/nacl.s...
File src/untrusted/nacl/nacl.scons (right):

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nacl/nacl.s...
src/untrusted/nacl/nacl.scons:160: sources = SRCS_NACL_EXTENSIONS + \
Nit: Python style is to use ()s in preference to backslashes, i.e.
x = (a +
     b +
     c)

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nacl/stubs/...
File src/untrusted/nacl/stubs/_execve-stub.c (right):

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nacl/stubs/...
src/untrusted/nacl/stubs/_execve-stub.c:1: /*
The naming style in NaCl is to use "_" not "-" in filenames, so please use
"_stub.c" rather than "-stub.c".  But I don't know why you added a "stub" suffix
-- it would be OK without a suffix.

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/_dummy.c
File src/untrusted/nosys/_dummy.c (right):

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/_dumm...
src/untrusted/nosys/_dummy.c:7: #include <unistd.h>
This #include is not needed.  This file needs a comment explaining why it's
empty.

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/nacl....
File src/untrusted/nosys/nacl.scons (right):

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/nacl....
src/untrusted/nosys/nacl.scons:11: # scripts complain when the archive is really
empty. We might remove the
Can you file a bug about this problem with the PNaCl toolchain, please, and link
to it here?

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/nosys...
File src/untrusted/nosys/nosys.gyp (right):

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/nosys...
src/untrusted/nosys/nosys.gyp:12: '_dummy.c',
"_dummy.c" needs a comment.  Also, put this in 'sources' below.  I don't see a
reason for this to be in 'common_sources'.

[Petr Hosek, 2013-08-23 21:35:07]

I have created a new issue.

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nacl/nacl.s...
File src/untrusted/nacl/nacl.scons (right):

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nacl/nacl.s...
src/untrusted/nacl/nacl.scons:160: sources = SRCS_NACL_EXTENSIONS + \
On 2013/08/23 15:45:22, Mark Seaborn wrote:
> Nit: Python style is to use ()s in preference to backslashes, i.e.
> x = (a +
>      b +
>      c)

Fixed.

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nacl/stubs/...
File src/untrusted/nacl/stubs/_execve-stub.c (right):

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nacl/stubs/...
src/untrusted/nacl/stubs/_execve-stub.c:1: /*
On 2013/08/23 15:45:22, Mark Seaborn wrote:
> The naming style in NaCl is to use "_" not "-" in filenames, so please use
> "_stub.c" rather than "-stub.c".  But I don't know why you added a "stub"
suffix
> -- it would be OK without a suffix.

Dropped the "stub" suffix.

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/_dummy.c
File src/untrusted/nosys/_dummy.c (right):

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/_dumm...
src/untrusted/nosys/_dummy.c:7: #include <unistd.h>
On 2013/08/23 15:45:22, Mark Seaborn wrote:
> This #include is not needed.  This file needs a comment explaining why it's
> empty.

Removed the entire source file.

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/nacl....
File src/untrusted/nosys/nacl.scons (right):

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/nacl....
src/untrusted/nosys/nacl.scons:11: # scripts complain when the archive is really
empty. We might remove the
On 2013/08/23 15:45:22, Mark Seaborn wrote:
> Can you file a bug about this problem with the PNaCl toolchain, please, and
link
> to it here?

I have already updated the PNaCl toolchain so this hack is no longer needed.

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/nosys...
File src/untrusted/nosys/nosys.gyp (right):

https://codereview.chromium.org/22923022/diff/78001/src/untrusted/nosys/nosys...
src/untrusted/nosys/nosys.gyp:12: '_dummy.c',
On 2013/08/23 15:45:22, Mark Seaborn wrote:
> "_dummy.c" needs a comment.  Also, put this in 'sources' below.  I don't see a
> reason for this to be in 'common_sources'.

Removed altogether.

[Sam Clegg, 2013-08-23 21:55:50]

slgtm

https://codereview.chromium.org/22923022/diff/40002/src/untrusted/nosys/geteg...
File src/untrusted/nosys/getegid.c (left):

https://codereview.chromium.org/22923022/diff/40002/src/untrusted/nosys/geteg...
src/untrusted/nosys/getegid.c:22: stub_warning(getegid);
Can you remove all the stub warnings?

[Petr Hosek, 2013-08-23 22:34:55]

https://codereview.chromium.org/22923022/diff/40002/src/untrusted/nosys/geteg...
File src/untrusted/nosys/getegid.c (left):

https://codereview.chromium.org/22923022/diff/40002/src/untrusted/nosys/geteg...
src/untrusted/nosys/getegid.c:22: stub_warning(getegid);
On 2013/08/23 21:55:51, Sam Clegg wrote:
> Can you remove all the stub warnings?

They should be all gone already, do you seen any left? I have even removed the
"warning.h" header file so I assume we would see build failure if there was some
leftover.

[Sam Clegg, 2013-08-23 22:36:33]

https://codereview.chromium.org/22923022/diff/40002/src/untrusted/nosys/geteg...
File src/untrusted/nosys/getegid.c (left):

https://codereview.chromium.org/22923022/diff/40002/src/untrusted/nosys/geteg...
src/untrusted/nosys/getegid.c:22: stub_warning(getegid);
On 2013/08/23 22:34:56, Petr Hosek wrote:
> On 2013/08/23 21:55:51, Sam Clegg wrote:
> > Can you remove all the stub warnings?
> 
> They should be all gone already, do you seen any left? I have even removed the
> "warning.h" header file so I assume we would see build failure if there was
some
> leftover.


Oops, my bad.  Was reading the wrong side of the diff.

[Petr Hosek, 2013-08-24 03:46:29]

Turned out that https://codereview.chromium.org/22903042/ was not enough to make
PNaCl happy about the empty archive. When looking into the issue, I found out
that PNaCl driver reads the archive content to determine the archive
architecture and it fails to so for the empty archive which is obviously
incorrect as archives themselves don't have any architecture and an empty
archive technically has any architecture.

The proper way to resolve this would be to fix the PNaCl driver, but that may
require more significant changes. Another option is to revert back the original
solution with dummy content which is more hacky but easier done. Which one do
you prefer?

[Sam Clegg, 2013-08-25 16:34:09]

On 2013/08/24 03:46:29, Petr Hosek wrote:
> Turned out that https://codereview.chromium.org/22903042/ was not enough to
make
> PNaCl happy about the empty archive. When looking into the issue, I found out
> that PNaCl driver reads the archive content to determine the archive
> architecture and it fails to so for the empty archive which is obviously
> incorrect as archives themselves don't have any architecture and an empty
> archive technically has any architecture.
> 
> The proper way to resolve this would be to fix the PNaCl driver, but that may
> require more significant changes. Another option is to revert back the
original
> solution with dummy content which is more hacky but easier done. Which one do
> you prefer?

I'd say, go back to old way with the dummy content, but include a TODO with a
reference to a bug to get the pnacl driver fixed...

[Petr Hosek, 2013-08-28 22:51:30]

On 2013/08/25 16:34:09, Sam Clegg wrote:
> On 2013/08/24 03:46:29, Petr Hosek wrote:
> > Turned out that https://codereview.chromium.org/22903042/ was not enough to
> make
> > PNaCl happy about the empty archive. When looking into the issue, I found
out
> > that PNaCl driver reads the archive content to determine the archive
> > architecture and it fails to so for the empty archive which is obviously
> > incorrect as archives themselves don't have any architecture and an empty
> > archive technically has any architecture.
> > 
> > The proper way to resolve this would be to fix the PNaCl driver, but that
may
> > require more significant changes. Another option is to revert back the
> original
> > solution with dummy content which is more hacky but easier done. Which one
do
> > you prefer?
> 
> I'd say, go back to old way with the dummy content, but include a TODO with a
> reference to a bug to get the pnacl driver fixed...

PNaCl toolchain was finally updated to include my patches to support empty
archives and trybots seem to be happy. Unless there are other comments, I'll
(finally) push the change then.

[Petr Hosek, 2013-08-29 16:25:29]

Committed patchset #6 manually as r12084 (presubmit successful).

[jvoung (off chromium), 2013-08-29 20:36:35]

Are there plans to add getrusage to the newlib headers?


This change is now making the PNaCl LLVM self-build detect getrusage as being
available during the "configure" step.    LLVM then starts trying to compile
code that uses getrusage.  However, I *think* that because it's not actually in
the newlib headers, the compile now fails with:



/mnt/data/b/build/slave/linux-pnacl-x86_64/build/native_client/pnacl/git/llvm/lib/Support/Unix/Process.inc:57:5:error:
no member named 'getrusage' in the global namespace; did you mean 'rusage'?
  ::getrusage(RUSAGE_SELF, &RU);
  ~~^
llvm[1]: Compiling SmallVector.cpp for Release build

[Sam Clegg, 2013-09-04 15:58:24]

On 2013/08/29 20:36:35, jvoung (cr) wrote:
> Are there plans to add getrusage to the newlib headers?
> 
> 
> This change is now making the PNaCl LLVM self-build detect getrusage as being
> available during the "configure" step.    LLVM then starts trying to compile
> code that uses getrusage.  However, I *think* that because it's not actually
in
> the newlib headers, the compile now fails with:
> 
> 
> 
>
/mnt/data/b/build/slave/linux-pnacl-x86_64/build/native_client/pnacl/git/llvm/lib/Support/Unix/Process.inc:57:5:error:
> no member named 'getrusage' in the global namespace; did you mean 'rusage'?
>   ::getrusage(RUSAGE_SELF, &RU);
>   ~~^
> llvm[1]: Compiling SmallVector.cpp for Release build

Yes, we should be declaring this (and others) in newlibs headers.

Most likely it is already in there guarded by "if CYGWIN || RTEMS".   There are
quite a
few places in the newlib headers where we should (IMHO) add nacl to these
conditional
blocks.  Please open a ticket and assign to me (or Petr).