[foozzie] Compare output before crashes

Crashes in the presence of RangeError happen often during differential
fuzzing. Until now we have ignored such cases completely.

After this change we compare as much output as possible when one or
both runs have crashed, dramatically increasing the coverage.

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

tools/clusterfuzz/v8_commands.py

@@ -105,8 +105,8 @@ def Execute(args, cwd, timeout=None):
     process = subprocess.Popen(
       args=popen_args,
       stdout=subprocess.PIPE,
-      stderr=subprocess.STDOUT,
-      cwd=cwd
+      stderr=subprocess.PIPE,
+      cwd=cwd,
     )
   except Exception as e:
     sys.stderr.write("Error executing: %s\n" % popen_args)

tools/clusterfuzz/v8_foozzie.py

@@ -256,16 +256,13 @@ def content_bailout(content, ignore_fun):
   return False
 
 
-def pass_bailout(output, step_number):
-  """Print info and return if in timeout or crash pass states."""
+def timeout_bailout(output, step_number):
+  """Print info and return if in timeout pass state."""
   if output.HasTimedOut():
     # Dashed output, so that no other clusterfuzz tools can match the
     # words timeout or crash.
     print('# V8 correctness - T-I-M-E-O-U-T %d' % step_number)
     return True
-  if output.HasCrashed():
-    print('# V8 correctness - C-R-A-S-H %d' % step_number)
-    return True
   return False
 
 
@@ -332,8 +329,7 @@ def main():
   if not options.skip_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)
+    difference, _ = suppress.diff(first_config_output, second_config_output)
     if difference:
       # Special source key for sanity checks so that clusterfuzz dedupes all
       # cases on this in case it's hit.
@@ -345,18 +341,17 @@ def main():
 
   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):
+  # Early bailout if first run was a timeout.
+  if timeout_bailout(first_config_output, 1):
     return RETURN_PASS
 
   second_config_output = second_cmd.run(options.testcase, verbose=True)
 
-  # Bailout based on second run's output.
-  if pass_bailout(second_config_output, 2):
+  # Bailout if second run was a timeout.
+  if timeout_bailout(second_config_output, 2):
     return RETURN_PASS
 
-  difference, source = suppress.diff(
-      first_config_output.stdout, second_config_output.stdout)
+  difference, source = suppress.diff(first_config_output, second_config_output)
 
   if source:
     source_key = hashlib.sha1(source).hexdigest()[:ORIGINAL_SOURCE_HASH_LENGTH]
@@ -377,11 +372,18 @@ def main():
         first_config_output, second_config_output, difference, source)
     return RETURN_FAIL
 
-  # TODO(machenbach): Figure out if we could also return a bug in case there's
-  # no difference, but one of the line suppressions has matched - and without
-  # the match there would be a difference.
+  # Show if a crash has happened in one of the runs and no difference was
+  # detected.
+  if first_config_output.HasCrashed():
+    print('# V8 correctness - C-R-A-S-H 1')
+  elif second_config_output.HasCrashed():
+    print('# V8 correctness - C-R-A-S-H 2')
+  else:
+    # TODO(machenbach): Figure out if we could also return a bug in case
+    # there's no difference, but one of the line suppressions has matched -
+    # and without the match there would be a difference.
+    print('# V8 correctness - pass')
 
-  print('# V8 correctness - pass')
   return RETURN_PASS
 
 

tools/clusterfuzz/v8_foozzie_test.py

@@ -9,6 +9,7 @@ import subprocess
 import sys
 import unittest
 
+import v8_commands
 import v8_foozzie
 import v8_fuzz_config
 import v8_suppressions
@@ -54,15 +55,18 @@ class UnitTest(unittest.TestCase):
     # TODO(machenbach): Mock out suppression configuration.
     suppress = v8_suppressions.get_suppression(
         'x64', 'ignition', 'x64', 'ignition_turbo')
+    def diff_fun(one, two):
+      return suppress.diff_lines(one.splitlines(), two.splitlines())
+
     one = ''
     two = ''
     diff = None, None
-    self.assertEquals(diff, suppress.diff(one, two))
+    self.assertEquals(diff, diff_fun(one, two))
 
     one = 'a \n  b\nc();'
     two = 'a \n  b\nc();'
     diff = None, None
-    self.assertEquals(diff, suppress.diff(one, two))
+    self.assertEquals(diff, diff_fun(one, two))
 
     # Ignore line before caret, caret position and error message.
     one = """
@@ -80,7 +84,7 @@ somefile.js: TypeError: baz is not a function
   undefined
 """
     diff = None, None
-    self.assertEquals(diff, suppress.diff(one, two))
+    self.assertEquals(diff, diff_fun(one, two))
 
     one = """
 Still equal
@@ -90,7 +94,7 @@ Extra line
 Still equal
 """
     diff = '- Extra line', None
