From 74c78019e7bc7f64c8c4b8e3c9bf03202facf38f Mon Sep 17 00:00:00 2001 From: Wes Kocher Date: Tue, 31 May 2016 11:15:41 -0700 Subject: [PATCH] Backed out 22 changesets (bug 1259850) for GC crashes in various tests CLOSED TREE Backed out changeset ef5cdcca45d9 (bug 1259850) Backed out changeset c95bdd426ced (bug 1259850) Backed out changeset a73f74f718e7 (bug 1259850) Backed out changeset 95107c3ad9cf (bug 1259850) Backed out changeset 788ac18818c9 (bug 1259850) Backed out changeset 19c13aa9b5ad (bug 1259850) Backed out changeset 0b9dedcf7163 (bug 1259850) Backed out changeset b641d01138ab (bug 1259850) Backed out changeset aa434447a11b (bug 1259850) Backed out changeset 4c7373c6c29e (bug 1259850) Backed out changeset 457cb29cad55 (bug 1259850) Backed out changeset 5762a8fba027 (bug 1259850) Backed out changeset 129559d4ac62 (bug 1259850) Backed out changeset d00b9c8a7984 (bug 1259850) Backed out changeset 266befcb8acd (bug 1259850) Backed out changeset c6615c7b0083 (bug 1259850) Backed out changeset 196ac1f813f9 (bug 1259850) Backed out changeset b6108a65dc38 (bug 1259850) Backed out changeset 0d58f8529b86 (bug 1259850) Backed out changeset a8d2730ada95 (bug 1259850) Backed out changeset e8544b072ee6 (bug 1259850) Backed out changeset 15de0d1d0b05 (bug 1259850) --- .hgignore | 1 - .../linux64/hazard.manifest | 68 ++-- .../linux64/hazard.manifest | 56 ++-- js/public/GCAPI.h | 2 +- js/public/GCAnnotations.h | 6 +- js/src/devtools/rootAnalysis/CFG.js | 2 +- js/src/devtools/rootAnalysis/analyze.py | 48 ++- js/src/devtools/rootAnalysis/analyzeRoots.js | 177 ++++------- js/src/devtools/rootAnalysis/annotations.js | 53 +--- .../rootAnalysis/build/sixgill.manifest | 16 +- .../devtools/rootAnalysis/computeCallgraph.js | 294 ++++++++---------- .../rootAnalysis/computeGCFunctions.js | 22 +- .../devtools/rootAnalysis/computeGCTypes.js | 43 --- js/src/devtools/rootAnalysis/explain.py | 19 +- js/src/devtools/rootAnalysis/loadCallgraph.js | 9 +- js/src/devtools/rootAnalysis/run-test.py | 201 ++++++++---- .../rootAnalysis/t/exceptions/source.cpp | 42 --- .../rootAnalysis/t/exceptions/test.py | 19 -- .../rootAnalysis/t/hazards/source.cpp | 89 ------ .../devtools/rootAnalysis/t/hazards/test.py | 36 --- js/src/devtools/rootAnalysis/t/sixgill.py | 63 ---- .../rootAnalysis/t/suppression/source.cpp | 64 ---- .../rootAnalysis/t/suppression/test.py | 23 -- js/src/devtools/rootAnalysis/t/testlib.py | 120 ------- .../{t/sixgill-tree => test}/source.cpp | 0 .../{t/sixgill-tree => test}/test.py | 16 +- js/src/devtools/rootAnalysis/utility.js | 27 -- js/src/gc/Iteration.cpp | 22 +- js/src/gc/Statistics.cpp | 47 +-- js/src/gc/Statistics.h | 31 +- js/src/gc/Zone.cpp | 13 +- js/src/gc/Zone.h | 10 - js/src/jit/BaselineJIT.cpp | 10 +- js/src/jit/Ion.cpp | 8 +- .../jsapi-tests/testGCStoreBufferRemoval.cpp | 2 +- js/src/jscompartment.cpp | 7 +- js/src/jsgc.cpp | 62 ++-- js/src/jsgc.h | 91 +----- js/src/jsgcinlines.h | 144 +++------ js/src/jsopcode.cpp | 11 +- js/src/vm/Debugger.cpp | 4 +- js/src/vm/HelperThreads.cpp | 5 +- js/src/vm/NativeObject.cpp | 2 +- js/src/vm/TypeInference.cpp | 18 +- 44 files changed, 591 insertions(+), 1412 deletions(-) delete mode 100644 js/src/devtools/rootAnalysis/t/exceptions/source.cpp delete mode 100644 js/src/devtools/rootAnalysis/t/exceptions/test.py delete mode 100644 js/src/devtools/rootAnalysis/t/hazards/source.cpp delete mode 100644 js/src/devtools/rootAnalysis/t/hazards/test.py delete mode 100644 js/src/devtools/rootAnalysis/t/sixgill.py delete mode 100644 js/src/devtools/rootAnalysis/t/suppression/source.cpp delete mode 100644 js/src/devtools/rootAnalysis/t/suppression/test.py delete mode 100644 js/src/devtools/rootAnalysis/t/testlib.py rename js/src/devtools/rootAnalysis/{t/sixgill-tree => test}/source.cpp (100%) rename js/src/devtools/rootAnalysis/{t/sixgill-tree => test}/test.py (82%) diff --git a/.hgignore b/.hgignore index 7aa79886a2da..895885254eab 100644 --- a/.hgignore +++ b/.hgignore @@ -43,7 +43,6 @@ _OPT\.OBJ/ ^js/src/autom4te.cache$ # SpiderMonkey test result logs ^js/src/tests/results-.*\.(html|txt)$ -^js/src/devtools/rootAnalysis/t/out # Java HTML5 parser classes ^parser/html/java/(html|java)parser/ diff --git a/b2g/dev/config/tooltool-manifests/linux64/hazard.manifest b/b2g/dev/config/tooltool-manifests/linux64/hazard.manifest index f4ab1fcabc74..fcef5bfc5738 100644 --- a/b2g/dev/config/tooltool-manifests/linux64/hazard.manifest +++ b/b2g/dev/config/tooltool-manifests/linux64/hazard.manifest @@ -1,47 +1,47 @@ [ { -"size" : 102421980, -"digest" : "f25292aa93dc449e0472eee511c0ac15b5f1a4272ab76cf53ce5d20dc57f29e83da49ae1a9d9e994192647f75e13ae60f75ba2ac3cb9d26d5f5d6cabf88de921", -"version" : "gcc 4.9.3", -"unpack" : true, -"filename" : "gcc.tar.xz", -"algorithm" : "sha512" +"version": "gcc 4.9.3", +"size": 102421980, +"digest": "f25292aa93dc449e0472eee511c0ac15b5f1a4272ab76cf53ce5d20dc57f29e83da49ae1a9d9e994192647f75e13ae60f75ba2ac3cb9d26d5f5d6cabf88de921", +"algorithm": "sha512", +"filename": "gcc.tar.xz", +"unpack": true }, { -"unpack" : true, +"hg_id" : "cd93f15a30ce", "algorithm" : "sha512", +"digest" : "541eb3842ab6b91bd87223cad7a5e4387ef3e496e5b580c8047b8b586bc7eb69fecf3c9eb8c45a1e0deebb53554f0e8acedfe1b4ca64d93b6d008f3f2eb11389", "filename" : "sixgill.tar.xz", -"hg_id" : "8cb9c3fb039a+ tip", -"digest" : "36dc644e24c0aa824975ad8f5c15714445d5cb064d823000c3cb637e885199414d7df551e6b99233f0656dcf5760918192ef04113c486af37f3c489bb93ad029", -"size" : 2631908 -}, -{ -"algorithm" : "sha512", -"filename" : "gtk3.tar.xz", -"setup" : "setup.sh", -"unpack" : true, -"digest" : "3915f8ec396c56a8a92e6f9695b70f09ce9d1582359d1258e37e3fd43a143bc974410e4cfc27f500e095f34a8956206e0ebf799b7287f0f38def0d5e34ed71c9", -"size" : 12072532 -}, -{ -"size" : 89319524, -"digest" : "5383d843c9f28abf0a6d254e9d975d96972d2c86d627ca836fa8e272a5d53230603b387d7d1499c49df7f84b1bb946946e800a85c88d968bdbe81c755fcb02e1", -"algorithm" : "sha512", -"filename" : "rustc.tar.xz", +"size" : 2626640, "unpack" : true }, { -"algorithm" : "sha512", -"filename" : "sccache.tar.bz2", -"unpack" : true, -"digest" : "0b71a936edf5bd70cf274aaa5d7abc8f77fe8e7b5593a208f805cc9436fac646b9c4f0b43c2b10de63ff3da671497d35536077ecbc72dba7f8159a38b580f831", -"size" : 167175 +"size": 12072532, +"digest": "3915f8ec396c56a8a92e6f9695b70f09ce9d1582359d1258e37e3fd43a143bc974410e4cfc27f500e095f34a8956206e0ebf799b7287f0f38def0d5e34ed71c9", +"algorithm": "sha512", +"filename": "gtk3.tar.xz", +"setup": "setup.sh", +"unpack": true }, { -"filename" : "moz-tt.tar.bz2", -"algorithm" : "sha512", -"unpack" : true, -"digest" : "2dffe4e5419a0c0c9908dc52b01cc07379a42e2aa8481be7a26bb8750b586b95bbac3fe57e64f5d37b43e206516ea70ad938a2e45858fdcf1e28258e70ae8d8c", -"size" : 31078810 +"size": 89319524, +"digest": "5383d843c9f28abf0a6d254e9d975d96972d2c86d627ca836fa8e272a5d53230603b387d7d1499c49df7f84b1bb946946e800a85c88d968bdbe81c755fcb02e1", +"algorithm": "sha512", +"filename": "rustc.tar.xz", +"unpack": true +}, +{ +"size": 167175, +"digest": "0b71a936edf5bd70cf274aaa5d7abc8f77fe8e7b5593a208f805cc9436fac646b9c4f0b43c2b10de63ff3da671497d35536077ecbc72dba7f8159a38b580f831", +"algorithm": "sha512", +"filename": "sccache.tar.bz2", +"unpack": true +}, +{ +"size": 31078810, +"digest": "2dffe4e5419a0c0c9908dc52b01cc07379a42e2aa8481be7a26bb8750b586b95bbac3fe57e64f5d37b43e206516ea70ad938a2e45858fdcf1e28258e70ae8d8c", +"algorithm": "sha512", +"filename": "moz-tt.tar.bz2", +"unpack": true } ] diff --git a/browser/config/tooltool-manifests/linux64/hazard.manifest b/browser/config/tooltool-manifests/linux64/hazard.manifest index f05dcf166c5b..9d0818273012 100644 --- a/browser/config/tooltool-manifests/linux64/hazard.manifest +++ b/browser/config/tooltool-manifests/linux64/hazard.manifest @@ -1,40 +1,40 @@ [ { -"size" : 102421980, -"version" : "gcc 4.9.3", -"filename" : "gcc.tar.xz", +"version": "gcc 4.9.3", +"size": 102421980, +"digest": "f25292aa93dc449e0472eee511c0ac15b5f1a4272ab76cf53ce5d20dc57f29e83da49ae1a9d9e994192647f75e13ae60f75ba2ac3cb9d26d5f5d6cabf88de921", +"algorithm": "sha512", +"filename": "gcc.tar.xz", +"unpack": true +}, +{ +"hg_id" : "cd93f15a30ce", "algorithm" : "sha512", -"digest" : "f25292aa93dc449e0472eee511c0ac15b5f1a4272ab76cf53ce5d20dc57f29e83da49ae1a9d9e994192647f75e13ae60f75ba2ac3cb9d26d5f5d6cabf88de921", +"digest" : "541eb3842ab6b91bd87223cad7a5e4387ef3e496e5b580c8047b8b586bc7eb69fecf3c9eb8c45a1e0deebb53554f0e8acedfe1b4ca64d93b6d008f3f2eb11389", +"filename" : "sixgill.tar.xz", +"size" : 2626640, "unpack" : true }, { -"digest" : "36dc644e24c0aa824975ad8f5c15714445d5cb064d823000c3cb637e885199414d7df551e6b99233f0656dcf5760918192ef04113c486af37f3c489bb93ad029", -"unpack" : true, -"algorithm" : "sha512", -"filename" : "sixgill.tar.xz", -"size" : 2631908, -"hg_id" : "8cb9c3fb039a+ tip" +"size": 12072532, +"digest": "3915f8ec396c56a8a92e6f9695b70f09ce9d1582359d1258e37e3fd43a143bc974410e4cfc27f500e095f34a8956206e0ebf799b7287f0f38def0d5e34ed71c9", +"algorithm": "sha512", +"filename": "gtk3.tar.xz", +"setup": "setup.sh", +"unpack": true }, { -"digest" : "3915f8ec396c56a8a92e6f9695b70f09ce9d1582359d1258e37e3fd43a143bc974410e4cfc27f500e095f34a8956206e0ebf799b7287f0f38def0d5e34ed71c9", -"unpack" : true, -"setup" : "setup.sh", -"algorithm" : "sha512", -"filename" : "gtk3.tar.xz", -"size" : 12072532 +"size": 89319524, +"digest": "5383d843c9f28abf0a6d254e9d975d96972d2c86d627ca836fa8e272a5d53230603b387d7d1499c49df7f84b1bb946946e800a85c88d968bdbe81c755fcb02e1", +"algorithm": "sha512", +"filename": "rustc.tar.xz", +"unpack": true }, { -"unpack" : true, -"digest" : "5383d843c9f28abf0a6d254e9d975d96972d2c86d627ca836fa8e272a5d53230603b387d7d1499c49df7f84b1bb946946e800a85c88d968bdbe81c755fcb02e1", -"filename" : "rustc.tar.xz", -"algorithm" : "sha512", -"size" : 89319524 -}, -{ -"filename" : "sccache.tar.bz2", -"algorithm" : "sha512", -"digest" : "0b71a936edf5bd70cf274aaa5d7abc8f77fe8e7b5593a208f805cc9436fac646b9c4f0b43c2b10de63ff3da671497d35536077ecbc72dba7f8159a38b580f831", -"unpack" : true, -"size" : 167175 +"size": 167175, +"digest": "0b71a936edf5bd70cf274aaa5d7abc8f77fe8e7b5593a208f805cc9436fac646b9c4f0b43c2b10de63ff3da671497d35536077ecbc72dba7f8159a38b580f831", +"algorithm": "sha512", +"filename": "sccache.tar.bz2", +"unpack": true } ] diff --git a/js/public/GCAPI.h b/js/public/GCAPI.h index c95ab18b1ac0..150dce3ac0cc 100644 --- a/js/public/GCAPI.h +++ b/js/public/GCAPI.h @@ -551,7 +551,7 @@ class JS_PUBLIC_API(AutoSuppressGCAnalysis) : public AutoAssertNoAlloc public: AutoSuppressGCAnalysis() : AutoAssertNoAlloc() {} explicit AutoSuppressGCAnalysis(JSRuntime* rt) : AutoAssertNoAlloc(rt) {} -} JS_HAZ_GC_SUPPRESSED; +}; /** * Assert that code is only ever called from a GC callback, disable the static diff --git a/js/public/GCAnnotations.h b/js/public/GCAnnotations.h index 366d787bf4dd..e73caf8d4da6 100644 --- a/js/public/GCAnnotations.h +++ b/js/public/GCAnnotations.h @@ -23,7 +23,7 @@ # define JS_HAZ_ROOTED __attribute__((tag("Rooted Pointer"))) // Mark a type as something that should not be held live across a GC, but which -// is not itself a GC pointer. +// is itself not a GC pointer. # define JS_HAZ_GC_INVALIDATED __attribute__((tag("Invalidated by GC"))) // Mark a type that would otherwise be considered a GC Pointer (eg because it @@ -39,9 +39,6 @@ // invalidating GC pointers. # define JS_HAZ_GC_CALL __attribute__((tag("GC Call"))) -// Mark an RAII class as suppressing GC within its scope. -# define JS_HAZ_GC_SUPPRESSED __attribute__((tag("Suppress GC"))) - #else # define JS_HAZ_GC_THING @@ -50,7 +47,6 @@ # define JS_HAZ_GC_INVALIDATED # define JS_HAZ_NON_GC_POINTER # define JS_HAZ_GC_CALL -# define JS_HAZ_GC_SUPPRESSED #endif diff --git a/js/src/devtools/rootAnalysis/CFG.js b/js/src/devtools/rootAnalysis/CFG.js index 6e9facaa1201..0b35e8914505 100644 --- a/js/src/devtools/rootAnalysis/CFG.js +++ b/js/src/devtools/rootAnalysis/CFG.js @@ -70,7 +70,7 @@ function allRAIIGuardedCallPoints(bodies, body, isConstructor) continue; var variable = callee.Variable; assert(variable.Kind == "Func"); - if (!isConstructor(edge.Type, variable.Name)) + if (!isConstructor(variable.Name)) continue; if (!("PEdgeCallInstance" in edge)) continue; diff --git a/js/src/devtools/rootAnalysis/analyze.py b/js/src/devtools/rootAnalysis/analyze.py index 69482dab7b71..3f2c34a44003 100755 --- a/js/src/devtools/rootAnalysis/analyze.py +++ b/js/src/devtools/rootAnalysis/analyze.py @@ -72,14 +72,12 @@ def generate_hazards(config, outfilename): '%(gcEdges)s', '%(suppressedFunctions_list)s', '%(gcTypes)s', - '%(typeInfo)s', str(i+1), '%(jobs)s', 'tmp.%s' % (i+1,)), config) outfile = 'rootingHazards.%s' % (i+1,) output = open(outfile, 'w') - if config['verbose']: - print_command(command, outfile=outfile, env=env(config)) + print_command(command, outfile=outfile, env=env(config)) jobs.append((command, Popen(command, stdout=output, env=env(config)))) final_status = 0 @@ -93,8 +91,7 @@ def generate_hazards(config, outfilename): with open(outfilename, 'w') as output: command = ['cat'] + [ 'rootingHazards.%s' % (i+1,) for i in range(int(config['jobs'])) ] - if config['verbose']: - print_command(command, outfile=outfilename) + print_command(command, outfile=outfilename) subprocess.call(command, stdout=output) JOBS = { 'dbs': @@ -114,7 +111,7 @@ JOBS = { 'dbs': ()), 'callgraph': - (('%(js)s', '%(analysis_scriptdir)s/computeCallgraph.js', '%(typeInfo)s'), + (('%(js)s', '%(analysis_scriptdir)s/computeCallgraph.js'), 'callgraph.txt'), 'gcFunctions': @@ -123,9 +120,8 @@ JOBS = { 'dbs': ('gcFunctions.txt', 'gcFunctions.lst', 'gcEdges.txt', 'suppressedFunctions.lst')), 'gcTypes': - (('%(js)s', '%(analysis_scriptdir)s/computeGCTypes.js', - '[gcTypes]', '[typeInfo]'), - ('gcTypes.txt', 'typeInfo.txt')), + (('%(js)s', '%(analysis_scriptdir)s/computeGCTypes.js',), + 'gcTypes.txt'), 'allFunctions': (('%(sixgill_bin)s/xdbkeys', 'src_body.xdb',), @@ -159,8 +155,7 @@ def run_job(name, config): if isinstance(outfiles, basestring): stdout_filename = '%s.tmp' % name temp_map[stdout_filename] = outfiles - if config['verbose']: - print_command(cmdspec, outfile=outfiles, env=env(config)) + print_command(cmdspec, outfile=outfiles, env=env(config)) else: stdout_filename = None pc = list(cmdspec) @@ -168,8 +163,7 @@ def run_job(name, config): for (i, name) in out_indexes(cmdspec): pc[i] = outfiles[outfile] outfile += 1 - if config['verbose']: - print_command(pc, env=env(config)) + print_command(pc, env=env(config)) command = list(cmdspec) outfile = 0 @@ -196,6 +190,15 @@ config = { 'ANALYSIS_SCRIPTDIR': os.path.dirname(__file__) } defaults = [ '%s/defaults.py' % config['ANALYSIS_SCRIPTDIR'], '%s/defaults.py' % os.getcwd() ] +for default in defaults: + try: + execfile(default, config) + print("Loaded %s" % default) + except: + pass + +data = config.copy() + parser = argparse.ArgumentParser(description='Statically analyze build tree for rooting hazards.') parser.add_argument('step', metavar='STEP', type=str, nargs='?', help='run starting from this step') @@ -217,21 +220,8 @@ parser.add_argument('--tag', '-t', type=str, nargs='?', help='name of job, also sets build command to "build."') parser.add_argument('--expect-file', type=str, nargs='?', help='deprecated option, temporarily still present for backwards compatibility') -parser.add_argument('--verbose', '-v', action='store_true', - help='Display cut & paste commands to run individual steps') args = parser.parse_args() - -for default in defaults: - try: - execfile(default, config) - if args.verbose: - print("Loaded %s" % default) - except: - pass - -data = config.copy() - for k,v in vars(args).items(): if v is not None: data[k] = v @@ -242,7 +232,7 @@ if args.tag and not args.buildcommand: if args.jobs is not None: data['jobs'] = args.jobs if not data.get('jobs'): - data['jobs'] = subprocess.check_output(['nproc', '--ignore=1']).strip() + data['jobs'] = subprocess.check_output(['nproc', '--ignore=1']) if args.buildcommand: data['buildcommand'] = args.buildcommand @@ -261,8 +251,8 @@ if not data.get('source') and data.get('sixgill_bin'): data['source'] = path.replace("/js/src/jsapi.cpp", "") steps = [ 'dbs', - 'gcTypes', 'callgraph', + 'gcTypes', 'gcFunctions', 'allFunctions', 'hazards', @@ -286,7 +276,7 @@ for step in steps: for (i, name) in out_indexes(command): data[name] = outfiles[outfile] outfile += 1 - assert len(outfiles) == outfile, 'step \'%s\': mismatched number of output files (%d) and params (%d)' % (step, outfile, len(outfiles)) + assert len(outfiles) == outfile, 'step \'%s\': mismatched number of output files and params' % step if args.step: steps = steps[steps.index(args.step):] diff --git a/js/src/devtools/rootAnalysis/analyzeRoots.js b/js/src/devtools/rootAnalysis/analyzeRoots.js index a0fa94e061c2..09b4de127a62 100644 --- a/js/src/devtools/rootAnalysis/analyzeRoots.js +++ b/js/src/devtools/rootAnalysis/analyzeRoots.js @@ -12,7 +12,7 @@ var functionName; var functionBodies; if (typeof scriptArgs[0] != 'string' || typeof scriptArgs[1] != 'string') - throw "Usage: analyzeRoots.js [-f function_name] [start end [tmpfile]]"; + throw "Usage: analyzeRoots.js [-f function_name] [start end [tmpfile]]"; var theFunctionNameToFind; if (scriptArgs[0] == '--function') { @@ -20,16 +20,13 @@ if (scriptArgs[0] == '--function') { scriptArgs = scriptArgs.slice(2); } -var gcFunctionsFile = scriptArgs[0] || "gcFunctions.lst"; -var gcEdgesFile = scriptArgs[1] || "gcEdges.txt"; -var suppressedFunctionsFile = scriptArgs[2] || "suppressedFunctions.lst"; -var gcTypesFile = scriptArgs[3] || "gcTypes.txt"; -var typeInfoFile = scriptArgs[4] || "typeInfo.txt"; -var batch = (scriptArgs[5]|0) || 1; -var numBatches = (scriptArgs[6]|0) || 1; -var tmpfile = scriptArgs[7] || "tmp.txt"; - -GCSuppressionTypes = loadTypeInfo(typeInfoFile)["Suppress GC"] || []; +var gcFunctionsFile = scriptArgs[0]; +var gcEdgesFile = scriptArgs[1]; +var suppressedFunctionsFile = scriptArgs[2]; +var gcTypesFile = scriptArgs[3]; +var batch = (scriptArgs[4]|0) || 1; +var numBatches = (scriptArgs[5]|0) || 1; +var tmpfile = scriptArgs[6] || "tmp.txt"; var gcFunctions = {}; var text = snarf("gcFunctions.lst").split("\n"); @@ -334,57 +331,31 @@ function edgeCanGC(edge) // // - 'gcInfo': a direct pointer to the GC call edge // -function findGCBeforeVariableUse(start_body, start_point, suppressed, variable) +function findGCBeforeVariableUse(suppressed, variable, worklist) { // Scan through all edges preceding an unrooted variable use, using an // explicit worklist, looking for a GC call. A worklist contains an // incoming edge together with a description of where it or one of its // successors GC'd (if any). - var bodies_visited = new Map(); - - let worklist = [{body: start_body, ppoint: start_point, preGCLive: false, gcInfo: null, why: null}]; while (worklist.length) { - // Grab an entry off of the worklist, representing a point within the - // CFG identified by . If this point has a descendant - // later in the CFG that can GC, gcInfo will be set to the information - // about that GC call. - var entry = worklist.pop(); - var { body, ppoint, gcInfo, preGCLive } = entry; + var { body, ppoint, gcInfo } = entry; - // Handle the case where there are multiple ways to reach this point - // (traversing backwards). - var visited = bodies_visited.get(body); - if (!visited) - bodies_visited.set(body, visited = new Map()); - if (visited.has(ppoint)) { - var seenEntry = visited.get(ppoint); - - // This point already knows how to GC through some other path, so - // we have nothing new to learn. (The other path will consider the - // predecessors.) - if (seenEntry.gcInfo) - continue; - - // If this worklist's entry doesn't know of any way to GC, then - // there's no point in continuing the traversal through it. Perhaps - // another edge will be found that *can* GC; otherwise, the first - // route to the point will traverse through predecessors. - // - // Note that this means we may visit a point more than once, if the - // first time we visit we don't have a known reachable GC call and - // the second time we do. - if (!gcInfo) - continue; + if (body.seen) { + if (ppoint in body.seen) { + var seenEntry = body.seen[ppoint]; + if (!gcInfo || seenEntry.gcInfo) + continue; + } + } else { + body.seen = []; } - visited.set(ppoint, {body: body, gcInfo: gcInfo}); + body.seen[ppoint] = {body: body, gcInfo: gcInfo}; - // Check for hitting the entry point of the current body (which may be - // the outer function or a loop within it.) if (ppoint == body.Index[0]) { if (body.BlockId.Kind == "Loop") { - // Propagate to outer body parents that enter the loop body. + // propagate to parents that enter the loop body. if ("BlockPPoint" in body) { for (var parent of body.BlockPPoint) { var found = false; @@ -402,13 +373,7 @@ function findGCBeforeVariableUse(start_body, start_point, suppressed, variable) } else if (variable.Kind == "Arg" && gcInfo) { // The scope of arguments starts at the beginning of the // function - return entry; - } else if (entry.preGCLive) { - // We didn't find a "good" explanation beginning of the live - // range, but we do know the variable was live across the GC. - // This can happen if the live range started when a variable is - // used as a retparam. - return entry; + return {gcInfo: gcInfo, why: entry}; } } @@ -434,52 +399,25 @@ function findGCBeforeVariableUse(start_body, start_point, suppressed, variable) // to a use after the GC call that proves its live range // extends at least that far. if (gcInfo) - return {body: body, ppoint: source, gcInfo: gcInfo, why: entry }; + return {gcInfo: gcInfo, why: {body: body, ppoint: source, gcInfo: gcInfo, why: entry } } - // Otherwise, keep searching through the graph, but truncate - // this particular branch of the search at this edge. + // Otherwise, we want to continue searching for the true + // minimumUse, for use in reporting unnecessary rooting, but we + // truncate this particular branch of the search at this edge. continue; } - var src_gcInfo = gcInfo; - var src_preGCLive = preGCLive; if (!gcInfo && !(source in body.suppressed) && !suppressed) { var gcName = edgeCanGC(edge, body); if (gcName) - src_gcInfo = {name:gcName, body:body, ppoint:source}; + gcInfo = {name:gcName, body:body, ppoint:source}; } if (edge_uses) { // The live range starts at least this far back, so we're done - // for the same reason as with edge_kills. The only difference - // is that a GC on this edge indicates a hazard, whereas if - // we're killing a live range in the GC call then it's not live - // *across* the call. - // - // However, we may want to generate a longer usage chain for - // the variable than is minimally necessary. For example, - // consider: - // - // Value v = f(); - // if (v.isUndefined()) - // return false; - // gc(); - // return v; - // - // The call to .isUndefined() is considered to be a use and - // therefore indicates that v must be live at that point. But - // it's more helpful to the user to continue the 'why' path to - // include the ancestor where the value was generated. So we - // will only return here if edge.Kind is Assign; otherwise, - // we'll pass a "preGCLive" value up through the worklist to - // remember that the variable *is* alive before the GC and so - // this function should be returning a true value. - - if (src_gcInfo) { - src_preGCLive = true; - if (edge.Kind == 'Assign') - return {body:body, ppoint:source, gcInfo:src_gcInfo, why:entry}; - } + // for the same reason as with edge_kills. + if (gcInfo) + return {gcInfo:gcInfo, why:entry}; } if (edge.Kind == "Loop") { @@ -491,8 +429,7 @@ function findGCBeforeVariableUse(start_body, start_point, suppressed, variable) assert(!found); found = true; worklist.push({body:xbody, ppoint:xbody.Index[1], - preGCLive: src_preGCLive, gcInfo:src_gcInfo, - why:entry}); + gcInfo:gcInfo, why:entry}); } } assert(found); @@ -500,9 +437,7 @@ function findGCBeforeVariableUse(start_body, start_point, suppressed, variable) } // Propagate the search to the predecessors of this edge. - worklist.push({body:body, ppoint:source, - preGCLive: src_preGCLive, gcInfo:src_gcInfo, - why:entry}); + worklist.push({body:body, ppoint:source, gcInfo:gcInfo, why:entry}); } } @@ -511,12 +446,13 @@ function findGCBeforeVariableUse(start_body, start_point, suppressed, variable) function variableLiveAcrossGC(suppressed, variable) { - // A variable is live across a GC if (1) it is used by an edge (as in, it - // was at least initialized), and (2) it is used after a GC in a successor - // edge. + // A variable is live across a GC if (1) it is used by an edge, and (2) it + // is used after a GC in a successor edge. - for (var body of functionBodies) + for (var body of functionBodies) { + body.seen = null; body.minimumUse = 0; + } for (var body of functionBodies) { if (!("PEdge" in body)) @@ -531,7 +467,8 @@ function variableLiveAcrossGC(suppressed, variable) // if (usePoint && !edgeKillsVariable(edge, variable)) { // Found a use, possibly after a GC. - var call = findGCBeforeVariableUse(body, usePoint, suppressed, variable); + var worklist = [{body:body, ppoint:usePoint, gcInfo:null, why:null}]; + var call = findGCBeforeVariableUse(suppressed, variable, worklist); if (!call) continue; @@ -564,9 +501,7 @@ function unsafeVariableAddressTaken(suppressed, variable) return null; } -// Read out the brief (non-JSON, semi-human-readable) CFG description for the -// given function and store it. -function loadPrintedLines(functionName) +function computePrintedLines(functionName) { assert(!os.system("xdbfind src_body.xdb '" + functionName + "' > " + tmpfile)); var lines = snarf(tmpfile).split('\n'); @@ -601,21 +536,13 @@ function loadPrintedLines(functionName) } } -function findLocation(body, ppoint, opts={brief: false}) +function findLocation(body, ppoint) { var location = body.PPoint[ppoint - 1].Location; - var file = location.CacheString; - - if (file.indexOf(sourceRoot) == 0) - file = file.substring(sourceRoot.length); - - if (opts.brief) { - var m = /.*\/(.*)/.exec(file); - if (m) - file = m[1]; - } - - return file + ":" + location.Line; + var text = location.CacheString + ":" + location.Line; + if (text.indexOf(sourceRoot) == 0) + return text.substring(sourceRoot.length); + return text; } function locationLine(text) @@ -630,11 +557,11 @@ function printEntryTrace(functionName, entry) var gcPoint = entry.gcInfo ? entry.gcInfo.ppoint : 0; if (!functionBodies[0].lines) - loadPrintedLines(functionName); + computePrintedLines(functionName); while (entry) { var ppoint = entry.ppoint; - var lineText = findLocation(entry.body, ppoint, {"brief": true}); + var lineText = findLocation(entry.body, ppoint); var edgeText = ""; if (entry.why && entry.why.body == entry.body) { @@ -645,8 +572,8 @@ function printEntryTrace(functionName, entry) var table = {}; entry.body.edgeTable = table; for (var line of entry.body.lines) { - if (match = /\((\d+,\d+),/.exec(line)) - table[match[1]] = line; // May be multiple? + if (match = /\((\d+),(\d+),/.exec(line)) + table[match[1] + "," + match[2]] = line; // May be multiple? } } @@ -732,7 +659,7 @@ function processBodies(functionName) " of type '" + typeDesc(variable.Type) + "'" + " live across GC call " + result.gcInfo.name + " at " + lineText); - printEntryTrace(functionName, result); + printEntryTrace(functionName, result.why); } result = unsafeVariableAddressTaken(suppressed, variable.Variable); if (result) { @@ -756,9 +683,9 @@ var minStream = xdb.min_data_stream()|0; var maxStream = xdb.max_data_stream()|0; var N = (maxStream - minStream) + 1; -var start = Math.floor((batch - 1) / numBatches * N) + minStream; -var start_next = Math.floor(batch / numBatches * N) + minStream; -var end = start_next - 1; +var each = Math.floor(N/numBatches); +var start = minStream + each * (batch - 1); +var end = Math.min(minStream + each * batch - 1, maxStream); function process(name, json) { functionName = name; diff --git a/js/src/devtools/rootAnalysis/annotations.js b/js/src/devtools/rootAnalysis/annotations.js index ea26539dc64d..b9072dbfb8f2 100644 --- a/js/src/devtools/rootAnalysis/annotations.js +++ b/js/src/devtools/rootAnalysis/annotations.js @@ -2,10 +2,6 @@ "use strict"; -// RAII types within which we should assume GC is suppressed, eg -// AutoSuppressGC. -var GCSuppressionTypes = []; - // Ignore calls made through these function pointers var ignoreIndirectCalls = { "mallocSizeOf" : true, @@ -85,7 +81,6 @@ var ignoreCallees = { "std::strstreambuf._M_alloc_fun" : true, "std::strstreambuf._M_free_fun" : true, "struct js::gc::Callback.op" : true, - "mozilla::ThreadSharedFloatArrayBufferList::Storage.mFree" : true, }; function fieldCallCannotGC(csu, fullfield) @@ -151,17 +146,10 @@ function ignoreEdgeAddressTaken(edge) return false; } -// Return whether csu.method is one that we claim can never GC. -function isSuppressedVirtualMethod(csu, method) -{ - return csu == "nsISupports" && (method == "AddRef" || method == "Release"); -} - // Ignore calls of these functions (so ignore any stack containing these) var ignoreFunctions = { "ptio.c:pt_MapError" : true, "je_malloc_printf" : true, - "vprintf_stderr" : true, "PR_ExplodeTime" : true, "PR_ErrorInstallTable" : true, "PR_SetThreadPrivate" : true, @@ -192,7 +180,6 @@ var ignoreFunctions = { // up wrapping a pending exception. See bug 898815 for the heavyweight fix. "void js::AutoCompartment::~AutoCompartment(int32)" : true, "void JSAutoCompartment::~JSAutoCompartment(int32)" : true, - "void js::AutoClearTypeInferenceStateOnOOM::~AutoClearTypeInferenceStateOnOOM()" : true, // Bug 948646 - the only thing AutoJSContext's constructor calls // is an Init() routine whose entire body is covered with an @@ -225,13 +212,6 @@ var ignoreFunctions = { "uint64 js::TenuringTracer::moveObjectToTenured(JSObject*, JSObject*, int32)" : true, "uint32 js::TenuringTracer::moveObjectToTenured(JSObject*, JSObject*, int32)" : true, "void js::Nursery::freeMallocedBuffers()" : true, - - // It would be cool to somehow annotate that nsTHashtable will use - // nsTHashtable::s_MatchEntry for its matchEntry function pointer, but - // there is no mechanism for that. So we will just annotate a particularly - // troublesome logging-related usage. - "EntryType* nsTHashtable::PutEntry(nsTHashtable::KeyType) [with EntryType = nsBaseHashtableET >; nsTHashtable::KeyType = const char*]" : true, - "EntryType* nsTHashtable::GetEntry(nsTHashtable::KeyType) const [with EntryType = nsBaseHashtableET >; nsTHashtable::KeyType = const char*]" : true, }; function isProtobuf(name) @@ -333,31 +313,16 @@ function isUnsafeStorage(typeName) return typeName.startsWith('UniquePtr<'); } -function isSuppressConstructor(edgeType, varName) +function isSuppressConstructor(varName) { - // Check whether this could be a constructor - if (edgeType.Kind != 'Function') - return false; - if (!('TypeFunctionCSU' in edgeType)) - return false; - if (edgeType.Type.Kind != 'Void') - return false; - - // Check whether the type is a known suppression type. - var type = edgeType.TypeFunctionCSU.Type.Name; - if (GCSuppressionTypes.indexOf(type) == -1) - return false; - - // And now make sure this is the constructor, not some other method on a - // suppression type. varName[0] contains the qualified name. - var [ mangled, unmangled ] = splitFunction(varName[0]); - if (mangled.search(/C\dE/) == -1) - return false; // Mangled names of constructors have CE - var m = unmangled.match(/([~\w]+)(?:<.*>)?\(/); - if (!m) - return false; - var type_stem = type.replace(/\w+::/g, '').replace(/\<.*\>/g, ''); - return m[1] == type_stem; + // varName[1] contains the unqualified name + return [ + "AutoSuppressGC", + "AutoAssertGCCallback", + "AutoEnterAnalysis", + "AutoSuppressGCAnalysis", + "AutoIgnoreRootingHazards" + ].indexOf(varName[1]) != -1; } // nsISupports subclasses' methods may be scriptable (or overridden diff --git a/js/src/devtools/rootAnalysis/build/sixgill.manifest b/js/src/devtools/rootAnalysis/build/sixgill.manifest index d02bb1bf4226..de0d12c709c6 100644 --- a/js/src/devtools/rootAnalysis/build/sixgill.manifest +++ b/js/src/devtools/rootAnalysis/build/sixgill.manifest @@ -1,10 +1,10 @@ [ -{ -"digest" : "36dc644e24c0aa824975ad8f5c15714445d5cb064d823000c3cb637e885199414d7df551e6b99233f0656dcf5760918192ef04113c486af37f3c489bb93ad029", -"size" : 2631908, -"hg_id" : "8cb9c3fb039a+ tip", -"unpack" : true, -"filename" : "sixgill.tar.xz", -"algorithm" : "sha512" -} + { + "hg_id" : "cd93f15a30ce", + "algorithm" : "sha512", + "digest" : "541eb3842ab6b91bd87223cad7a5e4387ef3e496e5b580c8047b8b586bc7eb69fecf3c9eb8c45a1e0deebb53554f0e8acedfe1b4ca64d93b6d008f3f2eb11389", + "filename" : "sixgill.tar.xz", + "size" : 2626640, + "unpack" : true + } ] diff --git a/js/src/devtools/rootAnalysis/computeCallgraph.js b/js/src/devtools/rootAnalysis/computeCallgraph.js index dab3f76216f6..5aeae6af843d 100644 --- a/js/src/devtools/rootAnalysis/computeCallgraph.js +++ b/js/src/devtools/rootAnalysis/computeCallgraph.js @@ -12,19 +12,25 @@ if (scriptArgs[0] == '--function') { scriptArgs = scriptArgs.slice(2); } -var typeInfo_filename = scriptArgs[0] || "typeInfo.txt"; +var subclasses = {}; +var superclasses = {}; +var classFunctions = {}; -var subclasses = new Map(); // Map from csu => set of immediate subclasses -var superclasses = new Map(); // Map from csu => set of immediate superclasses -var classFunctions = new Map(); // Map from "csu:name" => set of full method name +var fieldCallSeen = {}; -var virtualResolutionsSeen = new Set(); - -function addEntry(map, name, entry) +function addClassEntry(index, name, other) { - if (!map.has(name)) - map.set(name, new Set()); - map.get(name).add(entry); + if (!(name in index)) { + index[name] = [other]; + return; + } + + for (var entry of index[name]) { + if (entry == other) + return; + } + + index[name].push(other); } // CSU is "Class/Struct/Union" @@ -37,111 +43,86 @@ function processCSU(csuName, csu) var superclass = field.Field[1].Type.Name; var subclass = field.Field[1].FieldCSU.Type.Name; assert(subclass == csuName); - addEntry(subclasses, superclass, subclass); - addEntry(superclasses, subclass, superclass); + addClassEntry(subclasses, superclass, subclass); + addClassEntry(superclasses, subclass, superclass); } if ("Variable" in field) { // Note: not dealing with overloading correctly. var name = field.Variable.Name[0]; var key = csuName + ":" + field.Field[0].Name[0]; - addEntry(classFunctions, key, name); + if (!(key in classFunctions)) + classFunctions[key] = []; + classFunctions[key].push(name); } } } -// Return the nearest ancestor method definition, or all nearest definitions in -// the case of multiple inheritance. -function nearestAncestorMethods(csu, method) +function findVirtualFunctions(initialCSU, field, suppressed) { - var key = csu + ":" + method; + var worklist = [initialCSU]; + var functions = []; - if (classFunctions.has(key)) - return new Set(classFunctions.get(key)); + // Virtual call targets on subclasses of nsISupports may be incomplete, + // if the interface is scriptable. Just treat all indirect calls on + // nsISupports objects as potentially GC'ing, except AddRef/Release + // which should never enter the JS engine (even when calling dtors). + while (worklist.length) { + var csu = worklist.pop(); + if (csu == "nsISupports" && (field == "AddRef" || field == "Release")) { + suppressed[0] = true; + return []; + } + if (isOverridableField(initialCSU, csu, field)) { + // We will still resolve the virtual function call, because it's + // nice to have as complete a callgraph as possible for other uses. + // But push a token saying that we can run arbitrary code. + functions.push(null); + } - var functions = new Set(); - if (superclasses.has(csu)) { - for (var parent of superclasses.get(csu)) - functions.update(nearestAncestorMethods(parent, method)); + if (csu in superclasses) { + for (var superclass of superclasses[csu]) + worklist.push(superclass); + } + } + + worklist = [csu]; + while (worklist.length) { + var csu = worklist.pop(); + var key = csu + ":" + field; + + if (key in classFunctions) { + for (var name of classFunctions[key]) + functions.push(name); + } + + if (csu in subclasses) { + for (var subclass of subclasses[csu]) + worklist.push(subclass); + } } return functions; } -// Return [ instantations, suppressed ], where instantiations is a Set of all -// possible implementations of 'field' given static type 'initialCSU', plus -// null if arbitrary other implementations are possible, and suppressed is true -// if we the method is assumed to be non-GC'ing by annotation. -function findVirtualFunctions(initialCSU, field) -{ - var worklist = [initialCSU]; - var functions = new Set(); - - // Loop through all methods of initialCSU (by looking at all methods of ancestor csus). - // - // If field is nsISupports::AddRef or ::Release, return an empty list and a - // boolean that says we assert that it cannot GC. - // - // If this is a method that is annotated to be dangerous (eg, it could be - // overridden with an implementation that could GC), then use null as a - // signal value that it should be considered to GC, even though we'll also - // collect all of the instantiations for other purposes. - - while (worklist.length) { - var csu = worklist.pop(); - if (isSuppressedVirtualMethod(csu, field)) - return [ new Set(), true ]; - if (isOverridableField(initialCSU, csu, field)) { - // We will still resolve the virtual function call, because it's - // nice to have as complete a callgraph as possible for other uses. - // But push a token saying that we can run arbitrary code. - functions.add(null); - } - - if (superclasses.has(csu)) - worklist.push(...superclasses.get(csu)); - } - - // Now return a list of all the instantiations of the method named 'field' - // that could execute on an instance of initialCSU or a descendant class. - - // Start with the class itself, or if it doesn't define the method, all - // nearest ancestor definitions. - functions.update(nearestAncestorMethods(initialCSU, field)); - - // Then recurse through all descendants to add in their definitions. - var worklist = [initialCSU]; - while (worklist.length) { - var csu = worklist.pop(); - var key = csu + ":" + field; - - if (classFunctions.has(key)) - functions.update(classFunctions.get(key)); - - if (subclasses.has(csu)) - worklist.push(...subclasses.get(csu)); - } - - return [ functions, false ]; -} - -var memoized = new Map(); +var memoized = {}; var memoizedCount = 0; function memo(name) { - if (!memoized.has(name)) { - let id = memoized.size + 1; - memoized.set(name, "" + id); - print(`#${id} ${name}`); + if (!(name in memoized)) { + memoizedCount++; + memoized[name] = "" + memoizedCount; + print("#" + memoizedCount + " " + name); } - return memoized.get(name); + return memoized[name]; } +var seenCallees = null; +var seenSuppressedCallees = null; + // Return a list of all callees that the given edge might be a call to. Each // one is represented by an object with a 'kind' field that is one of -// ('direct', 'field', 'resolved-field', 'indirect', 'unknown'), though note -// that 'resolved-field' is really a global record of virtual method -// resolutions, indepedent of this particular edge. +// ('direct', 'field', 'indirect', 'unknown'). function getCallees(edge) { if (edge.Kind != "Call") @@ -158,42 +139,42 @@ function getCallees(edge) var field = callee.Exp[0].Field; var fieldName = field.Name[0]; var csuName = field.FieldCSU.Type.Name; - var functions; + var functions = null; if ("FieldInstanceFunction" in field) { - let suppressed; - [ functions, suppressed ] = findVirtualFunctions(csuName, fieldName, suppressed); - if (suppressed) { + var suppressed = [ false ]; + functions = findVirtualFunctions(csuName, fieldName, suppressed); + if (suppressed[0]) { // Field call known to not GC; mark it as suppressed so // direct invocations will be ignored callees.push({'kind': "field", 'csu': csuName, 'field': fieldName, 'suppressed': true}); } - } else { - functions = new Set([null]); // field call } - - // Known set of virtual call targets. Treat them as direct calls to - // all possible resolved types, but also record edges from this - // field call to each final callee. When the analysis is checking - // whether an edge can GC and it sees an unrooted pointer held live - // across this field call, it will know whether any of the direct - // callees can GC or not. - var targets = []; - var fullyResolved = true; - for (var name of functions) { - if (name === null) { - // Unknown set of call targets, meaning either a function - // pointer call ("field call") or a virtual method that can - // be overridden in extensions. - callees.push({'kind': "field", 'csu': csuName, 'field': fieldName}); - fullyResolved = false; - } else { - callees.push({'kind': "direct", 'name': name}); - targets.push({'kind': "direct", 'name': name}); + if (functions) { + // Known set of virtual call targets. Treat them as direct + // calls to all possible resolved types, but also record edges + // from this field call to each final callee. When the analysis + // is checking whether an edge can GC and it sees an unrooted + // pointer held live across this field call, it will know + // whether any of the direct callees can GC or not. + var targets = []; + var fullyResolved = true; + for (var name of functions) { + if (name === null) { + // virtual call on an nsISupports object + callees.push({'kind': "field", 'csu': csuName, 'field': fieldName}); + fullyResolved = false; + } else { + callees.push({'kind': "direct", 'name': name}); + targets.push({'kind': "direct", 'name': name}); + } } + if (fullyResolved) + callees.push({'kind': "resolved-field", 'csu': csuName, 'field': fieldName, 'callees': targets}); + } else { + // Unknown set of call targets. Non-virtual field call. + callees.push({'kind': "field", 'csu': csuName, 'field': fieldName}); } - if (fullyResolved) - callees.push({'kind': "resolved-field", 'csu': csuName, 'field': fieldName, 'callees': targets}); } else if (callee.Exp[0].Kind == "Var") { // indirect call through a variable. callees.push({'kind': "indirect", 'variable': callee.Exp[0].Variable.Name[0]}); @@ -238,6 +219,7 @@ function getTags(functionName, body) { var tags = new Set(); var annotations = getAnnotations(body); if (functionName in annotations) { + print("crawling through"); for (var [ annName, annValue ] of annotations[functionName]) { if (annName == 'Tag') tags.add(annValue); @@ -254,45 +236,39 @@ function processBody(functionName, body) for (var tag of getTags(functionName, body).values()) print("T " + memo(functionName) + " " + tag); - // Set of all callees that have been output so far, in order to suppress - // repeated callgraph edges from being recorded. Use a separate set for - // suppressed callees, since we don't want a suppressed edge (within one - // RAII scope) to prevent an unsuppressed edge from being recorded. The - // seen array is indexed by a boolean 'suppressed' variable. - var seen = [ new Set(), new Set() ]; - lastline = null; for (var edge of body.PEdge) { if (edge.Kind != "Call") continue; - - // Whether this call is within the RAII scope of a GC suppression class - var edgeSuppressed = (edge.Index[0] in body.suppressed); - + var edgeSuppressed = false; + var seen = seenCallees; + if (edge.Index[0] in body.suppressed) { + edgeSuppressed = true; + seen = seenSuppressedCallees; + } for (var callee of getCallees(edge)) { - var suppressed = Boolean(edgeSuppressed || callee.suppressed); - var prologue = suppressed ? "SUPPRESS_GC " : ""; + var prologue = (edgeSuppressed || callee.suppressed) ? "SUPPRESS_GC " : ""; prologue += memo(functionName) + " "; if (callee.kind == 'direct') { - if (!seen[+suppressed].has(callee.name)) { - seen[+suppressed].add(callee.name); + if (!(callee.name in seen)) { + seen[callee.name] = true; printOnce("D " + prologue + memo(callee.name)); } } else if (callee.kind == 'field') { var { csu, field } = callee; printOnce("F " + prologue + "CLASS " + csu + " FIELD " + field); } else if (callee.kind == 'resolved-field') { - // Fully-resolved field (virtual method) call. Record the - // callgraph edges. Do not consider suppression, since it is - // local to this callsite and we are writing out a global + // Fully-resolved field call (usually a virtual method). Record + // the callgraph edges. Do not consider suppression, since it + // is local to this callsite and we are writing out a global // record here. // // Any field call that does *not* have an R entry must be // assumed to call anything. var { csu, field, callees } = callee; var fullFieldName = csu + "." + field; - if (!virtualResolutionsSeen.has(fullFieldName)) { - virtualResolutionsSeen.add(fullFieldName); + if (!(fullFieldName in fieldCallSeen)) { + fieldCallSeen[fullFieldName] = true; for (var target of callees) printOnce("R " + memo(fullFieldName) + " " + memo(target.name)); } @@ -308,7 +284,7 @@ function processBody(functionName, body) } } -GCSuppressionTypes = loadTypeInfo(typeInfo_filename)["Suppress GC"] || []; +var callgraph = {}; var xdb = xdbLibrary(); xdb.open("src_comp.xdb"); @@ -346,12 +322,14 @@ function process(functionName, functionBodies) { for (var body of functionBodies) body.suppressed = []; - for (var body of functionBodies) { for (var [pbody, id] of allRAIIGuardedCallPoints(functionBodies, body, isSuppressConstructor)) pbody.suppressed[id] = true; } + seenCallees = {}; + seenSuppressedCallees = {}; + for (var body of functionBodies) processBody(functionName, body); @@ -373,7 +351,7 @@ function process(functionName, functionBodies) // Bug 1056410: Oh joy. GCC does something even funkier internally, // where it generates calls to ~Foo() but a body for ~Foo(int32) even // though it uses the same mangled name for both. So we need to add a - // synthetic edge from ~Foo() -> ~Foo(int32). + // synthetic edge from the former to the latter. // // inChargeXTor will have the (int32). if (functionName.indexOf("::~") > 0) { @@ -391,7 +369,7 @@ function process(functionName, functionBodies) // D1 # complete object destructor // D2 # base object destructor // - // In actual practice, I have observed C4 and D4 xtors generated by gcc + // In actual practice, I have observed a C4 constructor generated by gcc // 4.9.3 (but not 4.7.3). The gcc source code says: // // /* This is the old-style "[unified]" constructor. @@ -399,29 +377,23 @@ function process(functionName, functionBodies) // it from the clones in order to share code and save space. */ // // Unfortunately, that "call... from the clones" does not seem to appear in - // the CFG we get from GCC. So if we see a C4 constructor or D4 destructor, - // inject an edge to it from C1, C2, and C3 (or D1, D2, and D3). (Note that - // C3 isn't even used in current GCC, but add the edge anyway just in - // case.) - if (functionName.indexOf("C4E") != -1 || functionName.indexOf("D4Ev") != -1) { + // the CFG we get from GCC. So if we see a C4 constructor, inject an edge + // to it from C1, C2, and C3. (Note that C3 isn't even used in current GCC, + // but add the edge anyway just in case.) + if (functionName.indexOf("C4E") != -1) { var [ mangled, unmangled ] = splitFunction(functionName); // E terminates the method name (and precedes the method parameters). - // If eg "C4E" shows up in the mangled name for another reason, this - // will create bogus edges in the callgraph. But will affect little and - // is somewhat difficult to avoid, so we will live with it. - for (let [synthetic, variant] of [['C4E', 'C1E'], - ['C4E', 'C2E'], - ['C4E', 'C3E'], - ['D4Ev', 'D1Ev'], - ['D4Ev', 'D2Ev'], - ['D4Ev', 'D3Ev']]) - { - if (mangled.indexOf(synthetic) == -1) - continue; - - let variant_mangled = mangled.replace(synthetic, variant); - let variant_full = variant_mangled + "$" + unmangled; - print("D " + memo(variant_full) + " " + memo(functionName)); + if (mangled.indexOf("C4E") != -1) { + // If "C4E" shows up in the mangled name for another reason, this + // will create bogus edges in the callgraph. But that shouldn't + // matter too much, and is somewhat difficult to avoid, so we will + // live with it. + var C1 = mangled.replace("C4E", "C1E"); + var C2 = mangled.replace("C4E", "C2E"); + var C3 = mangled.replace("C4E", "C3E"); + print("D " + memo(C1) + " " + memo(mangled)); + print("D " + memo(C2) + " " + memo(mangled)); + print("D " + memo(C3) + " " + memo(mangled)); } } } diff --git a/js/src/devtools/rootAnalysis/computeGCFunctions.js b/js/src/devtools/rootAnalysis/computeGCFunctions.js index 97efcb38af22..086cfef35886 100644 --- a/js/src/devtools/rootAnalysis/computeGCFunctions.js +++ b/js/src/devtools/rootAnalysis/computeGCFunctions.js @@ -21,20 +21,16 @@ loadCallgraph(callgraph_filename); printErr("Writing " + gcFunctions_filename); redirect(gcFunctions_filename); - for (var name in gcFunctions) { - for (let readable of readableNames[name]) { - print(""); - print("GC Function: " + name + "$" + readable); - let current = name; - do { - current = gcFunctions[current]; - if (current in readableNames) - print(" " + readableNames[current][0]); - else - print(" " + current); - } while (current in gcFunctions); - } + print(""); + print("GC Function: " + name + "$" + readableNames[name][0]); + do { + name = gcFunctions[name]; + if (name in readableNames) + print(" " + readableNames[name][0]); + else + print(" " + name); + } while (name in gcFunctions); } printErr("Writing " + gcFunctionsList_filename); diff --git a/js/src/devtools/rootAnalysis/computeGCTypes.js b/js/src/devtools/rootAnalysis/computeGCTypes.js index af4d703896e1..f0712ecb47db 100644 --- a/js/src/devtools/rootAnalysis/computeGCTypes.js +++ b/js/src/devtools/rootAnalysis/computeGCTypes.js @@ -5,20 +5,14 @@ loadRelativeToScript('utility.js'); loadRelativeToScript('annotations.js'); -var gcTypes_filename = scriptArgs[0] || "gcTypes.txt"; -var typeInfo_filename = scriptArgs[1] || "typeInfo.txt"; - var annotations = { 'GCPointers': [], 'GCThings': [], 'NonGCTypes': {}, // unused 'NonGCPointers': {}, 'RootedPointers': {}, - 'GCSuppressors': {}, }; -var gDescriptors = new Map; // Map from descriptor string => Set of typeName - var structureParents = {}; // Map from field => list of var pointerParents = {}; // Map from field => list of var baseClasses = {}; // Map from struct name => list of base class name strings @@ -70,8 +64,6 @@ function processCSU(csu, body) annotations.NonGCPointers[csu] = true; else if (tag == 'Rooted Pointer') annotations.RootedPointers[csu] = true; - else if (tag == 'Suppress GC') - annotations.GCSuppressors[csu] = true; } } @@ -215,26 +207,6 @@ function addGCPointer(typeName) markGCType(typeName, '', '(annotation)', 1, 0, ""); } -// Add an arbitrary descriptor to a type, and apply it recursively to all base -// structs and structs that contain the given typeName as a field. -function addDescriptor(typeName, descriptor) -{ - if (!gDescriptors.has(descriptor)) - gDescriptors.set(descriptor, new Set); - let descriptorTypes = gDescriptors.get(descriptor); - if (!descriptorTypes.has(typeName)) { - descriptorTypes.add(typeName); - if (typeName in structureParents) { - for (let [holder, field] of structureParents[typeName]) - addDescriptor(holder, descriptor); - } - if (typeName in baseClasses) { - for (let base of baseClasses[typeName]) - addDescriptor(base, descriptor); - } - } -} - for (var type of listNonGCPointers()) annotations.NonGCPointers[type] = true; @@ -274,8 +246,6 @@ function explain(csu, indent, seen) { } } -var origOut = os.file.redirect(gcTypes_filename); - for (var csu in gcTypes) { print("GCThing: " + csu); explain(csu, " "); @@ -284,16 +254,3 @@ for (var csu in gcPointers) { print("GCPointer: " + csu); explain(csu, " "); } - -// Redirect output to the typeInfo file and close the gcTypes file. -os.file.close(os.file.redirect(typeInfo_filename)); - -for (let csu in annotations.GCSuppressors) - addDescriptor(csu, 'Suppress GC'); - -for (let [descriptor, types] of gDescriptors) { - for (let csu of types) - print(descriptor + "$$" + csu); -} - -os.file.close(os.file.redirect(origOut)); diff --git a/js/src/devtools/rootAnalysis/explain.py b/js/src/devtools/rootAnalysis/explain.py index dc8b76f5c687..ec7ef1949b1d 100755 --- a/js/src/devtools/rootAnalysis/explain.py +++ b/js/src/devtools/rootAnalysis/explain.py @@ -3,8 +3,6 @@ import re import argparse -from collections import defaultdict - parser = argparse.ArgumentParser(description='Process some integers.') parser.add_argument('rootingHazards', nargs='?', default='rootingHazards.txt') parser.add_argument('gcFunctions', nargs='?', default='gcFunctions.txt') @@ -24,7 +22,7 @@ try: # Map from a GC function name to the list of hazards resulting from # that GC function - hazardousGCFunctions = defaultdict(list) + hazardousGCFunctions = {} # List of tuples (gcFunction, index of hazard) used to maintain the # ordering of the hazards @@ -55,7 +53,7 @@ try: # Function names are surrounded by single quotes. Field calls # are unquoted. current_gcFunction = m.group(2) - hazardousGCFunctions[current_gcFunction].append(line) + hazardousGCFunctions.setdefault(current_gcFunction, []).append(line) hazardOrder.append((current_gcFunction, len(hazardousGCFunctions[current_gcFunction]) - 1)) num_hazards += 1 continue @@ -86,13 +84,12 @@ try: if current_func: gcExplanations[current_func] = explanation - for gcFunction, index in hazardOrder: - gcHazards = hazardousGCFunctions[gcFunction] - - if gcFunction in gcExplanations: - print >>hazards, (gcHazards[index] + gcExplanations[gcFunction]) - else: - print >>hazards, gcHazards[index] + for gcFunction, index in hazardOrder: + gcHazards = hazardousGCFunctions[gcFunction] + if gcFunction in gcExplanations: + print >>hazards, (gcHazards[index] + gcExplanations[gcFunction]) + else: + print >>hazards, gcHazards[index] except IOError as e: print 'Failed: %s' % str(e) diff --git a/js/src/devtools/rootAnalysis/loadCallgraph.js b/js/src/devtools/rootAnalysis/loadCallgraph.js index 7661aad8e53a..a221d7143764 100644 --- a/js/src/devtools/rootAnalysis/loadCallgraph.js +++ b/js/src/devtools/rootAnalysis/loadCallgraph.js @@ -53,8 +53,13 @@ function addGCFunction(caller, reason) function addCallEdge(caller, callee, suppressed) { - addToKeyedList(calleeGraph, caller, {callee:callee, suppressed:suppressed}); - addToKeyedList(callerGraph, callee, {caller:caller, suppressed:suppressed}); + if (!(caller in calleeGraph)) + calleeGraph[caller] = []; + calleeGraph[caller].push({callee:callee, suppressed:suppressed}); + + if (!(callee in callerGraph)) + callerGraph[callee] = []; + callerGraph[callee].push({caller:caller, suppressed:suppressed}); } // Map from identifier to full "mangled|readable" name. Or sometimes to a diff --git a/js/src/devtools/rootAnalysis/run-test.py b/js/src/devtools/rootAnalysis/run-test.py index 3bc9085a0aaf..a960b57d1399 100644 --- a/js/src/devtools/rootAnalysis/run-test.py +++ b/js/src/devtools/rootAnalysis/run-test.py @@ -3,87 +3,150 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +import sys import os -import site +import re +import json import subprocess -import argparse -testdir = os.path.abspath(os.path.join(os.path.dirname(__file__), 't')) -site.addsitedir(testdir) -from testlib import Test, equal +testdir = os.path.abspath(os.path.dirname(__file__)) -scriptdir = os.path.abspath(os.path.dirname(__file__)) - -parser = argparse.ArgumentParser(description='run hazard analysis tests') -parser.add_argument( - '--js', default=os.environ.get('JS'), - help='JS binary to run the tests with') -parser.add_argument( - '--sixgill', default=os.environ.get('SIXGILL', os.path.join(testdir, "sixgill")), - help='Path to root of sixgill installation') -parser.add_argument( - '--sixgill-bin', default=os.environ.get('SIXGILL_BIN'), - help='Path to sixgill binary dir') -parser.add_argument( - '--sixgill-plugin', default=os.environ.get('SIXGILL_PLUGIN'), - help='Full path to sixgill gcc plugin') -parser.add_argument( - '--gccdir', default=os.environ.get('GCCDIR'), - help='Path to GCC installation dir') -parser.add_argument( - '--cc', default=os.environ.get('CC'), - help='Path to gcc') -parser.add_argument( - '--cxx', default=os.environ.get('CXX'), - help='Path to g++') -parser.add_argument( - '--verbose', '-v', action='store_true', - help='Display verbose output, including commands executed') -parser.add_argument( - 'tests', nargs='*', default=['sixgill-tree', 'suppression', 'hazards', 'exceptions'], - help='tests to run') - -cfg = parser.parse_args() - -if not cfg.js: - exit('Must specify JS binary through environment variable or --js option') -if not cfg.cc: - if cfg.gccdir: - cfg.cc = os.path.join(cfg.gccdir, "bin", "gcc") - else: - cfg.cc = "gcc" -if not cfg.cxx: - if cfg.gccdir: - cfg.cxx = os.path.join(cfg.gccdir, "bin", "g++") - else: - cfg.cxx = "g++" -if not cfg.sixgill_bin: - cfg.sixgill_bin = os.path.join(cfg.sixgill, "usr", "bin") -if not cfg.sixgill_plugin: - cfg.sixgill_plugin = os.path.join(cfg.sixgill, "usr", "libexec", "sixgill", "gcc", "xgill.so") - -subprocess.check_call([cfg.js, '-e', 'if (!getBuildConfiguration()["has-ctypes"]) quit(1)']) +cfg = {} +cfg['SIXGILL_ROOT'] = os.environ.get('SIXGILL', + os.path.join(testdir, "sixgill")) +cfg['SIXGILL_BIN'] = os.environ.get('SIXGILL_BIN', + os.path.join(cfg['SIXGILL_ROOT'], "usr", "bin")) +cfg['SIXGILL_PLUGIN'] = os.environ.get('SIXGILL_PLUGIN', + os.path.join(cfg['SIXGILL_ROOT'], "usr", "libexec", "sixgill", "gcc", "xgill.so")) +cfg['CC'] = os.environ.get("CC", + "gcc") +cfg['CXX'] = os.environ.get("CXX", + cfg.get('CC', 'g++')) +cfg['JS_BIN'] = os.environ["JS"] def binpath(prog): - return os.path.join(cfg.sixgill_bin, prog) + return os.path.join(cfg['SIXGILL_BIN'], prog) -try: - os.mkdir(os.path.join('t', 'out')) -except OSError: - pass +if not os.path.exists("test-output"): + os.mkdir("test-output") -for name in cfg.tests: - name = os.path.basename(name) +# Simplified version of the body info. +class Body(dict): + def __init__(self, body): + self['BlockIdKind'] = body['BlockId']['Kind'] + if 'Variable' in body['BlockId']: + self['BlockName'] = body['BlockId']['Variable']['Name'][0] + self['LineRange'] = [ body['Location'][0]['Line'], body['Location'][1]['Line'] ] + self['Filename'] = body['Location'][0]['CacheString'] + self['Edges'] = body.get('PEdge', []) + self['Points'] = { i+1: body['PPoint'][i]['Location']['Line'] for i in range(len(body['PPoint'])) } + self['Index'] = body['Index'] + self['Variables'] = { x['Variable']['Name'][0]: x['Type'] for x in body['DefineVariable'] } + + # Indexes + self['Line2Points'] = {} + for point, line in self['Points'].items(): + self['Line2Points'].setdefault(line, []).append(point) + self['SrcPoint2Edges'] = {} + for edge in self['Edges']: + (src, dst) = edge['Index'] + self['SrcPoint2Edges'].setdefault(src, []).append(edge) + self['Line2Edges'] = {} + for (src, edges) in self['SrcPoint2Edges'].items(): + line = self['Points'][src] + self['Line2Edges'].setdefault(line, []).extend(edges) + + def edges_from_line(self, line): + return self['Line2Edges'][line] + + def edge_from_line(self, line): + edges = self.edges_from_line(line) + assert(len(edges) == 1) + return edges[0] + + def edges_from_point(self, point): + return self['SrcPoint2Edges'][point] + + def edge_from_point(self, point): + edges = self.edges_from_point(point) + assert(len(edges) == 1) + return edges[0] + + def assignment_point(self, varname): + for edge in self['Edges']: + if edge['Kind'] != 'Assign': + continue + dst = edge['Exp'][0] + if dst['Kind'] != 'Var': + continue + if dst['Variable']['Name'][0] == varname: + return edge['Index'][0] + raise Exception("assignment to variable %s not found" % varname) + + def assignment_line(self, varname): + return self['Points'][self.assignment_point(varname)] + +tests = ['test'] +for name in tests: indir = os.path.join(testdir, name) - outdir = os.path.join(testdir, 'out', name) - try: + outdir = os.path.join(testdir, "test-output", name) + if not os.path.exists(outdir): os.mkdir(outdir) - except OSError: - pass - test = Test(indir, outdir, cfg) + def compile(source): + cmd = "{CXX} -c {source} -fplugin={sixgill}".format(source=os.path.join(indir, source), + CXX=cfg['CXX'], sixgill=cfg['SIXGILL_PLUGIN']) + print("Running %s" % cmd) + subprocess.check_call(["sh", "-c", cmd]) + + def load_db_entry(dbname, pattern): + if not isinstance(pattern, basestring): + output = subprocess.check_output([binpath("xdbkeys"), dbname + ".xdb"]) + entries = output.splitlines() + matches = [f for f in entries if re.search(pattern, f)] + if len(matches) == 0: + raise Exception("entry not found") + if len(matches) > 1: + raise Exception("multiple entries found") + pattern = matches[0] + + output = subprocess.check_output([binpath("xdbfind"), "-json", dbname + ".xdb", pattern]) + return json.loads(output) + + def computeGCTypes(): + file("defaults.py", "w").write('''\ +analysis_scriptdir = '{testdir}' +sixgill_bin = '{bindir}' +'''.format(testdir=testdir, bindir=cfg['SIXGILL_BIN'])) + cmd = [ + os.path.join(testdir, "analyze.py"), + "gcTypes", "--upto", "gcTypes", + "--source=%s" % indir, + "--objdir=%s" % outdir, + "--js=%s" % cfg['JS_BIN'], + ] + print("Running " + " ".join(cmd)) + output = subprocess.check_call(cmd) + + def loadGCTypes(): + gctypes = {'GCThings': [], 'GCPointers': []} + for line in file(os.path.join(outdir, "gcTypes.txt")): + m = re.match(r'^(GC\w+): (.*)', line) + if m: + gctypes[m.group(1) + 's'].append(m.group(2)) + return gctypes + + def process_body(body): + return Body(body) + + def process_bodies(bodies): + return [ process_body(b) for b in bodies ] + + def equal(got, expected): + if got != expected: + print("Got '%s', expected '%s'" % (got, expected)) os.chdir(outdir) subprocess.call(["sh", "-c", "rm *.xdb"]) - execfile(os.path.join(indir, "test.py"), {'test': test, 'equal': equal}) + execfile(os.path.join(indir, "test.py")) print("TEST-PASSED: %s" % name) diff --git a/js/src/devtools/rootAnalysis/t/exceptions/source.cpp b/js/src/devtools/rootAnalysis/t/exceptions/source.cpp deleted file mode 100644 index 14169740e8f8..000000000000 --- a/js/src/devtools/rootAnalysis/t/exceptions/source.cpp +++ /dev/null @@ -1,42 +0,0 @@ -#define ANNOTATE(property) __attribute__((tag(property))) - -struct Cell { int f; } ANNOTATE("GC Thing"); - -extern void GC() ANNOTATE("GC Call"); - -void GC() -{ - // If the implementation is too trivial, the function body won't be emitted at all. - asm(""); -} - -class RAII_GC { - public: - RAII_GC() {} - ~RAII_GC() { GC(); } -}; - -// ~AutoSomething calls GC because of the RAII_GC field. The constructor, -// though, should *not* GC -- unless it throws an exception. Which is not -// possible when compiled with -fno-exceptions. -class AutoSomething { - RAII_GC gc; - public: - AutoSomething() : gc() { - asm(""); // Ooh, scary, this might throw an exception - } - ~AutoSomething() { - asm(""); - } -}; - -extern void usevar(Cell* cell); - -void f() { - Cell* thing = nullptr; // Live range starts here - - { - AutoSomething smth; // Constructor can GC only if exceptions are enabled - usevar(thing); // Live range ends here - } // In particular, 'thing' is dead at the destructor, so no hazard -} diff --git a/js/src/devtools/rootAnalysis/t/exceptions/test.py b/js/src/devtools/rootAnalysis/t/exceptions/test.py deleted file mode 100644 index f6d7f5e353f2..000000000000 --- a/js/src/devtools/rootAnalysis/t/exceptions/test.py +++ /dev/null @@ -1,19 +0,0 @@ -test.compile("source.cpp", '-fno-exceptions') -test.run_analysis_script('gcTypes') - -hazards = test.load_hazards() -assert(len(hazards) == 0) - -# If we compile with exceptions, then there *should* be a hazard because -# AutoSomething::AutoSomething might throw an exception, which would cause the -# partially-constructed value to be torn down, which will call ~RAII_GC. - -test.compile("source.cpp", '-fexceptions') -test.run_analysis_script('gcTypes') - -hazards = test.load_hazards() -assert(len(hazards) == 1) -hazard = hazards[0] -assert(hazard.function == 'void f()') -assert(hazard.variable == 'thing') -assert("AutoSomething::AutoSomething" in hazard.GCFunction) diff --git a/js/src/devtools/rootAnalysis/t/hazards/source.cpp b/js/src/devtools/rootAnalysis/t/hazards/source.cpp deleted file mode 100644 index 1e5191bc9445..000000000000 --- a/js/src/devtools/rootAnalysis/t/hazards/source.cpp +++ /dev/null @@ -1,89 +0,0 @@ -#define ANNOTATE(property) __attribute__((tag(property))) - -struct Cell { int f; } ANNOTATE("GC Thing"); - -class AutoSuppressGC_Base { - public: - AutoSuppressGC_Base() {} - ~AutoSuppressGC_Base() {} -} ANNOTATE("Suppress GC"); - -class AutoSuppressGC_Child : public AutoSuppressGC_Base { - public: - AutoSuppressGC_Child() : AutoSuppressGC_Base() {} -}; - -class AutoSuppressGC { - AutoSuppressGC_Child helpImBeingSuppressed; - - public: - AutoSuppressGC() {} -}; - -extern void GC() ANNOTATE("GC Call"); -extern void invisible(); - -void GC() -{ - // If the implementation is too trivial, the function body won't be emitted at all. - asm(""); - invisible(); -} - -extern void foo(Cell*); - -void suppressedFunction() { - GC(); // Calls GC, but is always called within AutoSuppressGC -} - -void halfSuppressedFunction() { - GC(); // Calls GC, but is sometimes called within AutoSuppressGC -} - -void unsuppressedFunction() { - GC(); // Calls GC, never within AutoSuppressGC -} - -volatile static int x = 3; -volatile static int* xp = &x; -struct GCInDestructor { - ~GCInDestructor() { - invisible(); - asm(""); - *xp = 4; - GC(); - } -}; - -Cell* -f() -{ - GCInDestructor kaboom; - - Cell cell; - Cell* cell1 = &cell; - Cell* cell2 = &cell; - Cell* cell3 = &cell; - Cell* cell4 = &cell; - { - AutoSuppressGC nogc; - suppressedFunction(); - halfSuppressedFunction(); - } - foo(cell1); - halfSuppressedFunction(); - foo(cell2); - unsuppressedFunction(); - { - // Old bug: it would look from the first AutoSuppressGC constructor it - // found to the last destructor. This statement *should* have no effect. - AutoSuppressGC nogc; - } - foo(cell3); - Cell* cell5 = &cell; - foo(cell5); - - // Hazard in return value due to ~GCInDestructor - Cell* cell6 = &cell; - return cell6; -} diff --git a/js/src/devtools/rootAnalysis/t/hazards/test.py b/js/src/devtools/rootAnalysis/t/hazards/test.py deleted file mode 100644 index 5e234d18061d..000000000000 --- a/js/src/devtools/rootAnalysis/t/hazards/test.py +++ /dev/null @@ -1,36 +0,0 @@ -test.compile("source.cpp") -test.run_analysis_script('gcTypes') - -# gcFunctions should be the inverse, but we get to rely on unmangled names here. -gcFunctions = test.load_gcFunctions() -print(gcFunctions) -assert('void GC()' in gcFunctions) -assert('void suppressedFunction()' not in gcFunctions) -assert('void halfSuppressedFunction()' in gcFunctions) -assert('void unsuppressedFunction()' in gcFunctions) -assert('Cell* f()' in gcFunctions) - -hazards = test.load_hazards() -hazmap = {haz.variable: haz for haz in hazards} -assert('cell1' not in hazmap) -assert('cell2' in hazmap) -assert('cell3' in hazmap) -assert('cell4' not in hazmap) -assert('cell5' not in hazmap) -assert('cell6' not in hazmap) -assert('' in hazmap) - -# All hazards should be in f() -assert(hazmap['cell2'].function == 'Cell* f()') -assert(len(set(haz.function for haz in hazards)) == 1) - -# Check that the correct GC call is reported for each hazard. (cell3 has a -# hazard from two different GC calls; it doesn't really matter which is -# reported.) -assert(hazmap['cell2'].GCFunction == 'void halfSuppressedFunction()') -assert(hazmap['cell3'].GCFunction in ('void halfSuppressedFunction()', 'void unsuppressedFunction()')) -assert(hazmap[''].GCFunction == 'void GCInDestructor::~GCInDestructor()') - -# Type names are handy to have in the report. -assert(hazmap['cell2'].type == 'Cell*') -assert(hazmap[''].type == 'Cell*') diff --git a/js/src/devtools/rootAnalysis/t/sixgill.py b/js/src/devtools/rootAnalysis/t/sixgill.py deleted file mode 100644 index 2bdf76a49b44..000000000000 --- a/js/src/devtools/rootAnalysis/t/sixgill.py +++ /dev/null @@ -1,63 +0,0 @@ -#!/usr/bin/env python -# 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 http://mozilla.org/MPL/2.0/. - -from collections import defaultdict - -# Simplified version of the body info. -class Body(dict): - def __init__(self, body): - self['BlockIdKind'] = body['BlockId']['Kind'] - if 'Variable' in body['BlockId']: - self['BlockName'] = body['BlockId']['Variable']['Name'][0].split("$")[-1] - loc = body['Location'] - self['LineRange'] = (loc[0]['Line'], loc[1]['Line']) - self['Filename'] = loc[0]['CacheString'] - self['Edges'] = body.get('PEdge', []) - self['Points'] = { i: p['Location']['Line'] for i, p in enumerate(body['PPoint'], 1) } - self['Index'] = body['Index'] - self['Variables'] = { x['Variable']['Name'][0].split("$")[-1]: x['Type'] for x in body['DefineVariable'] } - - # Indexes - self['Line2Points'] = defaultdict(list) - for point, line in self['Points'].items(): - self['Line2Points'][line].append(point) - self['SrcPoint2Edges'] = defaultdict(list) - for edge in self['Edges']: - src, dst = edge['Index'] - self['SrcPoint2Edges'][src].append(edge) - self['Line2Edges'] = defaultdict(list) - for (src, edges) in self['SrcPoint2Edges'].items(): - line = self['Points'][src] - self['Line2Edges'][line].extend(edges) - - def edges_from_line(self, line): - return self['Line2Edges'][line] - - def edge_from_line(self, line): - edges = self.edges_from_line(line) - assert(len(edges) == 1) - return edges[0] - - def edges_from_point(self, point): - return self['SrcPoint2Edges'][point] - - def edge_from_point(self, point): - edges = self.edges_from_point(point) - assert(len(edges) == 1) - return edges[0] - - def assignment_point(self, varname): - for edge in self['Edges']: - if edge['Kind'] != 'Assign': - continue - dst = edge['Exp'][0] - if dst['Kind'] != 'Var': - continue - if dst['Variable']['Name'][0] == varname: - return edge['Index'][0] - raise Exception("assignment to variable %s not found" % varname) - - def assignment_line(self, varname): - return self['Points'][self.assignment_point(varname)] diff --git a/js/src/devtools/rootAnalysis/t/suppression/source.cpp b/js/src/devtools/rootAnalysis/t/suppression/source.cpp deleted file mode 100644 index e7b41b4cb326..000000000000 --- a/js/src/devtools/rootAnalysis/t/suppression/source.cpp +++ /dev/null @@ -1,64 +0,0 @@ -#define ANNOTATE(property) __attribute__((tag(property))) - -struct Cell { int f; } ANNOTATE("GC Thing"); - -class AutoSuppressGC_Base { - public: - AutoSuppressGC_Base() {} - ~AutoSuppressGC_Base() {} -} ANNOTATE("Suppress GC"); - -class AutoSuppressGC_Child : public AutoSuppressGC_Base { - public: - AutoSuppressGC_Child() : AutoSuppressGC_Base() {} -}; - -class AutoSuppressGC { - AutoSuppressGC_Child helpImBeingSuppressed; - - public: - AutoSuppressGC() {} -}; - -extern void GC() ANNOTATE("GC Call"); - -void GC() -{ - // If the implementation is too trivial, the function body won't be emitted at all. - asm(""); -} - -extern void foo(Cell*); - -void suppressedFunction() { - GC(); // Calls GC, but is always called within AutoSuppressGC -} - -void halfSuppressedFunction() { - GC(); // Calls GC, but is sometimes called within AutoSuppressGC -} - -void unsuppressedFunction() { - GC(); // Calls GC, never within AutoSuppressGC -} - -void f() { - Cell* cell1 = nullptr; - Cell* cell2 = nullptr; - Cell* cell3 = nullptr; - { - AutoSuppressGC nogc; - suppressedFunction(); - halfSuppressedFunction(); - } - foo(cell1); - halfSuppressedFunction(); - foo(cell2); - unsuppressedFunction(); - { - // Old bug: it would look from the first AutoSuppressGC constructor it - // found to the last destructor. This statement *should* have no effect. - AutoSuppressGC nogc; - } - foo(cell3); -} diff --git a/js/src/devtools/rootAnalysis/t/suppression/test.py b/js/src/devtools/rootAnalysis/t/suppression/test.py deleted file mode 100644 index 65974cc3369a..000000000000 --- a/js/src/devtools/rootAnalysis/t/suppression/test.py +++ /dev/null @@ -1,23 +0,0 @@ -test.compile("source.cpp") -test.run_analysis_script('gcTypes', upto='gcFunctions') - -# The suppressions file uses only mangled names since it's for internal use, -# though I may change that soon given (1) the unfortunate non-uniqueness of -# mangled constructor names, and (2) the usefulness of this file for -# mrgiggles's reporting. -suppressed = test.load_suppressed_functions() - -# Only one of these is fully suppressed (ie, *always* called within the scope -# of an AutoSuppressGC). -assert(len(filter(lambda f: 'suppressedFunction' in f, suppressed)) == 1) -assert(len(filter(lambda f: 'halfSuppressedFunction' in f, suppressed)) == 0) -assert(len(filter(lambda f: 'unsuppressedFunction' in f, suppressed)) == 0) - -# gcFunctions should be the inverse, but we get to rely on unmangled names here. -gcFunctions = test.load_gcFunctions() -print(gcFunctions) -assert('void GC()' in gcFunctions) -assert('void suppressedFunction()' not in gcFunctions) -assert('void halfSuppressedFunction()' in gcFunctions) -assert('void unsuppressedFunction()' in gcFunctions) -assert('void f()' in gcFunctions) diff --git a/js/src/devtools/rootAnalysis/t/testlib.py b/js/src/devtools/rootAnalysis/t/testlib.py deleted file mode 100644 index 438398f1ed66..000000000000 --- a/js/src/devtools/rootAnalysis/t/testlib.py +++ /dev/null @@ -1,120 +0,0 @@ -import json -import os -import re -import subprocess - -from sixgill import Body -from collections import defaultdict, namedtuple - -scriptdir = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) - -HazardSummary = namedtuple('HazardSummary', ['function', 'variable', 'type', 'GCFunction', 'location']) - - -def equal(got, expected): - if got != expected: - print("Got '%s', expected '%s'" % (got, expected)) - -def extract_unmangled(func): - return func.split('$')[-1] - - -class Test(object): - def __init__(self, indir, outdir, cfg): - self.indir = indir - self.outdir = outdir - self.cfg = cfg - - def infile(self, path): - return os.path.join(self.indir, path) - - def binpath(self, prog): - return os.path.join(self.cfg.sixgill_bin, prog) - - def compile(self, source, options = ''): - cmd = "{CXX} -c {source} -O3 -std=c++11 -fplugin={sixgill} -fplugin-arg-xgill-mangle=1 {options}".format( - source=self.infile(source), - CXX=self.cfg.cxx, sixgill=self.cfg.sixgill_plugin, - options=options) - if self.cfg.verbose: - print("Running %s" % cmd) - subprocess.check_call(["sh", "-c", cmd]) - - def load_db_entry(self, dbname, pattern): - '''Look up an entry from an XDB database file, 'pattern' may be an exact - matching string, or an re pattern object matching a single entry.''' - - if not isinstance(pattern, basestring): - output = subprocess.check_output([self.binpath("xdbkeys"), dbname + ".xdb"]) - matches = filter(lambda _: re.search(pattern, _), output.splitlines()) - if len(matches) == 0: - raise Exception("entry not found") - if len(matches) > 1: - raise Exception("multiple entries found") - pattern = matches[0] - - output = subprocess.check_output([self.binpath("xdbfind"), "-json", dbname + ".xdb", pattern]) - return json.loads(output) - - def run_analysis_script(self, phase, upto=None): - file("defaults.py", "w").write('''\ -analysis_scriptdir = '{scriptdir}' -sixgill_bin = '{bindir}' -'''.format(scriptdir=scriptdir, bindir=self.cfg.sixgill_bin)) - cmd = [os.path.join(scriptdir, "analyze.py"), phase] - if upto: - cmd += ["--upto", upto] - cmd.append("--source=%s" % self.indir) - cmd.append("--objdir=%s" % self.outdir) - cmd.append("--js=%s" % self.cfg.js) - if self.cfg.verbose: - cmd.append("--verbose") - print("Running " + " ".join(cmd)) - subprocess.check_call(cmd) - - def computeGCTypes(self): - self.run_analysis_script("gcTypes", upto="gcTypes") - - def computeHazards(self): - self.run_analysis_script("gcTypes") - - def load_text_file(self, filename, extract=lambda l: l): - fullpath = os.path.join(self.outdir, filename) - values = (extract(line.strip()) for line in file(fullpath)) - return filter(lambda _: _ is not None, values) - - def load_suppressed_functions(self): - return set(self.load_text_file("suppressedFunctions.lst")) - - def load_gcTypes(self): - def grab_type(line): - m = re.match(r'^(GC\w+): (.*)', line) - if m: - return (m.group(1) + 's', m.group(2)) - return None - - gctypes = defaultdict(list) - for collection, typename in self.load_text_file('gcTypes.txt', extract=grab_type): - gctypes[collection].append(typename) - return gctypes - - def load_gcFunctions(self): - return self.load_text_file('gcFunctions.lst', extract=extract_unmangled) - - def load_hazards(self): - def grab_hazard(line): - m = re.match(r"Function '(.*?)' has unrooted '(.*?)' of type '(.*?)' live across GC call '(.*?)' at (.*)", line) - if m: - info = list(m.groups()) - info[0] = info[0].split("$")[-1] - info[3] = info[3].split("$")[-1] - return HazardSummary(*info) - return None - - return self.load_text_file('rootingHazards.txt', extract=grab_hazard) - - def process_body(self, body): - return Body(body) - - def process_bodies(self, bodies): - return [self.process_body(b) for b in bodies] diff --git a/js/src/devtools/rootAnalysis/t/sixgill-tree/source.cpp b/js/src/devtools/rootAnalysis/test/source.cpp similarity index 100% rename from js/src/devtools/rootAnalysis/t/sixgill-tree/source.cpp rename to js/src/devtools/rootAnalysis/test/source.cpp diff --git a/js/src/devtools/rootAnalysis/t/sixgill-tree/test.py b/js/src/devtools/rootAnalysis/test/test.py similarity index 82% rename from js/src/devtools/rootAnalysis/t/sixgill-tree/test.py rename to js/src/devtools/rootAnalysis/test/test.py index c0c0263cd024..59281570999d 100644 --- a/js/src/devtools/rootAnalysis/t/sixgill-tree/test.py +++ b/js/src/devtools/rootAnalysis/test/test.py @@ -1,8 +1,6 @@ -import re - -test.compile("source.cpp") -test.computeGCTypes() -body = test.process_body(test.load_db_entry("src_body", re.compile(r'root_arg'))[0]) +compile("source.cpp") +computeGCTypes() +body = process_body(load_db_entry("src_body", re.compile(r'root_arg'))[0]) # Rendering positive and negative integers marker1 = body.assignment_line('MARKER1') @@ -18,7 +16,7 @@ assert('other1' in body['Variables']) assert('other2' in body['Variables']) # Test function annotations -js_GC = test.process_body(test.load_db_entry("src_body", re.compile(r'js_GC'))[0]) +js_GC = process_body(load_db_entry("src_body", re.compile(r'js_GC'))[0]) annotations = js_GC['Variables']['void js_GC()']['Annotation'] assert(annotations) found_call_tag = False @@ -31,7 +29,7 @@ assert(found_call_tag) # Test type annotations # js::gc::Cell first -cell = test.load_db_entry("src_comp", 'js::gc::Cell')[0] +cell = load_db_entry("src_comp", 'js::gc::Cell')[0] assert(cell['Kind'] == 'Struct') annotations = cell['Annotation'] assert(len(annotations) == 1) @@ -40,14 +38,14 @@ assert(tag == 'Tag') assert(value == 'GC Thing') # Check JSObject inheritance. -JSObject = test.load_db_entry("src_comp", 'JSObject')[0] +JSObject = load_db_entry("src_comp", 'JSObject')[0] bases = [ b['Base'] for b in JSObject['CSUBaseClass'] ] assert('js::gc::Cell' in bases) assert('Bogon' in bases) assert(len(bases) == 2) # Check type analysis -gctypes = test.load_gcTypes() +gctypes = loadGCTypes() assert('js::gc::Cell' in gctypes['GCThings']) assert('JustACell' in gctypes['GCThings']) assert('JSObject' in gctypes['GCThings']) diff --git a/js/src/devtools/rootAnalysis/utility.js b/js/src/devtools/rootAnalysis/utility.js index 06c18804f362..7303c7b81fd9 100644 --- a/js/src/devtools/rootAnalysis/utility.js +++ b/js/src/devtools/rootAnalysis/utility.js @@ -6,15 +6,6 @@ // constructors/destructors. var internalMarker = " *INTERNAL* "; -if (! Set.prototype.hasOwnProperty("update")) { - Object.defineProperty(Set.prototype, "update", { - value: function (collection) { - for (let elt of collection) - this.add(elt); - } - }); -} - function assert(x, msg) { if (x) @@ -191,21 +182,3 @@ function* readFileLines_gen(filename) libc.fclose(fp); libc.free(ctypes.void_t.ptr(linebuf)); } - -function addToKeyedList(collection, key, entry) -{ - if (!(key in collection)) - collection[key] = []; - collection[key].push(entry); -} - -function loadTypeInfo(filename) -{ - var info = {}; - for (var line of readFileLines_gen(filename)) { - line = line.replace(/\n/, ""); - let [property, name] = line.split("$$"); - addToKeyedList(info, property, name); - } - return info; -} diff --git a/js/src/gc/Iteration.cpp b/js/src/gc/Iteration.cpp index 6c76f326954a..daa8c741a8ef 100644 --- a/js/src/gc/Iteration.cpp +++ b/js/src/gc/Iteration.cpp @@ -95,19 +95,21 @@ js::IterateScripts(JSRuntime* rt, JSCompartment* compartment, void* data, IterateScriptCallback scriptCallback) { MOZ_ASSERT(!rt->mainThread.suppressGC); - AutoEmptyNursery empty(rt); + rt->gc.evictNursery(); + MOZ_ASSERT(rt->gc.nursery.isEmpty()); + AutoPrepareForTracing prep(rt, SkipAtoms); if (compartment) { - Zone* zone = compartment->zone(); - for (auto script = zone->cellIter(empty); !script.done(); script.next()) { + for (ZoneCellIterUnderGC i(compartment->zone(), gc::AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); if (script->compartment() == compartment) scriptCallback(rt, data, script); } } else { for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) { - for (auto script = zone->cellIter(empty); !script.done(); script.next()) - scriptCallback(rt, data, script); + for (ZoneCellIterUnderGC i(zone, gc::AllocKind::SCRIPT); !i.done(); i.next()) + scriptCallback(rt, data, i.get()); } } } @@ -115,14 +117,14 @@ js::IterateScripts(JSRuntime* rt, JSCompartment* compartment, void js::IterateGrayObjects(Zone* zone, GCThingCallback cellCallback, void* data) { - JSRuntime* rt = zone->runtimeFromMainThread(); - AutoEmptyNursery empty(rt); - AutoPrepareForTracing prep(rt, SkipAtoms); + zone->runtimeFromMainThread()->gc.evictNursery(); + AutoPrepareForTracing prep(zone->runtimeFromMainThread(), SkipAtoms); for (auto thingKind : ObjectAllocKinds()) { - for (auto obj = zone->cellIter(thingKind, empty); !obj.done(); obj.next()) { + for (ZoneCellIterUnderGC i(zone, thingKind); !i.done(); i.next()) { + JSObject* obj = i.get(); if (obj->asTenured().isMarked(GRAY)) - cellCallback(data, JS::GCCellPtr(obj.get())); + cellCallback(data, JS::GCCellPtr(obj)); } } } diff --git a/js/src/gc/Statistics.cpp b/js/src/gc/Statistics.cpp index b5cc36eabef7..7dedbb8a8894 100644 --- a/js/src/gc/Statistics.cpp +++ b/js/src/gc/Statistics.cpp @@ -761,7 +761,7 @@ Statistics::Statistics(JSRuntime* rt) maxPauseInInterval(0), phaseNestingDepth(0), activeDagSlot(PHASE_DAG_NONE), - suspended(0), + suspendedPhaseNestingDepth(0), sliceCallback(nullptr), nurseryCollectionCallback(nullptr), aborted(false) @@ -1069,7 +1069,7 @@ Statistics::startTimingMutator() return false; } - MOZ_ASSERT(suspended == 0); + MOZ_ASSERT(suspendedPhaseNestingDepth == 0); timedGCTime = 0; phaseStartTimes[PHASE_MUTATOR] = 0; @@ -1094,37 +1094,6 @@ Statistics::stopTimingMutator(double& mutator_ms, double& gc_ms) return true; } -void -Statistics::suspendPhases(Phase suspension) -{ - MOZ_ASSERT(suspension == PHASE_EXPLICIT_SUSPENSION || suspension == PHASE_IMPLICIT_SUSPENSION); - while (phaseNestingDepth) { - MOZ_ASSERT(suspended < mozilla::ArrayLength(suspendedPhases)); - Phase parent = phaseNesting[phaseNestingDepth - 1]; - suspendedPhases[suspended++] = parent; - recordPhaseEnd(parent); - } - suspendedPhases[suspended++] = suspension; -} - -void -Statistics::resumePhases() -{ -#ifdef DEBUG - Phase popped = suspendedPhases[--suspended]; - MOZ_ASSERT(popped == PHASE_EXPLICIT_SUSPENSION || popped == PHASE_IMPLICIT_SUSPENSION); -#endif - while (suspended && - suspendedPhases[suspended - 1] != PHASE_EXPLICIT_SUSPENSION && - suspendedPhases[suspended - 1] != PHASE_IMPLICIT_SUSPENSION) - { - Phase resumePhase = suspendedPhases[--suspended]; - if (resumePhase == PHASE_MUTATOR) - timedGCTime += PRMJ_Now() - timedGCStart; - beginPhase(resumePhase); - } -} - void Statistics::beginPhase(Phase phase) { @@ -1136,7 +1105,9 @@ Statistics::beginPhase(Phase phase) // // Reuse this mechanism for managing PHASE_MUTATOR. if (parent == PHASE_GC_BEGIN || parent == PHASE_GC_END || parent == PHASE_MUTATOR) { - suspendPhases(PHASE_IMPLICIT_SUSPENSION); + MOZ_ASSERT(suspendedPhaseNestingDepth < mozilla::ArrayLength(suspendedPhases)); + suspendedPhases[suspendedPhaseNestingDepth++] = parent; + recordPhaseEnd(parent); parent = phaseNestingDepth ? phaseNesting[phaseNestingDepth - 1] : PHASE_NO_PARENT; } @@ -1183,8 +1154,12 @@ Statistics::endPhase(Phase phase) // When emptying the stack, we may need to resume a callback phase // (PHASE_GC_BEGIN/END) or return to timing the mutator (PHASE_MUTATOR). - if (phaseNestingDepth == 0 && suspended > 0 && suspendedPhases[suspended - 1] == PHASE_IMPLICIT_SUSPENSION) - resumePhases(); + if (phaseNestingDepth == 0 && suspendedPhaseNestingDepth > 0) { + Phase resumePhase = suspendedPhases[--suspendedPhaseNestingDepth]; + if (resumePhase == PHASE_MUTATOR) + timedGCTime += PRMJ_Now() - timedGCStart; + beginPhase(resumePhase); + } } void diff --git a/js/src/gc/Statistics.h b/js/src/gc/Statistics.h index 4de504e226e7..5fa4748a94c6 100644 --- a/js/src/gc/Statistics.h +++ b/js/src/gc/Statistics.h @@ -86,8 +86,6 @@ enum Phase : uint8_t { PHASE_LIMIT, PHASE_NONE = PHASE_LIMIT, - PHASE_EXPLICIT_SUSPENSION = PHASE_LIMIT, - PHASE_IMPLICIT_SUSPENSION, PHASE_MULTI_PARENTS }; @@ -171,22 +169,6 @@ struct Statistics void endPhase(Phase phase); void endParallelPhase(Phase phase, const GCParallelTask* task); - // Occasionally, we may be in the middle of something that is tracked by - // this class, and we need to do something unusual (eg evict the nursery) - // that doesn't normally nest within the current phase. Suspend the - // currently tracked phase stack, at which time the caller is free to do - // other tracked operations. - // - // This also happens internally with PHASE_GC_BEGIN and other "non-GC" - // phases. While in these phases, any beginPhase will automatically suspend - // the non-GC phase, until that inner stack is complete, at which time it - // will automatically resume the non-GC phase. Explicit suspensions do not - // get auto-resumed. - void suspendPhases(Phase suspension = PHASE_EXPLICIT_SUSPENSION); - - // Resume a suspended stack of phases. - void resumePhases(); - void beginSlice(const ZoneGCStats& zoneStats, JSGCInvocationKind gckind, SliceBudget budget, JS::gcreason::Reason reason); void endSlice(); @@ -336,14 +318,13 @@ struct Statistics size_t activeDagSlot; /* - * Certain phases can interrupt the phase stack, eg callback phases. When - * this happens, we move the suspended phases over to a sepearate list, - * terminated by a dummy PHASE_SUSPENSION phase (so that we can nest - * suspensions by suspending multiple stacks with a PHASE_SUSPENSION in - * between). + * To avoid recursive nesting, we discontinue a callback phase when any + * other phases are started. Remember what phase to resume when the inner + * phases are complete. (And because GCs can nest within the callbacks any + * number of times, we need a whole stack of of phases to resume.) */ - Phase suspendedPhases[MAX_NESTING * 3]; - size_t suspended; + Phase suspendedPhases[MAX_NESTING]; + size_t suspendedPhaseNestingDepth; /* Sweep times for SCCs of compartments. */ Vector sccTimes; diff --git a/js/src/gc/Zone.cpp b/js/src/gc/Zone.cpp index 19d80af1551d..1f4c6dee3b37 100644 --- a/js/src/gc/Zone.cpp +++ b/js/src/gc/Zone.cpp @@ -153,13 +153,13 @@ Zone::sweepBreakpoints(FreeOp* fop) */ MOZ_ASSERT(isGCSweepingOrCompacting()); - for (auto iter = cellIter(); !iter.done(); iter.next()) { - JSScript* script = iter; + for (ZoneCellIterUnderGC i(this, AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); if (!script->hasAnyBreakpointsOrStepMode()) continue; bool scriptGone = IsAboutToBeFinalizedUnbarriered(&script); - MOZ_ASSERT(script == iter); + MOZ_ASSERT(script == i.get()); for (unsigned i = 0; i < script->length(); i++) { BreakpointSite* site = script->getBreakpointSite(script->offsetToPC(i)); if (!site) @@ -207,8 +207,10 @@ Zone::discardJitCode(FreeOp* fop) #ifdef DEBUG /* Assert no baseline scripts are marked as active. */ - for (auto script = cellIter(); !script.done(); script.next()) + for (ZoneCellIter i(this, AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); MOZ_ASSERT_IF(script->hasBaselineScript(), !script->baselineScript()->active()); + } #endif /* Mark baseline scripts on the stack as active. */ @@ -217,7 +219,8 @@ Zone::discardJitCode(FreeOp* fop) /* Only mark OSI points if code is being discarded. */ jit::InvalidateAll(fop, this); - for (auto script = cellIter(); !script.done(); script.next()) { + for (ZoneCellIter i(this, AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); jit::FinishInvalidation(fop, script); /* diff --git a/js/src/gc/Zone.h b/js/src/gc/Zone.h index 17e53425be57..1d019e4a426a 100644 --- a/js/src/gc/Zone.h +++ b/js/src/gc/Zone.h @@ -84,9 +84,6 @@ using UniqueIdMap = GCHashMap -class ZoneCellIter; - } // namespace gc } // namespace js @@ -160,13 +157,6 @@ struct Zone : public JS::shadow::Zone, onTooMuchMalloc(); } - // Iterate over all cells in the zone. See the definition of ZoneCellIter - // in jsgcinlines.h for the possible arguments and documentation. - template - js::gc::ZoneCellIter cellIter(Args... args) { - return js::gc::ZoneCellIter(const_cast(this), mozilla::Forward(args)...); - } - bool isTooMuchMalloc() const { return gcMallocBytes <= 0; } void onTooMuchMalloc(); diff --git a/js/src/jit/BaselineJIT.cpp b/js/src/jit/BaselineJIT.cpp index 06529002441e..d649a5fb9ec7 100644 --- a/js/src/jit/BaselineJIT.cpp +++ b/js/src/jit/BaselineJIT.cpp @@ -488,7 +488,6 @@ BaselineScript::Trace(JSTracer* trc, BaselineScript* script) void BaselineScript::Destroy(FreeOp* fop, BaselineScript* script) { - MOZ_ASSERT(!script->hasPendingIonBuilder()); script->unlinkDependentWasmModules(fop); @@ -1172,7 +1171,8 @@ jit::ToggleBaselineProfiling(JSRuntime* runtime, bool enable) return; for (ZonesIter zone(runtime, SkipAtoms); !zone.done(); zone.next()) { - for (auto script = zone->cellIter(); !script.done(); script.next()) { + for (gc::ZoneCellIter i(zone, gc::AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); if (!script->hasBaselineScript()) continue; AutoWritableJitCode awjc(script->baselineScript()->method()); @@ -1186,7 +1186,8 @@ void jit::ToggleBaselineTraceLoggerScripts(JSRuntime* runtime, bool enable) { for (ZonesIter zone(runtime, SkipAtoms); !zone.done(); zone.next()) { - for (auto script = zone->cellIter(); !script.done(); script.next()) { + for (gc::ZoneCellIter i(zone, gc::AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); if (!script->hasBaselineScript()) continue; script->baselineScript()->toggleTraceLoggerScripts(runtime, script, enable); @@ -1198,7 +1199,8 @@ void jit::ToggleBaselineTraceLoggerEngine(JSRuntime* runtime, bool enable) { for (ZonesIter zone(runtime, SkipAtoms); !zone.done(); zone.next()) { - for (auto script = zone->cellIter(); !script.done(); script.next()) { + for (gc::ZoneCellIter i(zone, gc::AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); if (!script->hasBaselineScript()) continue; script->baselineScript()->toggleTraceLoggerEngine(enable); diff --git a/js/src/jit/Ion.cpp b/js/src/jit/Ion.cpp index 78d616f5e1db..ff012116a5f2 100644 --- a/js/src/jit/Ion.cpp +++ b/js/src/jit/Ion.cpp @@ -11,7 +11,6 @@ #include "mozilla/ThreadLocal.h" #include "jscompartment.h" -#include "jsgc.h" #include "jsprf.h" #include "gc/Marking.h" @@ -590,8 +589,8 @@ JitRuntime::Mark(JSTracer* trc, AutoLockForExclusiveAccess& lock) { MOZ_ASSERT(!trc->runtime()->isHeapMinorCollecting()); Zone* zone = trc->runtime()->atomsCompartment(lock)->zone(); - for (auto i = zone->cellIter(); !i.done(); i.next()) { - JitCode* code = i; + for (gc::ZoneCellIterUnderGC i(zone, gc::AllocKind::JITCODE); !i.done(); i.next()) { + JitCode* code = i.get(); TraceRoot(trc, &code, "wrapper"); } } @@ -1352,7 +1351,8 @@ jit::ToggleBarriers(JS::Zone* zone, bool needs) if (!rt->hasJitRuntime()) return; - for (auto script = zone->cellIter(); !script.done(); script.next()) { + for (gc::ZoneCellIterUnderGC i(zone, gc::AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); if (script->hasIonScript()) script->ionScript()->toggleBarriers(needs); if (script->hasBaselineScript()) diff --git a/js/src/jsapi-tests/testGCStoreBufferRemoval.cpp b/js/src/jsapi-tests/testGCStoreBufferRemoval.cpp index abf5bf00198f..c3bcb1d642c4 100644 --- a/js/src/jsapi-tests/testGCStoreBufferRemoval.cpp +++ b/js/src/jsapi-tests/testGCStoreBufferRemoval.cpp @@ -16,7 +16,7 @@ struct AutoIgnoreRootingHazards { static volatile int depth; AutoIgnoreRootingHazards() { depth++; } ~AutoIgnoreRootingHazards() { depth--; } -} JS_HAZ_GC_SUPPRESSED; +}; volatile int AutoIgnoreRootingHazards::depth = 0; BEGIN_TEST(testGCStoreBufferRemoval) diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index 537cfc4eb06c..be7d6213e26c 100644 --- a/js/src/jscompartment.cpp +++ b/js/src/jscompartment.cpp @@ -984,8 +984,8 @@ AddLazyFunctionsForCompartment(JSContext* cx, AutoObjectVector& lazyFunctions, A // want to delazify in that case: this pointer is weak so the JSScript // could be destroyed at the next GC. - for (auto i = cx->zone()->cellIter(kind); !i.done(); i.next()) { - JSFunction* fun = &i->as(); + for (gc::ZoneCellIter i(cx->zone(), kind); !i.done(); i.next()) { + JSFunction* fun = &i.get()->as(); // Sweeping is incremental; take care to not delazify functions that // are about to be finalized. GC things referenced by objects that are @@ -1157,7 +1157,8 @@ JSCompartment::clearScriptCounts() void JSCompartment::clearBreakpointsIn(FreeOp* fop, js::Debugger* dbg, HandleObject handler) { - for (auto script = zone()->cellIter(); !script.done(); script.next()) { + for (gc::ZoneCellIter i(zone(), gc::AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); if (script->compartment() == this && script->hasAnyBreakpointsOrStepMode()) script->clearBreakpointsIn(fop, dbg, handler); } diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index c37269f1b4a4..d884798c079d 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -2381,10 +2381,15 @@ GCRuntime::sweepTypesAfterCompacting(Zone* zone) AutoClearTypeInferenceStateOnOOM oom(zone); - for (auto script = zone->cellIter(); !script.done(); script.next()) + for (ZoneCellIterUnderGC i(zone, AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); script->maybeSweepTypes(&oom); - for (auto group = zone->cellIter(); !group.done(); group.next()) + } + + for (ZoneCellIterUnderGC i(zone, AllocKind::OBJECT_GROUP); !i.done(); i.next()) { + ObjectGroup* group = i.get(); group->maybeSweep(&oom); + } zone->types.endSweep(rt); } @@ -3976,11 +3981,10 @@ GCRuntime::checkForCompartmentMismatches() return; CompartmentCheckTracer trc(rt); - AutoAssertEmptyNursery empty(rt); for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) { trc.zone = zone; for (auto thingKind : AllAllocKinds()) { - for (auto i = zone->cellIter(thingKind, empty); !i.done(); i.next()) { + for (ZoneCellIterUnderGC i(zone, thingKind); !i.done(); i.next()) { trc.src = i.getCell(); trc.srcKind = MapAllocToTraceKind(thingKind); trc.compartment = DispatchTraceKindTyped(MaybeCompartmentFunctor(), @@ -3999,10 +4003,9 @@ RelazifyFunctions(Zone* zone, AllocKind kind) kind == AllocKind::FUNCTION_EXTENDED); JSRuntime* rt = zone->runtimeFromMainThread(); - AutoAssertEmptyNursery empty(rt); - for (auto i = zone->cellIter(kind, empty); !i.done(); i.next()) { - JSFunction* fun = &i->as(); + for (ZoneCellIterUnderGC i(zone, kind); !i.done(); i.next()) { + JSFunction* fun = &i.get()->as(); if (fun->hasScript()) fun->maybeRelazify(rt); } @@ -6916,7 +6919,8 @@ gc::MergeCompartments(JSCompartment* source, JSCompartment* target) RootedObject targetStaticGlobalLexicalScope(rt); targetStaticGlobalLexicalScope = &target->maybeGlobal()->lexicalScope().staticBlock(); - for (auto script = source->zone()->cellIter(); !script.done(); script.next()) { + for (ZoneCellIter iter(source->zone(), AllocKind::SCRIPT); !iter.done(); iter.next()) { + JSScript* script = iter.get(); MOZ_ASSERT(script->compartment() == source); script->compartment_ = target; script->setTypesGeneration(target->zone()->types.generation); @@ -6948,12 +6952,14 @@ gc::MergeCompartments(JSCompartment* source, JSCompartment* target) } } - for (auto base = source->zone()->cellIter(); !base.done(); base.next()) { + for (ZoneCellIter iter(source->zone(), AllocKind::BASE_SHAPE); !iter.done(); iter.next()) { + BaseShape* base = iter.get(); MOZ_ASSERT(base->compartment() == source); base->compartment_ = target; } - for (auto group = source->zone()->cellIter(); !group.done(); group.next()) { + for (ZoneCellIter iter(source->zone(), AllocKind::OBJECT_GROUP); !iter.done(); iter.next()) { + ObjectGroup* group = iter.get(); group->setGeneration(target->zone()->types.generation); group->compartment_ = target; @@ -6975,7 +6981,8 @@ gc::MergeCompartments(JSCompartment* source, JSCompartment* target) // After fixing JSFunctions' compartments, we can fix LazyScripts' // enclosing scopes. - for (auto lazy = source->zone()->cellIter(); !lazy.done(); lazy.next()) { + for (ZoneCellIter iter(source->zone(), AllocKind::LAZY_SCRIPT); !iter.done(); iter.next()) { + LazyScript* lazy = iter.get(); MOZ_ASSERT(lazy->functionNonDelazifying()->compartment() == target); // See warning in handleParseWorkload. If we start optimizing global @@ -7109,9 +7116,12 @@ js::ReleaseAllJITCode(FreeOp* fop) void js::PurgeJITCaches(Zone* zone) { - /* Discard Ion caches. */ - for (auto script = zone->cellIter(); !script.done(); script.next()) + for (ZoneCellIter i(zone, AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); + + /* Discard Ion caches. */ jit::PurgeCaches(script); + } } void @@ -7381,7 +7391,8 @@ js::gc::CheckHashTablesAfterMovingGC(JSRuntime* rt) for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) { zone->checkUniqueIdTableAfterMovingGC(); - for (auto baseShape = zone->cellIter(); !baseShape.done(); baseShape.next()) { + for (ZoneCellIterUnderGC i(zone, AllocKind::BASE_SHAPE); !i.done(); i.next()) { + BaseShape* baseShape = i.get(); if (baseShape->hasTable()) baseShape->table().checkAfterMovingGC(); } @@ -7859,28 +7870,5 @@ StateName(State state) return names[state]; } -void -AutoAssertHeapBusy::checkCondition(JSRuntime *rt) -{ - this->rt = rt; - MOZ_ASSERT(rt->isHeapBusy()); -} - -void -AutoAssertEmptyNursery::checkCondition(JSRuntime *rt) { - this->rt = rt; - MOZ_ASSERT(rt->gc.nursery.isEmpty()); -} - -AutoEmptyNursery::AutoEmptyNursery(JSRuntime *rt) - : AutoAssertEmptyNursery() -{ - MOZ_ASSERT(!rt->mainThread.suppressGC); - rt->gc.stats.suspendPhases(); - rt->gc.evictNursery(); - rt->gc.stats.resumePhases(); - checkCondition(rt); -} - } /* namespace gc */ } /* namespace js */ diff --git a/js/src/jsgc.h b/js/src/jsgc.h index 9ebc3c4ff249..409acd22dde6 100644 --- a/js/src/jsgc.h +++ b/js/src/jsgc.h @@ -1269,7 +1269,7 @@ MaybeVerifyBarriers(JSContext* cx, bool always = false) * read the comment in vm/Runtime.h above |suppressGC| and take all appropriate * precautions before instantiating this class. */ -class MOZ_RAII JS_HAZ_GC_SUPPRESSED AutoSuppressGC +class MOZ_RAII AutoSuppressGC { int32_t& suppressGC_; @@ -1328,95 +1328,6 @@ struct MOZ_RAII AutoAssertNoNurseryAlloc #endif }; -/* - * There are a couple of classes here that serve mostly as "tokens" indicating - * that a condition holds. Some functions force the caller to possess such a - * token because they would misbehave if the condition were false, and it is - * far more clear to make the condition visible at the point where it can be - * affected rather than just crashing in an assertion down in the place where - * it is relied upon. - */ - -/* - * Token meaning that the heap is busy and no allocations will be made. - * - * This class may be instantiated directly if it is known that the condition is - * already true, or it can be used as a base class for another RAII class that - * causes the condition to become true. Such base classes will use the no-arg - * constructor, establish the condition, then call checkCondition() to assert - * it and possibly record data needed to re-check the condition during - * destruction. - * - * Ordinarily, you would do something like this with a Maybe<> member that is - * emplaced during the constructor, but token-requiring functions want to - * require a reference to a base class instance. That said, you can always pass - * in the Maybe<> field as the token. - */ -class MOZ_RAII AutoAssertHeapBusy { - protected: - JSRuntime* rt; - - // Check that the heap really is busy, and record the rt for the check in - // the destructor. - void checkCondition(JSRuntime *rt); - - AutoAssertHeapBusy() : rt(nullptr) { - } - - public: - explicit AutoAssertHeapBusy(JSRuntime* rt) { - checkCondition(rt); - } - - ~AutoAssertHeapBusy() { - MOZ_ASSERT(rt); // checkCondition must always be called. - checkCondition(rt); - } -}; - -/* - * A class that serves as a token that the nursery is empty. It descends from - * AutoAssertHeapBusy, which means that it additionally requires the heap to be - * busy (which is not necessarily linked, but turns out to be true in practice - * for all users and simplifies the usage of these classes.) - */ -class MOZ_RAII AutoAssertEmptyNursery -{ - protected: - JSRuntime* rt; - - // Check that the nursery is empty. - void checkCondition(JSRuntime *rt); - - // For subclasses that need to empty the nursery in their constructors. - AutoAssertEmptyNursery() : rt(nullptr) { - } - - public: - explicit AutoAssertEmptyNursery(JSRuntime* rt) { - checkCondition(rt); - } - - ~AutoAssertEmptyNursery() { - checkCondition(rt); - } -}; - -/* - * Evict the nursery upon construction. Serves as a token indicating that the - * nursery is empty. (See AutoAssertEmptyNursery, above.) - * - * Note that this is very improper subclass of AutoAssertHeapBusy, in that the - * heap is *not* busy within the scope of an AutoEmptyNursery. I will most - * likely fix this by removing AutoAssertHeapBusy, but that is currently - * waiting on jonco's review. - */ -class MOZ_RAII AutoEmptyNursery : public AutoAssertEmptyNursery -{ - public: - explicit AutoEmptyNursery(JSRuntime *rt); -}; - const char* StateName(State state); diff --git a/js/src/jsgcinlines.h b/js/src/jsgcinlines.h index 6b710a85f5bf..e9687ea71cc9 100644 --- a/js/src/jsgcinlines.h +++ b/js/src/jsgcinlines.h @@ -195,71 +195,39 @@ class ArenaCellIterUnderFinalize : public ArenaCellIterImpl explicit ArenaCellIterUnderFinalize(Arena* arena) : ArenaCellIterImpl(arena) {} }; -template -class ZoneCellIter; - -template <> -class ZoneCellIter { +class ZoneCellIterImpl +{ ArenaIter arenaIter; ArenaCellIterImpl cellIter; - JS::AutoAssertNoAlloc noAlloc; - protected: - // For use when a subclass wants to insert some setup before init(). - ZoneCellIter() {} - - void init(JS::Zone* zone, AllocKind kind) { + public: + ZoneCellIterImpl(JS::Zone* zone, AllocKind kind) { JSRuntime* rt = zone->runtimeFromAnyThread(); MOZ_ASSERT(zone); MOZ_ASSERT_IF(IsNurseryAllocable(kind), rt->gc.nursery.isEmpty()); - // If called from outside a GC, ensure that the heap is in a state - // that allows us to iterate. - if (!rt->isHeapBusy()) { - // Assert that no GCs can occur while a ZoneCellIter is live. - noAlloc.disallowAlloc(rt); - } - // We have a single-threaded runtime, so there's no need to protect // against other threads iterating or allocating. However, we do have // background finalization; we may have to wait for this to finish if // it's currently active. if (IsBackgroundFinalized(kind) && zone->arenas.needBackgroundFinalizeWait(kind)) rt->gc.waitBackgroundSweepEnd(); + arenaIter.init(zone, kind); if (!arenaIter.done()) cellIter.init(arenaIter.get()); } - public: - ZoneCellIter(JS::Zone* zone, AllocKind kind) { - // If we are iterating a nursery-allocated kind then we need to - // evict first so that we can see all things. - if (IsNurseryAllocable(kind)) { - JSRuntime* rt = zone->runtimeFromMainThread(); - rt->gc.evictNursery(); - } - - init(zone, kind); - } - - ZoneCellIter(JS::Zone* zone, AllocKind kind, const js::gc::AutoAssertEmptyNursery&) { - // No need to evict the nursery. (This constructor is known statically - // to not GC.) - init(zone, kind); - } - bool done() const { return arenaIter.done(); } - template - T* get() const { + template T* get() const { MOZ_ASSERT(!done()); return cellIter.get(); } - TenuredCell* getCell() const { + Cell* getCell() const { MOZ_ASSERT(!done()); return cellIter.getCell(); } @@ -276,74 +244,44 @@ class ZoneCellIter { } }; -// Iterator over the cells in a Zone, where the GC type (JSString, JSObject) is -// known, for a single AllocKind. Example usages: -// -// for (auto obj = zone->cellIter(AllocKind::OBJECT0); !obj.done(); obj.next()) -// ... -// -// for (auto script = zone->cellIter(); !script.done(); script.next()) -// f(script->code()); -// -// As this code demonstrates, you can use 'script' as if it were a JSScript*. -// Its actual type is ZoneCellIter, but for most purposes it will -// autoconvert to JSScript*. -// -// Note that in the JSScript case, ZoneCellIter is able to infer the AllocKind -// from the type 'JSScript', whereas in the JSObject case, the kind must be -// given (because there are multiple AllocKinds for objects). -// -// Also, the static rooting hazard analysis knows that the JSScript case will -// not GC during construction. The JSObject case needs to GC, or more precisely -// to empty the nursery and clear out the store buffer, so that it can see all -// objects to iterate over (the nursery is not iterable) and remove the -// possibility of having pointers from the store buffer to data hanging off -// stuff we're iterating over that we are going to delete. (The latter should -// not be a problem, since such instances should be using RelocatablePtr do -// remove themselves from the store buffer on deletion, but currently for -// subtle reasons that isn't good enough.) -// -// If the iterator is used within a GC, then there is no need to evict the -// nursery (again). You may select a variant that will skip the eviction either -// by specializing on a GCType that is never allocated in the nursery, or -// explicitly by passing in a trailing AutoAssertEmptyNursery argument. -// -template -class ZoneCellIter : public ZoneCellIter { +class ZoneCellIterUnderGC : public ZoneCellIterImpl +{ public: - // Non-nursery allocated (equivalent to having an entry in - // MapTypeToFinalizeKind). The template declaration here is to discard this - // constructor overload if MapTypeToFinalizeKind::kind does not - // exist. Note that there will be no remaining overloads that will work, - // which makes sense given that you haven't specified which of the - // AllocKinds to use for GCType. - // - // If we later add a nursery allocable GCType with a single AllocKind, we - // will want to add an overload of this constructor that does the right - // thing (ie, it empties the nursery before iterating.) - explicit ZoneCellIter(JS::Zone* zone) : ZoneCellIter() { - init(zone, MapTypeToFinalizeKind::kind); - } - - // Non-nursery allocated, nursery is known to be empty: same behavior as above. - ZoneCellIter(JS::Zone* zone, const js::gc::AutoAssertEmptyNursery&) : ZoneCellIter(zone) { - } - - // Arbitrary kind, which will be assumed to be nursery allocable (and - // therefore the nursery will be emptied before iterating.) - ZoneCellIter(JS::Zone* zone, AllocKind kind) : ZoneCellIter(zone, kind) { - } - - // Arbitrary kind, which will be assumed to be nursery allocable, but the - // nursery is known to be empty already: same behavior as non-nursery types. - ZoneCellIter(JS::Zone* zone, AllocKind kind, const js::gc::AutoAssertEmptyNursery& empty) - : ZoneCellIter(zone, kind, empty) + ZoneCellIterUnderGC(JS::Zone* zone, AllocKind kind) + : ZoneCellIterImpl(zone, kind) { + MOZ_ASSERT(zone->runtimeFromAnyThread()->isHeapBusy()); + } +}; + +class ZoneCellIter +{ + mozilla::Maybe impl; + JS::AutoAssertNoAlloc noAlloc; + + public: + ZoneCellIter(JS::Zone* zone, AllocKind kind) { + // If called from outside a GC, ensure that the heap is in a state + // that allows us to iterate. + JSRuntime* rt = zone->runtimeFromMainThread(); + if (!rt->isHeapBusy()) { + // If we are iterating a nursery-allocated kind then we need to + // evict first so that we can see all things. + if (IsNurseryAllocable(kind)) + rt->gc.evictNursery(); + + // Assert that no GCs can occur while a ZoneCellIter is live. + noAlloc.disallowAlloc(rt); + } + + impl.emplace(zone, kind); } - GCType* get() const { return ZoneCellIter::get(); } - operator GCType*() const { return get(); } - GCType* operator ->() const { return get(); } + bool done() const { return impl->done(); } + template + T* get() const { return impl->get(); } + Cell* getCell() const { return impl->getCell(); } + void next() { impl->next(); } }; class GCZonesIter diff --git a/js/src/jsopcode.cpp b/js/src/jsopcode.cpp index aee66c49f59a..4514a3b73ca4 100644 --- a/js/src/jsopcode.cpp +++ b/js/src/jsopcode.cpp @@ -201,9 +201,8 @@ js::DumpPCCounts(JSContext* cx, HandleScript script, Sprinter* sp) void js::DumpCompartmentPCCounts(JSContext* cx) { - RootedScript script(cx); - for (auto iter = cx->zone()->cellIter(); !iter.done(); iter.next()) { - script = iter; + for (ZoneCellIter i(cx->zone(), gc::AllocKind::SCRIPT); !i.done(); i.next()) { + RootedScript script(cx, i.get()); if (script->compartment() != cx->compartment()) continue; @@ -1658,7 +1657,8 @@ js::StopPCCountProfiling(JSContext* cx) return; for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) { - for (auto script = zone->cellIter(); !script.done(); script.next()) { + for (ZoneCellIter i(zone, AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); if (script->hasScriptCounts() && script->types()) { if (!vec->append(script)) return; @@ -2025,7 +2025,8 @@ GenerateLcovInfo(JSContext* cx, JSCompartment* comp, GenericPrinter& out) } Rooted topScripts(cx, ScriptVector(cx)); for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) { - for (auto script = zone->cellIter(); !script.done(); script.next()) { + for (ZoneCellIter i(zone, AllocKind::SCRIPT); !i.done(); i.next()) { + JSScript* script = i.get(); if (script->compartment() != comp || !script->isTopLevel() || !script->filename()) diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 642e4e363bfd..a97e9ef89a89 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -2352,8 +2352,8 @@ UpdateExecutionObservabilityOfScriptsInZone(JSContext* cx, Zone* zone, return false; } } else { - for (auto iter = zone->cellIter(); !iter.done(); iter.next()) { - JSScript* script = iter; + for (gc::ZoneCellIter iter(zone, gc::AllocKind::SCRIPT); !iter.done(); iter.next()) { + JSScript* script = iter.get(); if (obs.shouldRecompileOrInvalidate(script) && !gc::IsAboutToBeFinalizedUnbarriered(&script)) { diff --git a/js/src/vm/HelperThreads.cpp b/js/src/vm/HelperThreads.cpp index c82c2ba82246..5a02869a35dd 100644 --- a/js/src/vm/HelperThreads.cpp +++ b/js/src/vm/HelperThreads.cpp @@ -1271,6 +1271,8 @@ GlobalHelperThreadState::mergeParseTaskCompartment(JSRuntime* rt, ParseTask* par LeaveParseTaskZone(rt, parseTask); { + gc::ZoneCellIter iter(parseTask->cx->zone(), gc::AllocKind::OBJECT_GROUP); + // Generator functions don't have Function.prototype as prototype but a // different function object, so the IdentifyStandardPrototype trick // below won't work. Just special-case it. @@ -1286,7 +1288,8 @@ GlobalHelperThreadState::mergeParseTaskCompartment(JSRuntime* rt, ParseTask* par // to the corresponding prototype in the new compartment. This will briefly // create cross compartment pointers, which will be fixed by the // MergeCompartments call below. - for (auto group = parseTask->cx->zone()->cellIter(); !group.done(); group.next()) { + for (; !iter.done(); iter.next()) { + ObjectGroup* group = iter.get(); TaggedProto proto(group->proto()); if (!proto.isObject()) continue; diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index 5d33a0224a8d..ffa472c6f538 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -1049,7 +1049,7 @@ CallAddPropertyHookDense(ExclusiveContext* cx, HandleNativeObject obj, uint32_t } static bool -UpdateShapeTypeAndValue(ExclusiveContext* cx, HandleNativeObject obj, HandleShape shape, const Value& value) +UpdateShapeTypeAndValue(ExclusiveContext* cx, NativeObject* obj, Shape* shape, const Value& value) { jsid id = shape->propid(); if (shape->hasSlot()) { diff --git a/js/src/vm/TypeInference.cpp b/js/src/vm/TypeInference.cpp index c78e363fc74c..fe6780b03f9f 100644 --- a/js/src/vm/TypeInference.cpp +++ b/js/src/vm/TypeInference.cpp @@ -2545,15 +2545,16 @@ js::PrintTypes(JSContext* cx, JSCompartment* comp, bool force) if (!force && !InferSpewActive(ISpewResult)) return; - RootedScript script(cx); - for (auto iter = zone->cellIter(); !iter.done(); iter.next()) { - script = iter; + for (gc::ZoneCellIter i(zone, gc::AllocKind::SCRIPT); !i.done(); i.next()) { + RootedScript script(cx, i.get()); if (script->types()) script->types()->printTypes(cx, script); } - for (auto group = zone->cellIter(); !group.done(); group.next()) + for (gc::ZoneCellIter i(zone, gc::AllocKind::OBJECT_GROUP); !i.done(); i.next()) { + ObjectGroup* group = i.get(); group->print(); + } #endif } @@ -4424,8 +4425,10 @@ TypeZone::endSweep(JSRuntime* rt) void TypeZone::clearAllNewScriptsOnOOM() { - for (auto iter = zone()->cellIter(); !iter.done(); iter.next()) { - ObjectGroup* group = iter; + for (gc::ZoneCellIter iter(zone(), gc::AllocKind::OBJECT_GROUP); + !iter.done(); iter.next()) + { + ObjectGroup* group = iter.get(); if (!IsAboutToBeFinalizedUnbarriered(&group)) group->maybeClearNewScriptOnOOM(); } @@ -4434,9 +4437,8 @@ TypeZone::clearAllNewScriptsOnOOM() AutoClearTypeInferenceStateOnOOM::~AutoClearTypeInferenceStateOnOOM() { if (oom) { - JSRuntime* rt = zone->runtimeFromMainThread(); zone->setPreservingCode(false); - zone->discardJitCode(rt->defaultFreeOp()); + zone->discardJitCode(zone->runtimeFromMainThread()->defaultFreeOp()); zone->types.clearAllNewScriptsOnOOM(); } }