Update NaCl's generator to support new wait APIs.

Update NaCl's generator to support new wait APIs.

TEST=compile monacl_shell and monacl_test in the Chrome repo and run the unit tests.
BUG=https://code.google.com/p/chromium/issues/detail?id=401761
R=mseaborn@chromium.org, viettrungluu@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/7adc5c43dc8f7718905d5d23d3f02fcc8a96d6af

[Nick Bray (chromium), 2014-12-05 00:51:46]

ncbray@chromium.org changed reviewers:
+ mseaborn@chromium.org, vtl@google.com

[Nick Bray (chromium), 2014-12-05 00:51:47]

vtl: Mojo.
I modified one of the tests to make everything pass.  I could instead change the
implementation to copy in all the "outputs" if required.

mseaborn: NaCl and security.
Note memcpy_volatile_out.  Technically copying data into untrusted space does
not need to be volatile for security?  It felt like the right thing, however,
and I wanted to set up a pattern for copying structs in.  If I need to modify
this code to preserve the value of unmodified outputs, I may need to deal with
volatile inputs sooner rather than later.

Diffing 1 and 2 will show the changes in the generated code.

[Mark Seaborn, 2014-12-08 22:24:12]

Can you add a TEST= field to explain how this is tested?

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/generator/gene...
File mojo/nacl/generator/generate_nacl_bindings.py (right):

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/generator/gene...
mojo/nacl/generator/generate_nacl_bindings.py:205: (p.uid +
1,CBool(p.is_optional), p.name))
Add space after ','

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/generator/gene...
mojo/nacl/generator/generate_nacl_bindings.py:308: if p.is_input and not
p.is_output and p.is_extensible:
Fix indentation

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/generator/inte...
File mojo/nacl/generator/interface_dsl.py (right):

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/generator/inte...
mojo/nacl/generator/interface_dsl.py:59: self.is_extensible = False
Can you explain somewhere what is_extensible/ExtStruct mean?

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/mojo_syscall_i...
File mojo/nacl/mojo_syscall_internal.h (right):

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/mojo_syscall_i...
mojo/nacl/mojo_syscall_internal.h:35: void memcpy_volatile_out(void volatile*
dst, const void* src, size_t n) {
Shouldn't this be static and/or inline, since it's in a header?

Also, this could do with a comment to explain why you don't use memcpy.

https://codereview.chromium.org/776643004/diff/20001/mojo/public/c/system/tes...
File mojo/public/c/system/tests/core_unittest_pure_c.c (right):

https://codereview.chromium.org/776643004/diff/20001/mojo/public/c/system/tes...
mojo/public/c/system/tests/core_unittest_pure_c.c:72: // The NaCl syscall
wrapper currently writes to all its outputs.
Should there be a TODO here?

[Nick Bray (chromium), 2014-12-09 22:14:31]

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/generator/gene...
File mojo/nacl/generator/generate_nacl_bindings.py (right):

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/generator/gene...
mojo/nacl/generator/generate_nacl_bindings.py:205: (p.uid +
1,CBool(p.is_optional), p.name))
On 2014/12/08 22:24:12, Mark Seaborn wrote:
> Add space after ','

Done.

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/generator/gene...
mojo/nacl/generator/generate_nacl_bindings.py:308: if p.is_input and not
p.is_output and p.is_extensible:
On 2014/12/08 22:24:12, Mark Seaborn wrote:
> Fix indentation

Done.

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/generator/inte...
File mojo/nacl/generator/interface_dsl.py (right):

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/generator/inte...
mojo/nacl/generator/interface_dsl.py:59: self.is_extensible = False
On 2014/12/08 22:24:12, Mark Seaborn wrote:
> Can you explain somewhere what is_extensible/ExtStruct mean?

Done.

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/mojo_syscall_i...
File mojo/nacl/mojo_syscall_internal.h (right):

