From 5d50024ed44d1a44d1bd80fb01a4e031ea611dd7 Mon Sep 17 00:00:00 2001 From: Michael Achenbach Date: Fri, 2 Sep 2022 14:31:27 +0200 Subject: [PATCH] [foozzie] Filter some contradictory flags Add logic to drop cyclic contradictory flags from correctness-fuzzing command lines. Add the currently known biggest offenders. Without this, the correctness fuzzing harness runs into a CHECK failure during smoke testing, when attempting to pass cyclic flags to d8. It fails fast, but uselessly burns fuzzing time. This change drops one of the known cyclic flags instead to make the test run still useful. The precedence is right to left like in the V8 test framework. Additionally on Clusterfuzz, all crashes during smoke testing are deduped as one crash report. We don't know if there are other problems before this one is fixed/hidden. No-Try: true Bug: chromium:1330303 Change-Id: I06cbb4655cd3cf467f5cce6f84dba653834ca72e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3865562 Reviewed-by: Clemens Backes Commit-Queue: Michael Achenbach Reviewed-by: Alexander Schulze Cr-Commit-Position: refs/heads/main@{#82939} --- tools/clusterfuzz/foozzie/PRESUBMIT.py | 3 ++- tools/clusterfuzz/foozzie/v8_foozzie.py | 28 ++++++++++++++++++-- tools/clusterfuzz/foozzie/v8_foozzie_test.py | 19 +++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/tools/clusterfuzz/foozzie/PRESUBMIT.py b/tools/clusterfuzz/foozzie/PRESUBMIT.py index cc94c5146d..8108608884 100644 --- a/tools/clusterfuzz/foozzie/PRESUBMIT.py +++ b/tools/clusterfuzz/foozzie/PRESUBMIT.py @@ -12,7 +12,8 @@ USE_PYTHON3 = True def _RunTests(input_api, output_api): return input_api.RunTests( input_api.canned_checks.GetUnitTestsInDirectory( - input_api, output_api, '.', files_to_check=[r'.+_test\.py$'])) + input_api, output_api, '.', files_to_check=[r'.+_test\.py$'], + run_on_python2=False)) def _CommonChecks(input_api, output_api): diff --git a/tools/clusterfuzz/foozzie/v8_foozzie.py b/tools/clusterfuzz/foozzie/v8_foozzie.py index 54ce9ea8ea..306306432b 100755 --- a/tools/clusterfuzz/foozzie/v8_foozzie.py +++ b/tools/clusterfuzz/foozzie/v8_foozzie.py @@ -179,9 +179,33 @@ DISALLOWED_FLAGS = [ '--multi-mapped-mock-allocator', ] +# List pairs of flags that lead to contradictory cycles, i.e.: +# A -> no-C and B -> C makes (A, B) contradictory. +# No need to list other contradictions, they are omitted by the +# --fuzzing flag). +CONTRADICTORY_FLAGS = [ + ('--always-turbofan', '--jitless'), +] + def filter_flags(flags): - return [flag for flag in flags if flag not in DISALLOWED_FLAGS] + """Drop disallowed and contradictory flags. + + The precedence for contradictions is right to left, similar to the V8 test + framework. + """ + result = [] + flags_to_drop = set(DISALLOWED_FLAGS) + for flag in reversed(flags): + if flag in flags_to_drop: + continue + result.append(flag) + for contradicting_pair in CONTRADICTORY_FLAGS: + if contradicting_pair[0] == flag: + flags_to_drop.add(contradicting_pair[1]) + if contradicting_pair[1] == flag: + flags_to_drop.add(contradicting_pair[0]) + return list(reversed(result)) def infer_arch(d8): @@ -233,7 +257,7 @@ class ExecutionArgumentsConfig(object): d8 = os.path.join(BASE_PATH, d8) assert os.path.exists(d8) - flags = CONFIGS[config] + filter_flags(get('config_extra_flags')) + flags = filter_flags(CONFIGS[config] + get('config_extra_flags')) RunOptions = namedtuple('RunOptions', ['arch', 'config', 'd8', 'flags']) return RunOptions(infer_arch(d8), config, d8, flags) diff --git a/tools/clusterfuzz/foozzie/v8_foozzie_test.py b/tools/clusterfuzz/foozzie/v8_foozzie_test.py index fe149620f9..796ee5d0c7 100755 --- a/tools/clusterfuzz/foozzie/v8_foozzie_test.py +++ b/tools/clusterfuzz/foozzie/v8_foozzie_test.py @@ -8,6 +8,7 @@ import random import subprocess import sys import unittest +import unittest.mock import v8_commands import v8_foozzie @@ -233,6 +234,24 @@ other weird stuff check('12', '345', True, True, '12', '34') check('123', '45', True, True, '12', '45') + @unittest.mock.patch('v8_foozzie.DISALLOWED_FLAGS', ['A']) + @unittest.mock.patch('v8_foozzie.CONTRADICTORY_FLAGS', + [('B', 'C'), ('B', 'D')]) + def testFilterFlags(self): + def check(input_flags, expected): + self.assertEqual(expected, v8_foozzie.filter_flags(input_flags)) + + check([], []) + check(['A'], []) + check(['D', 'A'], ['D']) + check(['A', 'D'], ['D']) + check(['C', 'D'], ['C', 'D']) + check(['E', 'C', 'D', 'F'], ['E', 'C', 'D', 'F']) + check(['B', 'D'], ['D']) + check(['D', 'B'], ['B']) + check(['C', 'B', 'D'], ['C', 'D']) + check(['E', 'C', 'A', 'F', 'B', 'G', 'D'], ['E', 'C', 'F', 'G', 'D']) + def cut_verbose_output(stdout, n_comp): # This removes the first lines containing d8 commands of `n_comp` comparison