Split the PNaCl IRT shim into 3 pieces, and include one piece into IRT.

Split the PNaCl IRT shim into 3 pieces, and include one piece into IRT.

(*) A "for irt" library, that will get linked into the IRT.
That library just has the auto-generated pnacl_shim.c file.
Also modify ppapi_proxy to include a "irt_shim_ppapi.c",
which makes use of the pnacl_shim.c file to define a private
hook, parallel to the hook provided by irt_ppapi.c.

(*) A "for the browser" library, that the browser-tester
and the PNaCl packager will include for PNaCl-in-the-browser.
This is still called "libpnacl_irt_shim.a" so that we don't
need to change the in-browser linker's commandline. However,
it will be placed in a different directory so that the
Chrome PNaCl packager can pick it up separately from
the AOT library. This for-browser library uses the new hook.

(*) A "AOT" library, that will go into the SDK for offline
pexe -> nexe translation. Still called "libpnacl_irt_shim.a"
as well, confusingly enough, so that we don't need to change
the pnacl-nativeld.py commandlines. Placed in a different
directory from the for-browser library. Does not use the
new hook, so that it only depends on stable interfaces.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3783
R=binji@chromium.org, dmichael@chromium.org, mseaborn@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255807

[jvoung (off chromium), 2014-02-14 23:11:54]

https://codereview.chromium.org/164373010/diff/1/ppapi/native_client/src/untr...
File ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp
(right):

https://codereview.chromium.org/164373010/diff/1/ppapi/native_client/src/untr...
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp:30:
'../../../../proxy/irt_shim_ppapi.c',
Wasn't sure how common this sort of thing is done w/ gyp, but tried to avoid
duplicating the code...

[Mark Seaborn, 2014-02-14 23:45:36]

Minor comment below.  This change will take me a while to understand fully...

https://codereview.chromium.org/164373010/diff/30001/ppapi/native_client/nati...
File ppapi/native_client/native_client.gyp (right):

https://codereview.chromium.org/164373010/diff/30001/ppapi/native_client/nati...
ppapi/native_client/native_client.gyp:209:
'>(tc_lib_dir_irt_arm/libpnacl_irt_shim_for_irt.a',
Missing ")" here

[jvoung (off chromium), 2014-02-25 22:44:10]

rebased --

Sorry, the ifdefs give me a bit of a headache. Haven't thought of a better way
yet =/

https://codereview.chromium.org/164373010/diff/30001/ppapi/native_client/nati...
File ppapi/native_client/native_client.gyp (right):

https://codereview.chromium.org/164373010/diff/30001/ppapi/native_client/nati...
ppapi/native_client/native_client.gyp:209:
'>(tc_lib_dir_irt_arm/libpnacl_irt_shim_for_irt.a',
On 2014/02/14 23:45:36, Mark Seaborn wrote:
> Missing ")" here

Done.

[Mark Seaborn, 2014-02-26 22:11:03]

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
File ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp
(right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp:39:
'-DPNACL_SHIM_AOT=1',
I don't see a corresponding -DPNACL_SHIM_AOT=0, which you should really have if
you're using "#if PNACL_SHIM_AOT" (which is preferable to #ifdef).

But if there's no good place for -DPNACL_SHIM_AOT=0, maybe use
"-DPNACL_SHIM_AOT" and #ifdef instead?

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp:97: ],
This version doesn't have "--strip-debug".  Is there a reason for having it (and
then not having it)?