-    self.assertEquals(diff, suppress.diff(one, two))
+    self.assertEquals(diff, diff_fun(one, two))
 
     one = """
 Still equal
@@ -100,7 +104,7 @@ Still equal
 Extra line
 """
     diff = '+ Extra line', None
-    self.assertEquals(diff, suppress.diff(one, two))
+    self.assertEquals(diff, diff_fun(one, two))
 
     one = """
 undefined
@@ -112,7 +116,39 @@ otherfile.js: TypeError: undefined is not a constructor
 """
     diff = """- somefile.js: TypeError: undefined is not a constructor
 + otherfile.js: TypeError: undefined is not a constructor""", None
-    self.assertEquals(diff, suppress.diff(one, two))
+    self.assertEquals(diff, diff_fun(one, two))
+
+  def testOutputCapping(self):
+    def output(stdout, is_crash):
+      exit_code = -1 if is_crash else 0
+      return v8_commands.Output(
+          exit_code=exit_code, timed_out=False, stdout=stdout, pid=0)
+
+    def check(stdout1, stdout2, is_crash1, is_crash2, capped_lines1,
+              capped_lines2):
+      output1 = output(stdout1, is_crash1)
+      output2 = output(stdout2, is_crash2)
+      self.assertEquals(
+          (capped_lines1.splitlines(), capped_lines2.splitlines()),
+          v8_suppressions.get_lines_capped(output1, output2))
+
+    # No capping, already equal.
+    check('1\n2', '1\n2', True, True, '1\n2', '1\n2')
+    # No crash, no capping.
+    check('1\n2', '1\n2\n3', False, False, '1\n2', '1\n2\n3')
+    check('1\n2\n3', '1\n2', False, False, '1\n2\n3', '1\n2')
+    # Cap smallest if all runs crash.
+    check('1\n2', '1\n2\n3', True, True, '1\n2', '1\n2')
+    check('1\n2\n3', '1\n2', True, True, '1\n2', '1\n2')
+    # Cap the non-crashy run.
+    check('1\n2\n3', '1\n2', False, True, '1\n2', '1\n2')
+    check('1\n2', '1\n2\n3', True, False, '1\n2', '1\n2')
+    # The crashy run has more output.
+    check('1\n2\n3', '1\n2', True, False, '1\n2\n3', '1\n2')
+    check('1\n2', '1\n2\n3', False, True, '1\n2', '1\n2\n3')
+    # Keep output difference when capping.
+    check('1\n2', '3\n4\n5', True, True, '1\n2', '3\n4')
+    check('1\n2\n3', '4\n5', True, True, '1\n2', '4\n5')
 
 
 def cut_verbose_output(stdout):

tools/clusterfuzz/v8_suppressions.py

@@ -142,6 +142,35 @@ IGNORE_LINES = [re.compile(exp) for exp in IGNORE_LINES]
 
 ORIGINAL_SOURCE_PREFIX = 'v8-foozzie source: '
 
+
+def get_lines_capped(output1, output2):
+  """Returns a pair of stdout line lists.
+
+  The lists are safely capped if at least one run has crashed. We assume
+  that output can never break off within the last line. If this assumption
+  turns out wrong, we need to add capping of the last line, too.
+  """
+  output1_lines = output1.stdout.splitlines()
+  output2_lines = output2.stdout.splitlines()
+
+  # No length difference or no crash -> no capping.
+  if (len(output1_lines) == len(output2_lines) or
+      (not output1.HasCrashed() and not output2.HasCrashed())):
+    return output1_lines, output2_lines
+
+  # Both runs have crashed, cap by the shorter output.
+  if output1.HasCrashed() and output2.HasCrashed():
+    cap = min(len(output1_lines), len(output2_lines))
+  # Only the first run has crashed, cap by its output length.
+  elif output1.HasCrashed():
+    cap = len(output1_lines)
+  # Similar if only the second run has crashed.
+  else:
+    cap = len(output2_lines)
+
+  return output1_lines[0:cap], output2_lines[0:cap]
+
+
 def line_pairs(lines):
   return itertools.izip_longest(
       lines, itertools.islice(lines, 1, None), fillvalue=None)
@@ -266,9 +295,13 @@ class V8Suppression(Suppression):
     self.config2 = config2
 
   def diff(self, output1, output2):
+    # Diff capped lines in the presence of crashes.
+    return self.diff_lines(*get_lines_capped(output1, output2))
+
+  def diff_lines(self, output1_lines, output2_lines):
     return diff_output(
-        output1.splitlines(),
-        output2.splitlines(),
+        output1_lines,
+        output2_lines,
         ALLOWED_LINE_DIFFS,
         IGNORE_LINES,
         IGNORE_LINES,