Added 32-bit version of nonsfi monacl shell

mojo/nacl/BUILD.gn

 # Copyright 2015 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # Trusted code
 if (!is_nacl) {
   # A simple shell for running untrusted binaries that talk to the Mojo
   # embedder. (No services.)
   executable("monacl_shell") {
     testonly = true
     sources = [
       "monacl_shell.cc",
     ]
     deps = [
       "//base:base",
       "//mojo/edk/system:system",
       "//nacl_bindings:monacl_sel",
     ]
 
     data_deps = [ "//nacl_bindings:irt_mojo(//native_client/build/toolchain/nacl:irt_${target_cpu})" ]
   }
 
   executable("monacl_shell_nonsfi") {

[Sean Klein, 2015/09/09 17:37:07]

I don't have a solution for this at the moment, but I just want to point out
that I do not like that this executable is being built with an x64 toolchain (if
the default is x64), even though it is a useless binary if it is built that way.

(executables build without anything depending on them, and this one must be
defined to say "build it again with a 32-bit toolchain" in the
monacl_shell_nonsfi_x86 group).

[Sean Klein, 2015/09/09 17:55:45]

With the most recent patch, I've worked around this problem -- it is possible to
"deps" on a target which exists inside a conditional that has not yet been
evaluated.

     testonly = true
     sources = [
       "monacl_shell_nonsfi.cc",
     ]
     deps = [
       ":irt_mojo_nonsfi",
       "//mojo/edk/system",
       "//native_client/src/nonsfi/loader:elf_loader",
     ]
   }
 }
 
+group("monacl_shell_nonsfi_x86") {

[Mark Seaborn, 2015/09/08 23:57:43]

Should this be added as a dependency of group("mojo_nacl"), so that it's
included in the set of targets built by the bots?  Currently the bots don't
build it.

Also, I think this should be conditionalised on "x86-32 or x86-64", otherwise
we'd attempt to build it when targeting ARM.

[Sean Klein, 2015/09/09 17:37:07]

I think it should be a dependency of "mojo_nacl_tests", as it is a testonly
executable (and the regular monacl_shell is there too).

Also, adding the conditional to the deps -- this way, the target still exists
and can be referenced on ARM without throwing an error, it just doesn't build
anything.

+  testonly = true
+  deps = [
+    ":monacl_shell_nonsfi(//build/toolchain/linux:clang_x86)",

[Mark Seaborn, 2015/09/08 23:57:43]

Since this is Linux-specific, it seems like the target should be conditionalised
on Linux too.  Though I'm not sure whether the codebase as a whole is intended
to support non-Linux OSes.

Is there a way to do this without assuming Clang?  i.e. Is there a way to ask
for "the 32-bit variant of whatever toolchain we're using (whether it's GCC or
Clang or something else)"?  If not, can you add a comment to explain the
assumption and say it's not ideal?

[Sean Klein, 2015/09/09 17:37:07]

Added a conditional on "is_linux".

Comment added.

+  ]
+}
+
 # Untrusted code
 if (is_nacl) {
   # Unit test for the Mojo public API.
   executable("monacl_test") {
     testonly = true
     sources = [
       "//mojo/public/cpp/system/tests/core_unittest.cc",
       "//mojo/public/cpp/system/tests/macros_unittest.cc",
     ]
     deps = [
@@ skipping 54 matching lines @@
 }
 
 group("mojo_nacl_tests") {
   testonly = true
   deps = [
     ":mojo_nacl_tests_untrusted(//native_client/build/toolchain/nacl:clang_newlib_${current_cpu})",
     ":monacl_shell",
     ":mojo_pnacl_tests",
   ]
 }