Is there a reason for using type=none?

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
File ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.h (right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.h:7: #define
PPAPI_NATIVE_CLIENT_SRC_UNTRUSTED_PNACL_IRT_SHIM_PNACL_SHIM_H_ 1
Chrome style is not to have the "1" here, so you don't need this change

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
File ppapi/native_client/src/untrusted/pnacl_irt_shim/shim_ppapi.c (right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/shim_ppapi.c:115: * For the
PNaCl in-the-browser, redirect to using
Nit: drop "the"?

https://codereview.chromium.org/164373010/diff/260001/ppapi/ppapi_proxy.gypi
File ppapi/ppapi_proxy.gypi (right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/ppapi_proxy.gypi#...
ppapi/ppapi_proxy.gypi:242: 'target_conditions': [
Undo whitespace change here

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_ppapi.c
File ppapi/proxy/irt_ppapi.c (right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_ppapi.c...
ppapi/proxy/irt_ppapi.c:44: static int ppapihook_pnacl_private_enabled = -1;
Since this will only be called once, you can leave out the complication of
trying to cache this (which is of slightly dubious thread safety).

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_ppapi.c...
ppapi/proxy/irt_ppapi.c:50: return ppapihook_pnacl_private_enabled;
Since this isn't thread safe, this could end up returning -1 (for true) instead
of 0.

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
File ppapi/proxy/irt_shim_ppapi.c (right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
ppapi/proxy/irt_shim_ppapi.c:11: #include
"ppapi/native_client/src/shared/ppapi_proxy/ppruntime.h"
You shouldn't need ppruntime.h.  It's now for user code only, not for IRT code.

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
ppapi/proxy/irt_shim_ppapi.c:20: * should not use that IRT hook. Instead PNaCl
nexes should embed the
Missing noun?  "Stable nexes should not use...".  They won't be able to use it
anyway, since it's meant to be hidden -- you might mention that here.

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
ppapi/proxy/irt_shim_ppapi.c:22: * for stability.
"ABI stability" (to clarify)

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
ppapi/proxy/irt_shim_ppapi.c:25: static struct PP_StartFunctions
user_start_functions;
Nit: add "g_" prefix to warn about use of global state

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
ppapi/proxy/irt_shim_ppapi.c:29: /* Record the original PPB_GetInterface and
provide a shimmed one. */
Nit: fix indentation to line up

[jvoung (off chromium), 2014-02-27 01:39:55]

Thanks!

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
File ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp
(right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp:39:
'-DPNACL_SHIM_AOT=1',
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> I don't see a corresponding -DPNACL_SHIM_AOT=0, which you should really have
if
> you're using "#if PNACL_SHIM_AOT" (which is preferable to #ifdef).
> 
> But if there's no good place for -DPNACL_SHIM_AOT=0, maybe use
> "-DPNACL_SHIM_AOT" and #ifdef instead?

The irt_shim_ppapi.h does check PNACL_SHIM_AOT, but that's #include'ed by
irt_ppapi.c in ppapi/proxy, as well as by the files here... so rather than put
compile flags in both gyp files I think it'll be better to just use
-DPNACL_SHIM_AOT and #ifdef.

Also moved irt_shim_ppapi to this directory so that the (direct) references to
PNACL_SHIM_AOT are all in the same directory.

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp:97: ],
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> This version doesn't have "--strip-debug".  Is there a reason for having it
(and
> then not having it)?

This will get linked into the IRT, which may get stripped by a later step anyway
(but there might be a full debug version of that stashed somewhere), so it
seemed better to let the IRT build take care of stripping as needed.


The old version has strip-debug because it was a library that would get packaged
w/ the pnacl-translator and I wanted to keep the whole set of translator files
small.


> 
> Is there a reason for using type=none?

I had thought this is the usual thing for cross compiled NaCl libraries. I'm not
entirely sure what the default would be.

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
File ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.h (right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.h:7: #define
PPAPI_NATIVE_CLIENT_SRC_UNTRUSTED_PNACL_IRT_SHIM_PNACL_SHIM_H_ 1
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> Chrome style is not to have the "1" here, so you don't need this change

Done.

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
File ppapi/native_client/src/untrusted/pnacl_irt_shim/shim_ppapi.c (right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/shim_ppapi.c:115: * For the
PNaCl in-the-browser, redirect to using
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> Nit: drop "the"?

Done.

https://codereview.chromium.org/164373010/diff/260001/ppapi/ppapi_proxy.gypi
File ppapi/ppapi_proxy.gypi (right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/ppapi_proxy.gypi#...
ppapi/ppapi_proxy.gypi:242: 'target_conditions': [
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> Undo whitespace change here

Done.

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_ppapi.c
File ppapi/proxy/irt_ppapi.c (right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_ppapi.c...
ppapi/proxy/irt_ppapi.c:44: static int ppapihook_pnacl_private_enabled = -1;
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> Since this will only be called once, you can leave out the complication of
> trying to cache this (which is of slightly dubious thread safety).

True it'll only be called on startup -- done.

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_ppapi.c...
ppapi/proxy/irt_ppapi.c:50: return ppapihook_pnacl_private_enabled;
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> Since this isn't thread safe, this could end up returning -1 (for true)
instead
> of 0.

Changed to not use the static int.

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
File ppapi/proxy/irt_shim_ppapi.c (right):

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
ppapi/proxy/irt_shim_ppapi.c:11: #include
"ppapi/native_client/src/shared/ppapi_proxy/ppruntime.h"
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> You shouldn't need ppruntime.h.  It's now for user code only, not for IRT
code.

Done.

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
ppapi/proxy/irt_shim_ppapi.c:20: * should not use that IRT hook. Instead PNaCl
nexes should embed the
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> Missing noun?  "Stable nexes should not use...".  They won't be able to use it
> anyway, since it's meant to be hidden -- you might mention that here.

Done.

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
ppapi/proxy/irt_shim_ppapi.c:22: * for stability.
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> "ABI stability" (to clarify)

Done.

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
ppapi/proxy/irt_shim_ppapi.c:25: static struct PP_StartFunctions
user_start_functions;
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> Nit: add "g_" prefix to warn about use of global state

Done.

https://codereview.chromium.org/164373010/diff/260001/ppapi/proxy/irt_shim_pp...
ppapi/proxy/irt_shim_ppapi.c:29: /* Record the original PPB_GetInterface and
provide a shimmed one. */
On 2014/02/26 22:11:03, Mark Seaborn wrote:
> Nit: fix indentation to line up

Done.

[Mark Seaborn, 2014-02-27 17:24:07]

LGTM

https://codereview.chromium.org/164373010/diff/280001/ppapi/native_client/src...
File ppapi/native_client/src/untrusted/pnacl_irt_shim/irt_shim_ppapi.h (right):

https://codereview.chromium.org/164373010/diff/280001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/irt_shim_ppapi.h:16: #else
Nit: add empty lines around #else/#ifdef/#endif for readability?

https://codereview.chromium.org/164373010/diff/280001/ppapi/native_client/src...
File ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp
(right):

https://codereview.chromium.org/164373010/diff/280001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp:32:
'irt_shim_ppapi.c',
Nit: sort list

[jvoung (off chromium), 2014-02-27 21:55:45]

+dmichael for ppapi OWNERS
+binji for native_client_sdk OWNERS

https://codereview.chromium.org/164373010/diff/280001/ppapi/native_client/src...
File ppapi/native_client/src/untrusted/pnacl_irt_shim/irt_shim_ppapi.h (right):

https://codereview.chromium.org/164373010/diff/280001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/irt_shim_ppapi.h:16: #else
On 2014/02/27 17:24:07, Mark Seaborn wrote:
> Nit: add empty lines around #else/#ifdef/#endif for readability?

Done.

https://codereview.chromium.org/164373010/diff/280001/ppapi/native_client/src...
File ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp
(right):

https://codereview.chromium.org/164373010/diff/280001/ppapi/native_client/src...
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_irt_shim.gyp:32:
'irt_shim_ppapi.c',
On 2014/02/27 17:24:07, Mark Seaborn wrote:
> Nit: sort list

Done.

[dmichael (off chromium), 2014-02-27 23:14:58]

https://codereview.chromium.org/164373010/diff/290001/ppapi/proxy/irt_ppapi.c
File ppapi/proxy/irt_ppapi.c (right):

https://codereview.chromium.org/164373010/diff/290001/ppapi/proxy/irt_ppapi.c...
ppapi/proxy/irt_ppapi.c:8: #include
"native_client/src/trusted/service_runtime/include/sys/unistd.h"
It seems wrong to include something from "trusted" here...? Is there another
place you can get it? I would like chromium to ultimately to include only from
someplace like native_client/src/public.

[Mark Seaborn, 2014-02-27 23:28:08]

https://codereview.chromium.org/164373010/diff/290001/ppapi/proxy/irt_ppapi.c
File ppapi/proxy/irt_ppapi.c (right):

https://codereview.chromium.org/164373010/diff/290001/ppapi/proxy/irt_ppapi.c...
ppapi/proxy/irt_ppapi.c:8: #include
"native_client/src/trusted/service_runtime/include/sys/unistd.h"
On 2014/02/27 23:14:59, dmichael wrote:
> It seems wrong to include something from "trusted" here...? Is there another
> place you can get it? I would like chromium to ultimately to include only from
> someplace like native_client/src/public.

That's covered by:
https://code.google.com/p/nativeclient/issues/detail?id=2832

We want to fix this, but I wouldn't want to block this change on fixing this
particular case.  There's lots of #includes from service_runtime/include/, but
these headers are subtle and tricky to move.

I already did one cleanup to make Jan's change cleaner (the newer parts of
https://code.google.com/p/nativeclient/issues/detail?id=3787).

[dmichael (off chromium), 2014-02-27 23:58:53]

OK, ppapi lgtm

[jvoung (off chromium), 2014-02-28 02:45:08]

Hmm, what's the limit on the IRT's data/rodata? Is it 1MB? If so, non-text
sections are getting close to 1MB.

I see "NaClElfImageLoadDynamically: failed 
to map data segment" on a 32-bit build (but not 64-bit).

I could try to add -fdata-sections and -gc-sections flags to try and reduce the
size...

[Mark Seaborn, 2014-02-28 03:33:44]

On 27 February 2014 18:45, <jvoung@chromium.org> wrote:

> Hmm, what's the limit on the IRT's data/rodata? Is it 1MB? If so, non-text
> sections are getting close to 1MB.
>

From untrusted.gypi:

    # See native_client/Sconstruct for more details.
    # Expected address for beginning of data in for the IRT.
    'NACL_IRT_DATA_START': '0x3ef00000',

That leaves 17MB on 1GB-address-space systems, but 16MB of that is for the
stack, I think, so 1MB, yes.


> I see "NaClElfImageLoadDynamically: failed
> to map data segment" on a 32-bit build (but not 64-bit).
>

Yeah, the segment address is not optimal for x86-64 -- it will fragment the
4GB address space more than necessary.


> I could try to add -fdata-sections and -gc-sections flags to try and
> reduce the
> size...


We should probably figure out how to automate the placement.  Maybe get
build_nexe.py to link it twice: first to estimate the size, then with the
load addresses chosen.

Mark

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Mark Seaborn, 2014-02-28 04:00:14]

On 27 February 2014 19:33, Mark Seaborn <mseaborn@chromium.org> wrote:

> On 27 February 2014 18:45, <jvoung@chromium.org> wrote:
>
>> I could try to add -fdata-sections and -gc-sections flags to try and
>> reduce the
>>
> size...
>
>
> We should probably figure out how to automate the placement.  Maybe get
> build_nexe.py to link it twice: first to estimate the size, then with the
> load addresses chosen.
>

Or otherwise, it would make sense for the segment sizes to be tweakable via
command line options to build_nexe.py so that they can be adjusted from the
Chromium repo rather than requiring a DEPS roll...

Mark

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jvoung (off chromium), 2014-02-28 19:57:09]

On 2014/02/28 03:33:44, Mark Seaborn wrote:
> On 27 February 2014 18:45, <mailto:jvoung@chromium.org> wrote:
> 
> > Hmm, what's the limit on the IRT's data/rodata? Is it 1MB? If so, non-text
> > sections are getting close to 1MB.
> >
> 
> From untrusted.gypi:
> 
>     # See native_client/Sconstruct for more details.
>     # Expected address for beginning of data in for the IRT.
>     'NACL_IRT_DATA_START': '0x3ef00000',
> 
> That leaves 17MB on 1GB-address-space systems, but 16MB of that is for the
> stack, I think, so 1MB, yes.
> 
> 
> > I see "NaClElfImageLoadDynamically: failed
> > to map data segment" on a 32-bit build (but not 64-bit).
> >
> 
> Yeah, the segment address is not optimal for x86-64 -- it will fragment the
> 4GB address space more than necessary.
> 
> 
> > I could try to add -fdata-sections and -gc-sections flags to try and
> > reduce the
> > size...
> 
> 
> We should probably figure out how to automate the placement.  Maybe get
> build_nexe.py to link it twice: first to estimate the size, then with the
> load addresses chosen.
> 
> Mark
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.


I like the re-linking idea since we don't need to worry about keeping an eye on
these limits as much, e.g., when we overflow the limit on ARM, we might not know
for a while since the ARM tests are not on the main chrome waterfall/trybots?
The PNaCl debug build is also different from the release build (uses -O0, etc.)
since we wanted to make the incremental build time faster and that is more
likely to overflow than the release build, so that the overflow might cause pain
for developers doing debug builds.

Downsides are that we aren't as aware of things getting bigger, and the
incremental build time could be a shade longer since it has to link twice. The
automatic sizing will also need to be aware of the stack limit, etc. but we can
pass that in as a parameter to build_nexe.py?

Reducing the size would be orthogonal but would be a good thing too. Any
downsides to -f*-sections, -gc-sections? I guess the intermediate object files
will be bigger.

[Mark Seaborn, 2014-02-28 20:07:12]

On 28 February 2014 11:57, <jvoung@chromium.org> wrote:

> On 2014/02/28 03:33:44, Mark Seaborn wrote:
>
>  On 27 February 2014 18:45, <mailto:jvoung@chromium.org> wrote:
>>
>
>  > Hmm, what's the limit on the IRT's data/rodata? Is it 1MB? If so,
>> non-text
>> > sections are getting close to 1MB.
>> >
>>
>
>   From untrusted.gypi:
>>
>
>       # See native_client/Sconstruct for more details.
>>      # Expected address for beginning of data in for the IRT.
>>      'NACL_IRT_DATA_START': '0x3ef00000',
>>
>
>  That leaves 17MB on 1GB-address-space systems, but 16MB of that is for the
>> stack, I think, so 1MB, yes.
>>
>
>
>  > I see "NaClElfImageLoadDynamically: failed
>> > to map data segment" on a 32-bit build (but not 64-bit).
>> >
>>
>
>  Yeah, the segment address is not optimal for x86-64 -- it will fragment
>> the
>> 4GB address space more than necessary.
>>
>
>
>  > I could try to add -fdata-sections and -gc-sections flags to try and
>> > reduce the
>> > size...
>>
>
>
>  We should probably figure out how to automate the placement.  Maybe get
>> build_nexe.py to link it twice: first to estimate the size, then with the
>> load addresses chosen.
>>
>
>> I like the re-linking idea since we don't need to worry about keeping an
> eye on
> these limits as much, e.g., when we overflow the limit on ARM, we might
> not know
> for a while since the ARM tests are not on the main chrome
> waterfall/trybots?
> The PNaCl debug build is also different from the release build (uses -O0,
> etc.)
> since we wanted to make the incremental build time faster and that is more
> likely to overflow than the release build, so that the overflow might
> cause pain
> for developers doing debug builds.
>

Sounds good to me.


> Downsides are that we aren't as aware of things getting bigger, and the
> incremental build time could be a shade longer since it has to link twice.



> The
> automatic sizing will also need to be aware of the stack limit, etc. but
> we can
> pass that in as a parameter to build_nexe.py?
>

The stack limit can be hard-coded in build_nexe.py if it's easier.  It only
needs to be kept in sync with NACL_DEFAULT_STACK_MAX in sel_ldr.h (in the
same repo) and it will likely never be changed.


> Reducing the size would be orthogonal but would be a good thing too. Any
> downsides to -f*-sections, -gc-sections? I guess the intermediate object
> files
> will be bigger.


No down sides that I know of.

Mark

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jvoung (off chromium), 2014-03-06 19:38:56]

Mark's roll of NaCl -> chrome should have reduced the IRT to be small enough
now. Running tryjobs

So, ping binji for NaCl SDK OWNERS?

[jvoung (off chromium), 2014-03-07 19:41:01]

or +sbc

[binji, 2014-03-07 20:29:49]

native_client_sdk lgtm

[jvoung (off chromium), 2014-03-07 20:50:46]

The CQ bit was checked by jvoung@chromium.org

[commit-bot: I haz the power, 2014-03-07 20:51:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/164373010/310001

[commit-bot: I haz the power, 2014-03-08 10:57:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/164373010/310001

[commit-bot: I haz the power, 2014-03-08 13:44:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-08 13:44:49]

Retried try job too often on mac_rel for step(s) telemetry_perf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[Mark Seaborn, 2014-03-08 16:24:50]

The CQ bit was checked by mseaborn@chromium.org

[commit-bot: I haz the power, 2014-03-08 16:25:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/164373010/310001

[commit-bot: I haz the power, 2014-03-08 17:04:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-08 17:04:20]

Retried try job too often on mac_rel for step(s) telemetry_perf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[jvoung (off chromium), 2014-03-09 02:00:28]

Committed patchset #11 manually as r255807 (presubmit successful).