[foozzie] Refactor command abstraction

This moves code for running d8 into its own class. No functional
changes intended.

No-Try: true
Bug: chromium:1023091
Change-Id: I7cbfeebd2911dc758322f89cf93666550f2956d9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1906378
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Tamer Tas <tmrts@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64928}

tools/clusterfuzz/v8_commands.py

@@ -4,11 +4,105 @@
 
 # Fork from commands.py and output.py in v8 test driver.
 
+import json
+import os
 import signal
 import subprocess
 import sys
 from threading import Event, Timer
 
+import v8_fuzz_config
+
+# List of default flags passed to each d8 run.
+DEFAULT_FLAGS = [
+  '--correctness-fuzzer-suppressions',
+  '--expose-gc',
+  '--allow-natives-syntax',
+  '--invoke-weak-callbacks',
+  '--omit-quit',
+  '--es-staging',
+  '--wasm-staging',
+  '--no-wasm-async-compilation',
+  '--suppress-asm-messages',
+]
+
+BASE_PATH = os.path.dirname(os.path.abspath(__file__))
+
+# List of files passed to each d8 run before the testcase.
+DEFAULT_FILES = [
+  os.path.join(BASE_PATH, 'v8_mock.js'),
+  os.path.join(BASE_PATH, 'v8_suppressions.js'),
+]
+
+# Architecture-specific mock file
+ARCH_MOCKS = os.path.join(BASE_PATH, 'v8_mock_archs.js')
+
+DEFAULT_ARCH = 'x64'
+SUPPORTED_ARCHS = ['ia32', 'x64', 'arm', 'arm64']
+
+# Timeout in seconds for one d8 run.
+TIMEOUT = 3
+
+
+def _infer_v8_architecture(executable):
+  """Infer the V8 architecture from the build configuration next to the
+  executable.
+  """
+  build_dir = os.path.dirname(executable)
+  with open(os.path.join(build_dir, 'v8_build_config.json')) as f:
+    arch = json.load(f)['v8_current_cpu']
+  arch = 'ia32' if arch == 'x86' else arch
+  assert arch in SUPPORTED_ARCHS
+  return arch
+
+
+def _startup_files(arch):
+  """Default files and optional architecture-specific mock file."""
+  files = DEFAULT_FILES[:]
+  if arch != DEFAULT_ARCH:
+    files.append(ARCH_MOCKS)
+  return files
+
+
+class Command(object):
+  """Represents a configuration for running V8 multiple times with certain
+  flags and files.
+  """
+  def __init__(self, label, executable, random_seed, config_flags):
+    self.label = label
+    self.executable = executable
+    self.config_flags = config_flags
+    self.common_flags =  DEFAULT_FLAGS + ['--random-seed', str(random_seed)]
+
+    # Ensure absolute paths.
+    if not os.path.isabs(self.executable):
+      self.executable = os.path.join(BASE_PATH, self.executable)
+
+    # Ensure executables exist.
+    assert os.path.exists(self.executable)
+
+    self.arch = _infer_v8_architecture(self.executable)
+    self.files = _startup_files(self.arch)
+
+  def run(self, testcase, verbose=False):
+    """Run the executable with a specific testcase."""
+    args = [self.executable] + self.flags + self.files + [testcase]
+    if verbose:
+      print('# Command line for %s comparison:' % self.label)
+      print(' '.join(args))
+    if self.executable.endswith('.py'):
+      # Wrap with python in tests.
+      args = [sys.executable] + args
+    return Execute(
+        args,
+        cwd=os.path.dirname(os.path.abspath(testcase)),
+        timeout=TIMEOUT,
+    )
+
+  @property
+  def flags(self):
+    return self.common_flags + self.config_flags
+
 
 class Output(object):
   def __init__(self, exit_code, timed_out, stdout, pid):
@@ -50,7 +144,6 @@ def Execute(args, cwd, timeout=None):
     except OSError:
       sys.stderr.write('Error: Process %s already ended.\n' % process.pid)
 
-
   timer = Timer(timeout, kill_process)
   timer.start()
   stdout, _ = process.communicate()

tools/clusterfuzz/v8_foozzie.py