https://codereview.chromium.org/776643004/diff/20001/mojo/nacl/mojo_syscall_i...
mojo/nacl/mojo_syscall_internal.h:35: void memcpy_volatile_out(void volatile*
dst, const void* src, size_t n) {
On 2014/12/08 22:24:12, Mark Seaborn wrote:
> Shouldn't this be static and/or inline, since it's in a header?
> 
> Also, this could do with a comment to explain why you don't use memcpy.

Done.

https://codereview.chromium.org/776643004/diff/20001/mojo/public/c/system/tes...
File mojo/public/c/system/tests/core_unittest_pure_c.c (right):

https://codereview.chromium.org/776643004/diff/20001/mojo/public/c/system/tes...
mojo/public/c/system/tests/core_unittest_pure_c.c:72: // The NaCl syscall
wrapper currently writes to all its outputs.
On 2014/12/08 22:24:12, Mark Seaborn wrote:
> Should there be a TODO here?

What should be done here is currently under discussion with vtl.

[Nick Bray (chromium), 2014-12-11 00:58:29]

PTAL

Longer term, I think we need to chew on how outputs should behave, but short
term this CL needs to land.  I changed the generation to make all the outputs
other than the return value effectively be inout parameters.

[Mark Seaborn, 2014-12-11 15:54:20]

As we discussed in person, I'd like someone from the Mojo project to review this
change first, since the change is more Mojo-related than NaCl-related.  I can
then review it from a NaCl point of view if you want.

[viettrungluu, 2014-12-11 17:22:45]

viettrungluu@chromium.org changed reviewers:
+ viettrungluu@chromium.org

[viettrungluu, 2014-12-11 17:22:46]

lgtm

https://codereview.chromium.org/776643004/diff/60001/mojo/nacl/mojo_syscall_i...
File mojo/nacl/mojo_syscall_internal.h (right):

https://codereview.chromium.org/776643004/diff/60001/mojo/nacl/mojo_syscall_i...
mojo/nacl/mojo_syscall_internal.h:49: char volatile* c_dst =
reinterpret_cast<char volatile*>(dst);
A static_cast suffices.

https://codereview.chromium.org/776643004/diff/60001/mojo/nacl/mojo_syscall_i...
mojo/nacl/mojo_syscall_internal.h:50: const char* c_src = reinterpret_cast<const
char*>(src);
"

[Nick Bray (chromium), 2014-12-12 00:03:26]

PTAL Mark?

https://codereview.chromium.org/776643004/diff/60001/mojo/nacl/mojo_syscall_i...
File mojo/nacl/mojo_syscall_internal.h (right):

https://codereview.chromium.org/776643004/diff/60001/mojo/nacl/mojo_syscall_i...
mojo/nacl/mojo_syscall_internal.h:49: char volatile* c_dst =
reinterpret_cast<char volatile*>(dst);
On 2014/12/11 17:22:45, viettrungluu wrote:
> A static_cast suffices.

Done.

[Mark Seaborn, 2014-12-15 02:59:04]

LGTM for NaCl security (largely a rubber stamp)

https://codereview.chromium.org/776643004/diff/80001/mojo/nacl/generator/inte...
File mojo/nacl/generator/interface_dsl.py (right):

https://codereview.chromium.org/776643004/diff/80001/mojo/nacl/generator/inte...
mojo/nacl/generator/interface_dsl.py:123: # Is it valid to pass a null pointer?
How about: "Declares that it is valid to pass a null pointer"

Otherwise this looks like some sort of TODO. :-)

[Nick Bray (chromium), 2014-12-15 21:05:50]

https://codereview.chromium.org/776643004/diff/80001/mojo/nacl/generator/inte...
File mojo/nacl/generator/interface_dsl.py (right):

https://codereview.chromium.org/776643004/diff/80001/mojo/nacl/generator/inte...
mojo/nacl/generator/interface_dsl.py:123: # Is it valid to pass a null pointer?
On 2014/12/15 02:59:04, Mark Seaborn wrote:
> How about: "Declares that it is valid to pass a null pointer"
> 
> Otherwise this looks like some sort of TODO. :-)

Done.

[Nick Bray (chromium), 2014-12-15 21:24:08]

Committed patchset #7 (id:120001) manually as
7adc5c43dc8f7718905d5d23d3f02fcc8a96d6af (presubmit successful).