Remove globals and other cleanup

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,18 +307,25 @@ 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
         # pylint: disable=missing-return-type-doc
         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

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```

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
 

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():

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

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)

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

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."""