Add a scons pnacl finalize step for pnacl_generate_pexe tests.

SConstruct

 #! -*- python -*-
 # Copyright (c) 2012 The Native Client Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import atexit
 import os
 import platform
 import re
 import subprocess
@@ skipping 334 matching lines @@
   BitFromArgument(env, 'x86_64_zero_based_sandbox', default=False,
     desc='Use the zero-address-based x86-64 sandbox model instead of '
       'the r15-based model.')
 
   BitFromArgument(env, 'android', default=False,
                   desc='Build for Android target')
 
   BitFromArgument(env, 'arm_hard_float', default=False,
                   desc='Build for hard float ARM ABI')
 
+  BitFromArgument(env, 'skip_nonstable_bitcode', default=False,
+                  desc='Skip tests involving non-stable bitcode')
+
   #########################################################################
   # EXPERIMENTAL
   # This is for generating a testing library for use within private test
   # enuminsts, where we want to compare and test different validators.
   #
   BitFromArgument(env, 'ncval_testing', default=False,
     desc='EXPERIMENTAL: Compile validator code for testing within enuminsts')
 
   # PNaCl sanity checks
   if ((env.Bit('pnacl_generate_pexe') or env.Bit('use_sandboxed_translator'))
@@ skipping 1188 matching lines @@
 def ShouldUseVerboseOptions(env, extra):
   """ Heuristic for setting up Verbose NACLLOG options. """
   return ('process_output_single' in extra or
           'log_golden' in extra)
 
 pre_base_env.AddMethod(ShouldUseVerboseOptions)
 
 
 DeclareBit('tests_use_irt', 'Non-browser tests also load the IRT image', False)
 
-# Translate the given pexe.
-def GetTranslatedNexe(env, pexe):
+# Bit to be set by individual test/nacl.scons files that need to opt out.
+DeclareBit('nonstable_bitcode', 'Tests use non-stable bitcode features', False)
+
+def GetFinalizedPexe(env, pexe):

[Mark Seaborn, 2013/03/28 18:44:17]

Nit: 2 empty lines before this to separate top-level decls

[jvoung (off chromium), 2013/03/28 19:45:31]

Done.

+  """ Prep and finalize the ABI for a given pexe if needed.
+  """
+  if not env.Bit('pnacl_generate_pexe') or env.Bit('nonstable_bitcode'):
+    return pexe
+
+  # There is no bitcode for trusted code.

[Mark Seaborn, 2013/03/28 18:44:17]

BTW, is this check necessary?  Surely no pexes can be generated in a trusted env
anyway?  You could make this an assertion if you think having a check is useful.

[jvoung (off chromium), 2013/03/28 19:45:31]

There are a couple of CommandSelLdrTestNaCl() coming from one build.scons file,
where the nexe is a canned nexe (old_abi, negative_hole, etc.):

find src/ -name "build.scons" | xargs grep "CommandSelLdrTestNacl"
src/trusted/service_runtime/build.scons:node = env.CommandSelLdrTestNacl(
src/trusted/service_runtime/build.scons:node = env.CommandSelLdrTestNacl(
src/trusted/service_runtime/build.scons:  node = env.CommandSelLdrTestNacl(
src/trusted/service_runtime/build.scons:  node = env.CommandSelLdrTestNacl(
src/trusted/service_runtime/build.scons:node =
env.CommandSelLdrTestNacl('unaligned_data.out', unaligned_data_nexe)
src/trusted/service_runtime/build.scons:node = env.CommandSelLdrTestNacl(
src/trusted/service_runtime/build.scons:  node = env.CommandSelLdrTestNacl(
src/trusted/service_runtime/build.scons:  node = env.CommandSelLdrTestNacl(
src/trusted/service_runtime/build.scons:  node = env.CommandSelLdrTestNacl(

+  if env['NACL_BUILD_FAMILY'] == 'TRUSTED':
+    return pexe
+
+  # Otherwise, finalize during the build step, since there is no finalize tool
+  # that can run on triggered bots such as the ARM HW bots.
   pexe_name = pexe.abspath
-  nexe_name = pexe_name[:pexe_name.index('.pexe')] + '.nexe'
+  final_name = pexe_name[:pexe_name.index('.nonfinal.pexe')] + '.final.pexe'

[Mark Seaborn, 2013/03/28 18:44:17]

I think you missed this before:

On 2013/03/28 01:31:34, Mark Seaborn wrote:
Looking at this again, using .index() makes this a little safer, but dodgy in a
different way, because ".nonfinal.pexe" *could* appear somewhere other than the
end of the string.

[jvoung (off chromium), 2013/03/28 19:45:31]

Ah yes, the StripSuffix helper is safer -- using the asserting version now.

   # Make sure the pexe doesn't get removed by the fake builders when
   # built_elsewhere=1
   env.Precious(pexe)
-  node = env.Command(target=nexe_name, source=[pexe_name],
-                     action=[Action('${TRANSLATECOM}', '${TRANSLATECOMSTR}')])
+  node = env.Command(target=final_name, source=[pexe_name],
+                     action=[Action('${PNACLFINALIZECOM}',
+                                    '${PNACLFINALIZECOMSTR}')])
   assert len(node) == 1, node
   return node[0]
 
-pre_base_env.AddMethod(GetTranslatedNexe)
 
+# Translate the given pexe.
+def GetTranslatedNexe(env, pexe):
+  # First finalize the pexe.
+  pexe = GetFinalizedPexe(env, pexe)
 
-def ShouldTranslateToNexe(env, pexe):
-  """ Determine when we need to translate a Pexe to a Nexe.
-  """
+  # Then check if we need to translate.
   # Check if we started with a pexe, so there is actually a translation step.
   if not env.Bit('pnacl_generate_pexe'):
-    return False
+    return pexe
 
   # There is no bitcode for trusted code.
   if env['NACL_BUILD_FAMILY'] == 'TRUSTED':
-    return False
+    return pexe
 
   # Often there is a build step (do_not_run_tests=1) and a test step
   # (which is run with -j1). Normally we want to translate in the build step
   # so we can translate in parallel. However when we do sandboxed translation
   # on arm hw, we do the build step on x86 and translation on arm, so we have
   # to force the translation to be done in the test step. Hence,
   # we check the bit 'translate_in_build_step' / check if we are
   # in the test step.
-  return (
-    env.Bit('translate_in_build_step') or not env.Bit('do_not_run_tests'))
+  if not env.Bit('translate_in_build_step') and env.Bit('do_not_run_tests'):
+    return pexe
 
-pre_base_env.AddMethod(ShouldTranslateToNexe)
+  pexe_name = pexe.abspath
+  nexe_name = pexe_name[:pexe_name.index('.pexe')] + '.nexe'

[Mark Seaborn, 2013/03/28 18:44:17]

Can you add an assertion here too, please?
i.e.
assert pexe_name.endswith('.pexe')

Also, should you be stripping off the full ".{non,}final.pexe" suffix?  That
would make the .nexe names more concise.

You could add a helper function:

def StripSuffix(string, suffix):
  assert string.endswith(suffix)
  return string[:-len(suffix)]

or

def StripSuffix(string, suffix):
  if string.endswith(suffix):
    return string[:-len(suffix)]
  return string

[jvoung (off chromium), 2013/03/28 19:45:31]

Done (using the asserting version), and simplified the nexe name.

+  # Make sure the pexe doesn't get removed by the fake builders when
+  # built_elsewhere=1
+  env.Precious(pexe)
+  node = env.Command(target=nexe_name, source=[pexe_name],
+                     action=[Action('${TRANSLATECOM}', '${TRANSLATECOMSTR}')])
+  assert len(node) == 1, node
+  return node[0]
+
+pre_base_env.AddMethod(GetTranslatedNexe)
 
 
 def CommandTestFileDumpCheck(env,
                              name,
                              target,
                              check_file,
                              objdump_flags):
   """Create a test that disassembles a binary (|target|) and checks for
   patterns in the |check_file|.  Disassembly is done using |objdump_flags|.
   """
 
   # Do not try to run OBJDUMP if 'built_elsewhere', since that *might* mean
   # that a toolchain is not even present.  E.g., the arm hw buildbots do
   # not have the pnacl toolchain. We should be able to look for the host
   # ARM objdump though... a TODO(jvoung) for when there is time.
   if env.Bit('built_elsewhere'):
     return []
-  if env.ShouldTranslateToNexe(target):
-    target_obj = env.GetTranslatedNexe(target)
-  else:
-    target_obj = target
+  target = env.GetTranslatedNexe(target)
   return env.CommandTestFileCheck(name,
-                                  ['${OBJDUMP}', objdump_flags, target_obj],
+                                  ['${OBJDUMP}', objdump_flags, target],
                                   check_file)
 
 pre_base_env.AddMethod(CommandTestFileDumpCheck)
 
 
 def CommandTestFileCheck(env, name, cmd, check_file):
   """Create a test that runs a |cmd| (array of strings),
   which is expected to print to stdout.  The results
   of stdout will then be piped to the file_check.py tool which
   will search for the regexes specified in |check_file|. """
@@ skipping 20 matching lines @@
                           # e.g., [ 'python', 'time_check.py', '--' ]
                           **extra):
   # Disable all sel_ldr tests for windows under coverage.
   # Currently several .S files block sel_ldr from being instrumented.
   # See http://code.google.com/p/nativeclient/issues/detail?id=831
   if ('TRUSTED_ENV' in env and
       env['TRUSTED_ENV'].Bit('coverage_enabled') and
       env['TRUSTED_ENV'].Bit('windows')):
     return []
 
-  if env.ShouldTranslateToNexe(nexe):
-    # The nexe is actually a pexe.  Translate it before we run it.
-    nexe = env.GetTranslatedNexe(nexe)
+  # The nexe might be a pexe that needs finalization, and translation.
+  nexe = env.GetTranslatedNexe(nexe)
+
   command = [nexe]
   if args is not None:
     command += args
 
   if loader is None:
     loader = env.GetSelLdr()
     if loader is None:
       print 'WARNING: no sel_ldr found. Skipping test %s' % name
       return []
 
@@ skipping 2108 matching lines @@
   nacl_env.ValidateSdk()
 
 if BROKEN_TEST_COUNT > 0:
   msg = "There are %d broken tests." % BROKEN_TEST_COUNT
   if GetOption('brief_comstr'):
     msg += " Add --verbose to the command line for more information."
   print msg
 
 # separate warnings from actual build output
 Banner('B U I L D - O U T P U T:')