@@ -13,7 +13,6 @@ from __future__ import print_function
 import argparse
 import hashlib
 import itertools
-import json
 import os
 import random
 import re
@@ -88,28 +87,13 @@ CONFIGS = dict(
   ],
 )
 
-# Timeout in seconds for one d8 run.
-TIMEOUT = 3
-
 # Return codes.
 RETURN_PASS = 0
 RETURN_FAIL = 2
 
 BASE_PATH = os.path.dirname(os.path.abspath(__file__))
-PREAMBLE = [
-  os.path.join(BASE_PATH, 'v8_mock.js'),
-  os.path.join(BASE_PATH, 'v8_suppressions.js'),
-]
-ARCH_MOCKS = os.path.join(BASE_PATH, 'v8_mock_archs.js')
 SANITY_CHECKS = os.path.join(BASE_PATH, 'v8_sanity_checks.js')
 
-FLAGS = ['--correctness-fuzzer-suppressions', '--expose-gc',
-         '--allow-natives-syntax', '--invoke-weak-callbacks', '--omit-quit',
-         '--es-staging', '--wasm-staging', '--no-wasm-async-compilation',
-         '--suppress-asm-messages']
-
-SUPPORTED_ARCHS = ['ia32', 'x64', 'arm', 'arm64']
-
 # Output for suppressed failure case.
 FAILURE_HEADER_TEMPLATE = """#
 # V8 correctness failure
@@ -158,15 +142,6 @@ ORIGINAL_SOURCE_HASH_LENGTH = 3
 ORIGINAL_SOURCE_DEFAULT = 'none'
 
 
-def infer_arch(d8):
-  """Infer the V8 architecture from the build configuration next to the
-  executable.
-  """
-  with open(os.path.join(os.path.dirname(d8), 'v8_build_config.json')) as f:
-    arch = json.load(f)['v8_current_cpu']
-  return 'ia32' if arch == 'x86' else arch
-
-
 def parse_args():
   parser = argparse.ArgumentParser()
   parser.add_argument(
@@ -203,27 +178,11 @@ def parse_args():
   # Use first d8 as default for second d8.
   options.second_d8 = options.second_d8 or options.first_d8
 
-  # Ensure absolute paths.
-  if not os.path.isabs(options.first_d8):
-    options.first_d8 = os.path.join(BASE_PATH, options.first_d8)
-  if not os.path.isabs(options.second_d8):
-    options.second_d8 = os.path.join(BASE_PATH, options.second_d8)
-
-  # Ensure executables exist.
-  assert os.path.exists(options.first_d8)
-  assert os.path.exists(options.second_d8)
-
   # Ensure we make a sane comparison.
   if (options.first_d8 == options.second_d8 and
       options.first_config == options.second_config):
     parser.error('Need either executable or config difference.')
 
-  # Infer architecture from build artifacts.
-  options.first_arch = infer_arch(options.first_d8)
-  options.second_arch = infer_arch(options.second_d8)
-
-  assert options.first_arch in SUPPORTED_ARCHS
-  assert options.second_arch in SUPPORTED_ARCHS
   assert options.first_config in CONFIGS
   assert options.second_config in CONFIGS
 
@@ -274,12 +233,12 @@ def fail_bailout(output, ignore_by_output_fun):
 
 
 def print_difference(
-    options, source_key, first_config_flags, second_config_flags,
+    options, source_key, first_command, second_command,
     first_config_output, second_config_output, difference, source=None):
   # The first three entries will be parsed by clusterfuzz. Format changes
   # will require changes on the clusterfuzz side.
-  first_config_label = '%s,%s' % (options.first_arch, options.first_config)
-  second_config_label = '%s,%s' % (options.second_arch, options.second_config)
+  first_config_label = '%s,%s' % (first_command.arch, options.first_config)
+  second_config_label = '%s,%s' % (second_command.arch, options.second_config)
   source_file_text = SOURCE_FILE_TEMPLATE % source if source else ''
   print((FAILURE_TEMPLATE % dict(
       configs='%s:%s' % (first_config_label, second_config_label),
@@ -288,8 +247,8 @@ def print_difference(
       suppression='', # We can't tie bugs to differences.
       first_config_label=first_config_label,
       second_config_label=second_config_label,
-      first_config_flags=' '.join(first_config_flags),
-      second_config_flags=' '.join(second_config_flags),
+      first_config_flags=' '.join(first_command.flags),
+      second_config_flags=' '.join(second_command.flags),
       first_config_output=
           first_config_output.stdout.decode('utf-8', 'replace'),
       second_config_output=
@@ -302,10 +261,21 @@ def print_difference(
 def main():
   options = parse_args()
 
+  # Set up runtime arguments.
+  first_config_flags = (CONFIGS[options.first_config] +
+                        options.first_config_extra_flags)
+  second_config_flags = (CONFIGS[options.second_config] +
+                         options.second_config_extra_flags)
+
+  first_cmd = v8_commands.Command(
+      'first', options.first_d8, options.random_seed, first_config_flags)
+  second_cmd = v8_commands.Command(
+      'second', options.second_d8, options.random_seed, second_config_flags)
+
   # Suppressions are architecture and configuration specific.
   suppress = v8_suppressions.get_suppression(
-      options.first_arch, options.first_config,
-      options.second_arch, options.second_config,
+      first_cmd.arch, options.first_config,
+      second_cmd.arch, options.second_config,
   )
 
   # Static bailout based on test case content or metadata.
@@ -316,37 +286,11 @@ def main():
   if content_bailout(content, suppress.ignore_by_content):
     return RETURN_FAIL
 
-  # Set up runtime arguments.
-  common_flags = FLAGS + ['--random-seed', str(options.random_seed)]
-  first_config_flags = (common_flags + CONFIGS[options.first_config] +
-                        options.first_config_extra_flags)
-  second_config_flags = (common_flags + CONFIGS[options.second_config] +
-                         options.second_config_extra_flags)
-
-  def run_d8(d8, config_flags, config_label=None, testcase=options.testcase):
-    preamble = PREAMBLE[:]
-    if options.first_arch != options.second_arch:
-      preamble.append(ARCH_MOCKS)
-    args = [d8] + config_flags + preamble + [testcase]
-    if config_label:
-      print('# Command line for %s comparison:' % config_label)
-      print(' '.join(args))
-    if d8.endswith('.py'):
-      # Wrap with python in tests.
-      args = [sys.executable] + args
-    return v8_commands.Execute(
-        args,
-        cwd=os.path.dirname(os.path.abspath(testcase)),
-        timeout=TIMEOUT,
-    )
-
   # Sanity checks. Run both configurations with the sanity-checks file only and
   # bail out early if different.
   if not options.skip_sanity_checks:
-    first_config_output = run_d8(
-        options.first_d8, first_config_flags, testcase=SANITY_CHECKS)
-    second_config_output = run_d8(
-        options.second_d8, second_config_flags, testcase=SANITY_CHECKS)
+    first_config_output = first_cmd.run(SANITY_CHECKS)
+    second_config_output = second_cmd.run(SANITY_CHECKS)
     difference, _ = suppress.diff(
         first_config_output.stdout, second_config_output.stdout)
     if difference:
@@ -354,18 +298,17 @@ def main():
       # cases on this in case it's hit.
       source_key = 'sanity check failed'
       print_difference(
-          options, source_key, first_config_flags, second_config_flags,
+          options, source_key, first_cmd, second_cmd,
           first_config_output, second_config_output, difference)
       return RETURN_FAIL
 
-  first_config_output = run_d8(options.first_d8, first_config_flags, 'first')
+  first_config_output = first_cmd.run(options.testcase, verbose=True)
 
   # Early bailout based on first run's output.
   if pass_bailout(first_config_output, 1):
     return RETURN_PASS
 
-  second_config_output = run_d8(
-      options.second_d8, second_config_flags, 'second')
+  second_config_output = second_cmd.run(options.testcase, verbose=True)
 
   # Bailout based on second run's output.
   if pass_bailout(second_config_output, 2):
@@ -389,7 +332,7 @@ def main():
       return RETURN_FAIL
 
     print_difference(
-        options, source_key, first_config_flags, second_config_flags,
+        options, source_key, first_cmd, second_cmd,
         first_config_output, second_config_output, difference, source)
     return RETURN_FAIL