NonSFI Mode: Make hello_world test run with nonsfi_nacl=1 bitcode=1 pnacl_generate_pexe=0.

NonSFI Mode:  Make hello_world test run with nonsfi_nacl=1 bitcode=1 pnacl_generate_pexe=0.

It was compiling as IRT binary that requires to be loaded via a loader
when it was supposed to be building without IRT.

Disable tests/pnacl_native_objects/nacl.scons for now since it seem to error out at scons time.



TESTED=./scons -j20 --mode=dbg-host,nacl,nacl_irt_test \
  platform=x86-32 nonsfi_nacl=1 bitcode=1 pnacl_generate_pexe=0 \
  run_hello_world_test
TESTED= ./scons -j20 --mode=dbg-host,nacl,nacl_irt_test \
  run_hello_world_test_irt platform=x86-32 nonsfi_nacl=1 bitcode=1 \
  pnacl_generate_pexe=0

BUG=https://code.google.com/p/chromium/issues/detail?id=408879

Committed: http://src.chromium.org/viewvc/native_client?view=rev&revision=13797

[Junichi Uekawa, 2014-09-25 04:56:49]

uekawa@chromium.org changed reviewers:
+ hamaji@chromium.org, mseaborn@chromium.org

[Junichi Uekawa, 2014-09-25 04:57:10]

uekawa@chromium.org changed reviewers:
+ hidehiko@chromium.org

[Junichi Uekawa, 2014-09-25 04:57:10]

ptal

[Mark Seaborn, 2014-09-25 17:08:09]

https://codereview.chromium.org/599353002/diff/1/SConstruct
File SConstruct (right):

https://codereview.chromium.org/599353002/diff/1/SConstruct#newcode3704
SConstruct:3704: '--pnacl-allow-native',
So this fails on the bots, with:
pnacl-clang++: --pnacl-allow-native without -arch is not meaningful.

'--pnacl-allow-native' is required to use '-Wt,--noirt'.  '-Wt,--noirt' is also
rejected when linking a pexe.

How about doing this:

+  if nacl_env.Bit('pnacl_generate_pexe'):
+    # Not-IRT-using non-SFI code uses Linux syscalls directly.  Since this
+    # involves using inline assembly, this requires turning off the PNaCl
+    # ABI checker.
+    nacl_env.SetBits('nonstable_bitcode')
+    nacl_env.Append(LINKFLAGS=['--pnacl-disable-abi-check'])
+    # Tell the PNaCl translator to link a Linux executable.
+    nacl_env.Append(TRANSLATEFLAGS=['--noirt'])
+  else:
+    nacl_env.Append(LINKFLAGS=['--pnacl-allow-native', '-Wt,--noirt'])

This is also more self-documenting, since it indicates which flags are required
in the pnacl_generate_pexe=0,1 cases.

[Junichi Uekawa, 2014-09-25 23:05:08]

Let's see if the bots are happy.

https://codereview.chromium.org/599353002/diff/1/SConstruct
File SConstruct (right):

https://codereview.chromium.org/599353002/diff/1/SConstruct#newcode3704
SConstruct:3704: '--pnacl-allow-native',
On 2014/09/25 17:08:09, Mark Seaborn wrote:
> So this fails on the bots, with:
> pnacl-clang++: --pnacl-allow-native without -arch is not meaningful.
> 
> '--pnacl-allow-native' is required to use '-Wt,--noirt'.  '-Wt,--noirt' is
also
> rejected when linking a pexe.
> 
> How about doing this:
> 
> +  if nacl_env.Bit('pnacl_generate_pexe'):
> +    # Not-IRT-using non-SFI code uses Linux syscalls directly.  Since this
> +    # involves using inline assembly, this requires turning off the PNaCl
> +    # ABI checker.
> +    nacl_env.SetBits('nonstable_bitcode')
> +    nacl_env.Append(LINKFLAGS=['--pnacl-disable-abi-check'])
> +    # Tell the PNaCl translator to link a Linux executable.
> +    nacl_env.Append(TRANSLATEFLAGS=['--noirt'])
> +  else:
> +    nacl_env.Append(LINKFLAGS=['--pnacl-allow-native', '-Wt,--noirt'])
> 
> This is also more self-documenting, since it indicates which flags are
required
> in the pnacl_generate_pexe=0,1 cases.

Done.

[Junichi Uekawa, 2014-09-26 01:17:04]

ptal


seems like trybots are now happy with this.

[Mark Seaborn, 2014-09-26 01:31:07]

LGTM.  Please use an unabbreviated URL in BUG= (not crbug) so that Bugdroid
recognises it.

https://codereview.chromium.org/599353002/diff/40001/tests/pnacl_native_objec...
File tests/pnacl_native_objects/nacl.scons (right):

https://codereview.chromium.org/599353002/diff/40001/tests/pnacl_native_objec...
tests/pnacl_native_objects/nacl.scons:13: if env.Bit('nonsfi_nacl'):
It's not nonsfi_nacl=1 that causes this file to fail.  It's
"--mode=dbg-host,nacl,nacl_irt_test pnacl_generate_pexe=0".

So it would be more appropriate to write:

# This file fails with "--mode=dbg-host,nacl,nacl_irt_test
# pnacl_generate_pexe=0", so disable it for that case.
# TODO(uekawa): Investigate why this fails in that case and re-enable.
if env.Bit('tests_use_irt'):
  Return()

(The cause is probably something to do with this file's use of library paths and
Replicate().)

Actually, a simpler fix is to modify SConstruct to move this nacl.scons file
from irt_variant_tests to nonvariant_tests.

[Junichi Uekawa, 2014-09-26 01:59:50]

https://codereview.chromium.org/599353002/diff/40001/tests/pnacl_native_objec...
File tests/pnacl_native_objects/nacl.scons (right):

https://codereview.chromium.org/599353002/diff/40001/tests/pnacl_native_objec...
tests/pnacl_native_objects/nacl.scons:13: if env.Bit('nonsfi_nacl'):
On 2014/09/26 01:31:07, Mark Seaborn wrote:
> It's not nonsfi_nacl=1 that causes this file to fail.  It's
> "--mode=dbg-host,nacl,nacl_irt_test pnacl_generate_pexe=0".
> 
> So it would be more appropriate to write:
> 
> # This file fails with "--mode=dbg-host,nacl,nacl_irt_test
> # pnacl_generate_pexe=0", so disable it for that case.
> # TODO(uekawa): Investigate why this fails in that case and re-enable.
> if env.Bit('tests_use_irt'):
>   Return()
> 
> (The cause is probably something to do with this file's use of library paths
and
> Replicate().)
> 
> Actually, a simpler fix is to modify SConstruct to move this nacl.scons file
> from irt_variant_tests to nonvariant_tests.

hmm.. I see this is not just nonsfi nacl that breaks.
updated the comment here.

[Junichi Uekawa, 2014-09-26 03:39:51]

The CQ bit was checked by uekawa@chromium.org

[commit-bot: I haz the power, 2014-09-26 03:40:23]

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

[commit-bot: I haz the power, 2014-09-26 03:40:30]

Committed patchset #4 (id:60001) as 13797