diff --git a/src/funfuzz/autobisectjs/autobisectjs.py b/src/funfuzz/autobisectjs/autobisectjs.py index 8936f6af1..4ae44b001 100644 --- a/src/funfuzz/autobisectjs/autobisectjs.py +++ b/src/funfuzz/autobisectjs/autobisectjs.py @@ -23,8 +23,10 @@ from . import known_broken_earliest_working as kbew from ..js import build_options +from ..js import compare_jit from ..js import compile_shell from ..js import inspect_shell +from ..js import js_interesting from ..util import hg_helpers from ..util import s3cache from ..util import subprocesses as sps @@ -305,7 +307,8 @@ def inner(shellFilename, _hgHash): # pylint: disable=invalid-name,missing-docst def externalTestAndLabel(options, interestingness): # pylint: disable=invalid-name,missing-param-doc,missing-return-doc # pylint: disable=missing-return-type-doc,missing-type-doc """Make use of interestingness scripts to decide whether the changeset is good or bad.""" - conditionScript = rel_or_abs_import(interestingness[0]) # pylint: disable=invalid-name + interestingness_name = interestingness[0] + conditionScript = rel_or_abs_import(interestingness_name) # pylint: disable=invalid-name conditionArgPrefix = interestingness[1:] # pylint: disable=invalid-name def inner(shellFilename, hgHash): # pylint: disable=invalid-name,missing-docstring,missing-return-doc @@ -313,10 +316,16 @@ def inner(shellFilename, hgHash): # pylint: disable=invalid-name,missing-docstr conditionArgs = conditionArgPrefix + [shellFilename] + options.paramList # pylint: disable=invalid-name tempDir = tempfile.mkdtemp(prefix="abExtTestAndLabel-" + hgHash) # pylint: disable=invalid-name tempPrefix = os.path.join(tempDir, "t") # pylint: disable=invalid-name - if hasattr(conditionScript, "init"): - # Since we're changing the js shell name, call init() again! - conditionScript.init(conditionArgs) - if conditionScript.interesting(conditionArgs, tempPrefix): + + assert "js_interesting" in interestingness_name or "compare_jit" in interestingness_name + if "js_interesting" in interestingness_name: + opts = js_interesting.parseOptions(conditionArgs) + elif "compare_jit" in interestingness_name: + opts = compare_jit.parseOptions(conditionArgs) + else: + raise ValueError("Invalid condition script specified: %s" % interestingness_name) + + if conditionScript.interesting(opts, tempPrefix): innerResult = ("bad", "interesting") # pylint: disable=invalid-name else: innerResult = ("good", "not interesting") # pylint: disable=invalid-name diff --git a/src/funfuzz/autobisectjs/examples.md b/src/funfuzz/autobisectjs/examples.md index 2922893a4..9d27adbf4 100644 --- a/src/funfuzz/autobisectjs/examples.md +++ b/src/funfuzz/autobisectjs/examples.md @@ -44,4 +44,4 @@ You could specify the assertion message this way too: * To bisect **bugs found by compare_jit**: -```python -m funfuzz.autobisectjs -s 6ec4eb9786d8 -p 1183423.js -b "--enable-debug --enable-more-deterministic -R ~/trees/mozilla-central" -i funfuzz.js.compare_jit --minlevel=6 mozilla-central``` +```python -m funfuzz.autobisectjs -s 6ec4eb9786d8 -p 1183423.js -b "--enable-debug --enable-more-deterministic -R ~/trees/mozilla-central" -i funfuzz.js.compare_jit --minlevel=6``` diff --git a/src/funfuzz/bot.py b/src/funfuzz/bot.py index ba3957585..8dbfda9c6 100644 --- a/src/funfuzz/bot.py +++ b/src/funfuzz/bot.py @@ -236,7 +236,6 @@ def mtrArgsCreation(options, cshell): # pylint: disable=invalid-name,missing-pa # Ordering of elements in manyTimedRunArgs is important. manyTimedRunArgs.append(str(options.timeout)) - manyTimedRunArgs.append(cshell.getRepoName()) # known bugs' directory manyTimedRunArgs.append(cshell.getShellCacheFullPath()) return manyTimedRunArgs diff --git a/src/funfuzz/js/compare_jit.py b/src/funfuzz/js/compare_jit.py index 54ba0ffd5..2edc1ca7d 100644 --- a/src/funfuzz/js/compare_jit.py +++ b/src/funfuzz/js/compare_jit.py @@ -25,7 +25,6 @@ from ..util import lithium_helpers from ..util import subprocesses as sps -gOptions = "" # pylint: disable=invalid-name lengthLimit = 1000000 # pylint: disable=invalid-name @@ -56,7 +55,7 @@ def compare_jit(jsEngine, flags, infilename, logPrefix, repo, build_options_str, if lev != js_interesting.JS_FINE: itest = [__name__, "--flags=" + " ".join(flags), - "--minlevel=" + str(lev), "--timeout=" + str(options.timeout), options.knownPath] + "--minlevel=" + str(lev), "--timeout=" + str(options.timeout)] (lithResult, _lithDetails, autoBisectLog) = lithium_helpers.pinpoint( # pylint: disable=invalid-name itest, logPrefix, jsEngine, [], infilename, repo, build_options_str, targetTime, lev) if lithResult == lithium_helpers.LITH_FINISHED: @@ -85,7 +84,7 @@ def compareLevel(jsEngine, flags, infilename, logPrefix, options, showDetailedDi # pylint: disable=too-many-branches,too-many-arguments,too-many-locals # options dict must be one we can pass to js_interesting.ShellResult - # we also use it directly for knownPath, timeout, and collector + # we also use it directly for timeout, and collector # Return: (lev, crashInfo) or (js_interesting.JS_FINE, None) combos = shell_flags.basic_flag_sets(jsEngine) @@ -166,7 +165,6 @@ def optionDisabledAsmOnOneSide(): # pylint: disable=invalid-name,missing-docstr rerunCommand = " ".join(quote(x) for x in ["python -m funfuzz.js.compare_jit", "--flags=" + " ".join(flags), "--timeout=" + str(options.timeout), - options.knownPath, jsEngine, os.path.basename(infilename)]) (summary, issues) = summarizeMismatch(mismatchErr, mismatchOut, prefix0, prefix) @@ -247,11 +245,8 @@ def parseOptions(args): # pylint: disable=invalid-name default="", help="space-separated list of one set of flags") options, args = parser.parse_args(args) - if len(args) != 3: - raise Exception("Wrong number of positional arguments. Need 3 (knownPath, jsengine, infilename).") - options.knownPath = args[0] - options.jsengine = args[1] - options.infilename = args[2] + options.jsengine = args[0] + options.infilename = args[1] options.flags = options.flagsSpaceSep.split(" ") if options.flagsSpaceSep else [] if not os.path.exists(options.jsengine): raise Exception("js shell does not exist: " + options.jsengine) @@ -264,17 +259,10 @@ def parseOptions(args): # pylint: disable=invalid-name return options -# For use by Lithium and autobisectjs. (autobisectjs calls init multiple times because it changes the js engine name) -def init(args): - global gOptions # pylint: disable=invalid-name,global-statement - gOptions = parseOptions(args) - - -# FIXME: _args is unused here, we should check if it can be removed? # pylint: disable=fixme -def interesting(_args, tempPrefix): # pylint: disable=invalid-name +def interesting(opts, tempPrefix): # pylint: disable=invalid-name actualLevel = compareLevel( # pylint: disable=invalid-name - gOptions.jsengine, gOptions.flags, gOptions.infilename, tempPrefix, gOptions, False, False)[0] - return actualLevel >= gOptions.minimumInterestingLevel + opts.jsengine, opts.flags, opts.infilename, tempPrefix, opts, False, False)[0] + return actualLevel >= opts.minimumInterestingLevel def main(): diff --git a/src/funfuzz/js/js_interesting.py b/src/funfuzz/js/js_interesting.py index f6e81f3fc..416457370 100644 --- a/src/funfuzz/js/js_interesting.py +++ b/src/funfuzz/js/js_interesting.py @@ -24,7 +24,7 @@ from . import inspect_shell from ..util import create_collector -from ..util import detect_malloc_errors +from ..util import file_manipulation from ..util import subprocesses as sps if sys.version_info.major == 2: @@ -59,13 +59,12 @@ ) = range(JS_LEVELS) -gOptions = "" # pylint: disable=invalid-name VALGRIND_ERROR_EXIT_CODE = 77 class ShellResult(object): # pylint: disable=missing-docstring,too-many-instance-attributes,too-few-public-methods - # options dict should include: timeout, knownPath, collector, valgrind, shellIsDeterministic + # options dict should include: timeout, collector, valgrind, shellIsDeterministic def __init__(self, options, runthis, logPrefix, in_compare_jit): # pylint: disable=too-complex,too-many-branches # pylint: disable=too-many-locals,too-many-statements pathToBinary = runthis[0] # pylint: disable=invalid-name @@ -108,7 +107,7 @@ def __init__(self, options, runthis, logPrefix, in_compare_jit): # pylint: disa if sps.grabCrashLog(runthis[0], runinfo.pid, logPrefix, True): with open(logPrefix + "-crash.txt") as f: auxCrashData = [line.strip() for line in f.readlines()] - elif detect_malloc_errors.amiss(logPrefix): + elif file_manipulation.amiss(logPrefix): issues.append("malloc error") lev = max(lev, JS_NEW_ASSERT_OR_CRASH) elif runinfo.return_code == 0 and not in_compare_jit: @@ -296,10 +295,7 @@ def parseOptions(args): # pylint: disable=invalid-name,missing-docstring,missin default=120, help="timeout in seconds") options, args = parser.parse_args(args) - if len(args) < 2: - raise Exception("Not enough positional arguments") - options.knownPath = args[0] - options.jsengineWithArgs = args[1:] + options.jsengineWithArgs = args options.collector = create_collector.createCollector("jsfunfuzz") if not os.path.exists(options.jsengineWithArgs[0]): raise Exception("js shell does not exist: " + options.jsengineWithArgs[0]) @@ -312,21 +308,12 @@ def parseOptions(args): # pylint: disable=invalid-name,missing-docstring,missin # loop uses parseOptions and ShellResult [with in_compare_jit = False] # compare_jit uses ShellResult [with in_compare_jit = True] -# For use by Lithium and autobisectjs. (autobisectjs calls init multiple times because it changes the js engine name) -def init(args): # pylint: disable=missing-docstring - global gOptions # pylint: disable=global-statement,invalid-name - gOptions = parseOptions(args) - - -# FIXME: _args is unused here, we should check if it can be removed? # pylint: disable=fixme -def interesting(_args, tempPrefix): # pylint: disable=invalid-name,missing-docstring,missing-return-doc +def interesting(opts, tempPrefix): # pylint: disable=invalid-name,missing-docstring,missing-return-doc # pylint: disable=missing-return-type-doc - options = gOptions - # options, runthis, logPrefix, in_compare_jit - res = ShellResult(options, options.jsengineWithArgs, tempPrefix, False) + res = ShellResult(opts, opts.jsengineWithArgs, tempPrefix, False) truncateFile(tempPrefix + "-out.txt", 1000000) truncateFile(tempPrefix + "-err.txt", 1000000) - return res.lev >= gOptions.minimumInterestingLevel + return res.lev >= opts.minimumInterestingLevel # For direct, manual use diff --git a/src/funfuzz/js/loop.py b/src/funfuzz/js/loop.py index 6508b23d3..d5b093b98 100644 --- a/src/funfuzz/js/loop.py +++ b/src/funfuzz/js/loop.py @@ -60,11 +60,8 @@ def parseOpts(args): # pylint: disable=invalid-name,missing-docstring,missing-r # higher = more complex mixing, especially with regression tests. # lower = less time wasted in timeouts and in compare_jit testcases that are thrown away due to OOMs. options.timeout = int(args[0]) - - # FIXME: We can probably remove args[1] # pylint: disable=fixme - options.knownPath = "mozilla-central" - options.jsEngine = args[2] - options.engineFlags = args[3:] + options.jsEngine = args[1] + options.engineFlags = args[2:] return options @@ -146,7 +143,6 @@ def many_timed_runs(targetTime, wtmpDir, args, collector): # pylint: disable=in js_interesting_args.append("--timeout=" + str(options.timeout)) if options.valgrind: js_interesting_args.append("--valgrind") - js_interesting_args.append(options.knownPath) js_interesting_args.append(options.jsEngine) if options.randomFlags: engineFlags = shell_flags.random_flag_set(options.jsEngine) # pylint: disable=invalid-name @@ -185,7 +181,6 @@ def many_timed_runs(targetTime, wtmpDir, args, collector): # pylint: disable=in itest.append("--valgrind") itest.append("--minlevel=" + str(res.lev)) itest.append("--timeout=" + str(options.timeout)) - itest.append(options.knownPath) (lithResult, _lithDetails, autoBisectLog) = lithium_helpers.pinpoint( # pylint: disable=invalid-name itest, logPrefix, options.jsEngine, engineFlags, filenameToReduce, options.repo, options.build_options_str, targetTime, res.lev) diff --git a/src/funfuzz/util/__init__.py b/src/funfuzz/util/__init__.py index 30015c124..acbd2b34d 100644 --- a/src/funfuzz/util/__init__.py +++ b/src/funfuzz/util/__init__.py @@ -10,7 +10,6 @@ from . import crashesat from . import create_collector -from . import detect_malloc_errors from . import file_manipulation from . import fork_join from . import hg_helpers diff --git a/src/funfuzz/util/detect_malloc_errors.py b/src/funfuzz/util/detect_malloc_errors.py deleted file mode 100644 index 7a6c5568e..000000000 --- a/src/funfuzz/util/detect_malloc_errors.py +++ /dev/null @@ -1,50 +0,0 @@ -# coding=utf-8 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at https://mozilla.org/MPL/2.0/. - -"""Look for "szone_error" (Tiger), "malloc_error_break" (Leopard), "MallocHelp" (?) -which are signs of malloc being unhappy (double free, out-of-memory, etc). -""" - -from __future__ import absolute_import, print_function # isort:skip - -PLINE = "" -PPLINE = "" - - -def amiss(log_prefix): # pylint: disable=missing-docstring,missing-return-doc,missing-return-type-doc - found_something = False - global PLINE, PPLINE # pylint: disable=global-statement - - PLINE = "" - PPLINE = "" - - with open(log_prefix + "-err.txt") as f: - for line in f: - if scanLine(line): - found_something = True - break # Don't flood the log with repeated malloc failures - - return found_something - - -def scanLine(line): # pylint: disable=inconsistent-return-statements,invalid-name,missing-docstring,missing-return-doc - # pylint: disable=missing-return-type-doc - global PPLINE, PLINE # pylint: disable=global-statement - - line = line.strip("\x07").rstrip("\n") - - if (line.find("szone_error") != -1 or - line.find("malloc_error_break") != -1 or - line.find("MallocHelp") != -1): - if PLINE.find("can't allocate region") == -1: - print() - print(PPLINE) - print(PLINE) - print(line) - return True - - PPLINE = PLINE - PLINE = line diff --git a/src/funfuzz/util/file_manipulation.py b/src/funfuzz/util/file_manipulation.py index 56d29b006..a7c4654a6 100644 --- a/src/funfuzz/util/file_manipulation.py +++ b/src/funfuzz/util/file_manipulation.py @@ -10,6 +10,25 @@ from __future__ import absolute_import, print_function # isort:skip +def amiss(log_prefix): # pylint: disable=missing-param-doc,missing-return-doc,missing-return-type-doc,missing-type-doc + """Look for "szone_error" (Tiger), "malloc_error_break" (Leopard), "MallocHelp" (?) + which are signs of malloc being unhappy (double free, out-of-memory, etc). + """ + found_something = False + with open(log_prefix + "-err.txt") as f: + for line in f: + line = line.strip("\x07").rstrip("\n") + if (line.find("szone_error") != -1 or + line.find("malloc_error_break") != -1 or + line.find("MallocHelp") != -1): + print() + print(line) + found_something = True + break # Don't flood the log with repeated malloc failures + + return found_something + + def fuzzSplice(filename): # pylint: disable=invalid-name,missing-param-doc,missing-return-doc,missing-return-type-doc # pylint: disable=missing-type-doc """Return the lines of a file, minus the ones between the two lines containing SPLICE."""