From 5e48e30b98fa6961b19e9ab40f6670ea8b96cba5 Mon Sep 17 00:00:00 2001 From: Marian-Vasile Laza Date: Thu, 20 Jan 2022 02:39:17 +0200 Subject: [PATCH] Backed out 10 changesets (bug 1746090) for causing build bustages on TestingFunctions.cpp. CLOSED TREE Backed out changeset edbf96722e4b (bug 1746090) Backed out changeset f4e4bf6ba8ff (bug 1746090) Backed out changeset c288fe1c6c84 (bug 1746090) Backed out changeset 2b0caa13d0fc (bug 1746090) Backed out changeset 1ed9d77885c6 (bug 1746090) Backed out changeset 54a60388fb11 (bug 1746090) Backed out changeset a9c16e721533 (bug 1746090) Backed out changeset 774bdb9939a9 (bug 1746090) Backed out changeset 5c5742535301 (bug 1746090) Backed out changeset ff509fe4671d (bug 1746090) --- devtools/shared/heapsnapshot/HeapSnapshot.cpp | 58 +- js/examples/jorendb.js | 8 +- js/public/GCAPI.h | 19 +- js/public/UbiNode.h | 33 +- js/src/builtin/Object.cpp | 2 +- js/src/builtin/TestingFunctions.cpp | 9 +- js/src/debugger/Debugger.cpp | 8 +- js/src/debugger/DebuggerMemory.cpp | 8 +- js/src/devtools/rootAnalysis/CFG.js | 675 ----------- js/src/devtools/rootAnalysis/analyze.py | 15 +- js/src/devtools/rootAnalysis/analyzeRoots.js | 1008 +++++++++++------ js/src/devtools/rootAnalysis/annotations.js | 8 +- .../devtools/rootAnalysis/computeCallgraph.js | 70 +- .../rootAnalysis/computeGCFunctions.js | 87 +- .../devtools/rootAnalysis/computeGCTypes.js | 10 +- js/src/devtools/rootAnalysis/loadCallgraph.js | 7 +- js/src/devtools/rootAnalysis/mergeJSON.js | 18 - js/src/devtools/rootAnalysis/run-test.py | 4 - .../rootAnalysis/t/hazards/source.cpp | 43 - .../devtools/rootAnalysis/t/hazards/test.py | 20 +- js/src/devtools/rootAnalysis/utility.js | 135 +-- js/src/vm/UbiNode.cpp | 42 +- js/src/vm/UbiNodeShortestPaths.cpp | 11 +- 23 files changed, 836 insertions(+), 1462 deletions(-) delete mode 100644 js/src/devtools/rootAnalysis/mergeJSON.js diff --git a/devtools/shared/heapsnapshot/HeapSnapshot.cpp b/devtools/shared/heapsnapshot/HeapSnapshot.cpp index 206d2f95bac8..c0881b726463 100644 --- a/devtools/shared/heapsnapshot/HeapSnapshot.cpp +++ b/devtools/shared/heapsnapshot/HeapSnapshot.cpp @@ -695,13 +695,10 @@ static bool AddGlobalsAsRoots(HandleObjectVector globals, // If `boundaries` is incoherent, or we encounter an error while trying to // handle it, or we run out of memory, set `rv` appropriately and return // `false`. -// -// Return value is a pair of the status and an AutoCheckCannotGC token, -// forwarded from ubi::RootList::init(), to ensure that the caller does -// not GC while the RootList is live and initialized. -static std::pair EstablishBoundaries( - JSContext* cx, ErrorResult& rv, const HeapSnapshotBoundaries& boundaries, - ubi::RootList& roots, CompartmentSet& compartments) { +static bool EstablishBoundaries(JSContext* cx, ErrorResult& rv, + const HeapSnapshotBoundaries& boundaries, + ubi::RootList& roots, + CompartmentSet& compartments) { MOZ_ASSERT(!roots.initialized()); MOZ_ASSERT(compartments.empty()); @@ -712,49 +709,48 @@ static std::pair EstablishBoundaries( if (!boundaries.mRuntime.Value()) { rv.Throw(NS_ERROR_INVALID_ARG); - return {false, AutoCheckCannotGC(cx)}; + return false; } - auto [ok, nogc] = roots.init(); - if (!ok) { + if (!roots.init()) { rv.Throw(NS_ERROR_OUT_OF_MEMORY); - return {false, nogc}; + return false; } } if (boundaries.mDebugger.WasPassed()) { if (foundBoundaryProperty) { rv.Throw(NS_ERROR_INVALID_ARG); - return {false, AutoCheckCannotGC(cx)}; + return false; } foundBoundaryProperty = true; JSObject* dbgObj = boundaries.mDebugger.Value(); if (!dbgObj || !dbg::IsDebugger(*dbgObj)) { rv.Throw(NS_ERROR_INVALID_ARG); - return {false, AutoCheckCannotGC(cx)}; + return false; } RootedObjectVector globals(cx); if (!dbg::GetDebuggeeGlobals(cx, *dbgObj, &globals) || !PopulateCompartmentsWithGlobals(compartments, globals) || - !roots.init(compartments).first || !AddGlobalsAsRoots(globals, roots)) { + !roots.init(compartments) || !AddGlobalsAsRoots(globals, roots)) { rv.Throw(NS_ERROR_OUT_OF_MEMORY); - return {false, AutoCheckCannotGC(cx)}; + return false; } } if (boundaries.mGlobals.WasPassed()) { if (foundBoundaryProperty) { rv.Throw(NS_ERROR_INVALID_ARG); - return {false, AutoCheckCannotGC(cx)}; + return false; } foundBoundaryProperty = true; uint32_t length = boundaries.mGlobals.Value().Length(); if (length == 0) { rv.Throw(NS_ERROR_INVALID_ARG); - return {false, AutoCheckCannotGC(cx)}; + return false; } RootedObjectVector globals(cx); @@ -762,29 +758,28 @@ static std::pair EstablishBoundaries( JSObject* global = boundaries.mGlobals.Value().ElementAt(i); if (!JS_IsGlobalObject(global)) { rv.Throw(NS_ERROR_INVALID_ARG); - return {false, AutoCheckCannotGC(cx)}; + return false; } if (!globals.append(global)) { rv.Throw(NS_ERROR_OUT_OF_MEMORY); - return {false, AutoCheckCannotGC(cx)}; + return false; } } if (!PopulateCompartmentsWithGlobals(compartments, globals) || - !roots.init(compartments).first || !AddGlobalsAsRoots(globals, roots)) { + !roots.init(compartments) || !AddGlobalsAsRoots(globals, roots)) { rv.Throw(NS_ERROR_OUT_OF_MEMORY); - return {false, AutoCheckCannotGC(cx)}; + return false; } } - AutoCheckCannotGC nogc(cx); if (!foundBoundaryProperty) { rv.Throw(NS_ERROR_INVALID_ARG); - return {false, nogc}; + return false; } MOZ_ASSERT(roots.initialized()); - return {true, nogc}; + return true; } // A variant covering all the various two-byte strings that we can get from the @@ -1478,16 +1473,15 @@ void ChromeUtils::SaveHeapSnapshotShared( JSContext* cx = global.Context(); { - ubi::RootList rootList(cx, wantNames); - auto [ok, nogc] = - EstablishBoundaries(cx, rv, boundaries, rootList, compartments); - if (!ok) { + Maybe maybeNoGC; + ubi::RootList rootList(cx, maybeNoGC, wantNames); + if (!EstablishBoundaries(cx, rv, boundaries, rootList, compartments)) return; - } StreamWriter writer(cx, gzipStream, wantNames, !compartments.empty() ? &compartments : nullptr); + MOZ_ASSERT(maybeNoGC.isSome()); ubi::Node roots(&rootList); // Serialize the initial heap snapshot metadata to the core dump. @@ -1495,10 +1489,10 @@ void ChromeUtils::SaveHeapSnapshotShared( // Serialize the heap graph to the core dump, starting from our list of // roots. !WriteHeapGraph(cx, roots, writer, wantNames, - !compartments.empty() ? &compartments : nullptr, nogc, - nodeCount, edgeCount)) { + !compartments.empty() ? &compartments : nullptr, + maybeNoGC.ref(), nodeCount, edgeCount)) { rv.Throw(zeroCopyStream.failed() ? zeroCopyStream.result() - : NS_ERROR_UNEXPECTED); + : NS_ERROR_UNEXPECTED); return; } } diff --git a/js/examples/jorendb.js b/js/examples/jorendb.js index 7ab6393f1738..a920c5880d34 100644 --- a/js/examples/jorendb.js +++ b/js/examples/jorendb.js @@ -59,10 +59,7 @@ wrap(this, 'putstr'); // Convert a debuggee value v to a string. function dvToString(v) { - if (typeof(v) === 'object' && v !== null) { - return `[object ${v.class}]`; - } - return uneval(v); + return (typeof v !== 'object' || v === null) ? uneval(v) : "[object " + v.class + "]"; } function summaryObject(dv) { @@ -79,7 +76,7 @@ function summaryObject(dv) { function debuggeeValueToString(dv, style) { var dvrepr = dvToString(dv); - if (!style.pretty || (typeof dv !== 'object') || (dv === null)) + if (!style.pretty || (typeof dv !== 'object')) return [dvrepr, undefined]; const exec = debuggeeGlobalWrapper.executeInGlobalWithBindings.bind(debuggeeGlobalWrapper); @@ -877,7 +874,6 @@ while (rerun) { } catch (exc) { print("Caught exception " + exc); print(exc.stack); - break; } } else if (task.action == 'repl') { repl(); diff --git a/js/public/GCAPI.h b/js/public/GCAPI.h index 9126901d0429..8e5221fe77c6 100644 --- a/js/public/GCAPI.h +++ b/js/public/GCAPI.h @@ -989,7 +989,6 @@ class JS_PUBLIC_API AutoRequireNoGC { */ class JS_PUBLIC_API AutoAssertNoGC : public AutoRequireNoGC { #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED - protected: JSContext* cx_; public: @@ -1064,19 +1063,13 @@ class JS_PUBLIC_API AutoAssertGCCallback : public AutoSuppressGCAnalysis { class JS_PUBLIC_API AutoCheckCannotGC : public AutoAssertNoGC { public: explicit AutoCheckCannotGC(JSContext* cx = nullptr) : AutoAssertNoGC(cx) {} -# ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED - AutoCheckCannotGC(const AutoCheckCannotGC& other) - : AutoCheckCannotGC(other.cx_) {} -# else - AutoCheckCannotGC(const AutoCheckCannotGC& other) : AutoCheckCannotGC() {} -# endif -#else -class JS_PUBLIC_API AutoCheckCannotGC : public AutoRequireNoGC{ - public : - explicit AutoCheckCannotGC(JSContext* cx = nullptr){} AutoCheckCannotGC( - const AutoCheckCannotGC& other) : AutoCheckCannotGC(){} -#endif } JS_HAZ_GC_INVALIDATED; +#else +class JS_PUBLIC_API AutoCheckCannotGC : public AutoRequireNoGC { + public: + explicit AutoCheckCannotGC(JSContext* cx = nullptr) {} +} JS_HAZ_GC_INVALIDATED; +#endif extern JS_PUBLIC_API void SetLowMemoryState(JSContext* cx, bool newState); diff --git a/js/public/UbiNode.h b/js/public/UbiNode.h index 51faeb526972..54f565d2a170 100644 --- a/js/public/UbiNode.h +++ b/js/public/UbiNode.h @@ -967,51 +967,50 @@ class PreComputedEdgeRange : public EdgeRange { // // RootList::init itself causes a minor collection, but once the list of roots // has been created, GC must not occur, as the referent ubi::Nodes are not -// stable across GC. It returns a [[nodiscard]] AutoCheckCannotGC token in order -// to enforce this. The token's lifetime must extend at least as long as the -// RootList itself. Note that the RootList does not itself contain a nogc field, -// which means that it is possible to store it somewhere that it can escape -// the init()'s nogc scope. Don't do that. (Or you could call some function -// and pass in the RootList and GC, but that would be caught.) +// stable across GC. The init calls emplace on |noGC|'s AutoCheckCannotGC, whose +// lifetime must extend at least as long as the RootList itself. // // Example usage: // // { -// JS::ubi::RootList rootList(cx); -// auto [ok, nogc] = rootList.init(); -// if (!ok()) { +// mozilla::Maybe maybeNoGC; +// JS::ubi::RootList rootList(cx, maybeNoGC); +// if (!rootList.init()) { // return false; // } // +// // The AutoCheckCannotGC is guaranteed to exist if init returned true. +// MOZ_ASSERT(maybeNoGC.isSome()); +// // JS::ubi::Node root(&rootList); // // ... // } class MOZ_STACK_CLASS JS_PUBLIC_API RootList { + Maybe& noGC; + public: JSContext* cx; EdgeVector edges; bool wantNames; - bool inited; - explicit RootList(JSContext* cx, bool wantNames = false); + RootList(JSContext* cx, Maybe& noGC, + bool wantNames = false); // Find all GC roots. - [[nodiscard]] std::pair init(); + [[nodiscard]] bool init(); // Find only GC roots in the provided set of |JS::Compartment|s. Note: it's // important to take a CompartmentSet and not a RealmSet: objects in // same-compartment realms can reference each other directly, without going // through CCWs, so if we used a RealmSet here we would miss edges. - [[nodiscard]] std::pair init( - CompartmentSet& debuggees); + [[nodiscard]] bool init(CompartmentSet& debuggees); // Find only GC roots in the given Debugger object's set of debuggee // compartments. - [[nodiscard]] std::pair init( - HandleObject debuggees); + [[nodiscard]] bool init(HandleObject debuggees); // Returns true if the RootList has been initialized successfully, false // otherwise. - bool initialized() { return inited; } + bool initialized() { return noGC.isSome(); } // Explicitly add the given Node as a root in this RootList. If wantNames is // true, you must pass an edgeName. The RootList does not take ownership of diff --git a/js/src/builtin/Object.cpp b/js/src/builtin/Object.cpp index bc951abc6ce6..41987e0d48ff 100644 --- a/js/src/builtin/Object.cpp +++ b/js/src/builtin/Object.cpp @@ -488,7 +488,7 @@ JSString* js::ObjectToSource(JSContext* cx, HandleObject obj) { val.set(desc->value()); - JSFunction* fun = nullptr; + JSFunction* fun; if (IsFunctionObject(val, &fun) && fun->isMethod()) { if (!AddProperty(id, val, PropertyKind::Method)) { return nullptr; diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp index dc3ab82d2b8c..55aafacde381 100644 --- a/js/src/builtin/TestingFunctions.cpp +++ b/js/src/builtin/TestingFunctions.cpp @@ -5712,20 +5712,21 @@ static bool ShortestPaths(JSContext* cx, unsigned argc, Value* vp) { Vector>> names(cx); { + mozilla::Maybe maybeNoGC; JS::ubi::Node root; - JS::ubi::RootList rootList(cx, true); + JS::ubi::RootList rootList(cx, maybeNoGC, true); if (start.isNull()) { - auto [ok, nogc] = rootList.init(); - if (!ok) { + if (!rootList.init()) { ReportOutOfMemory(cx); return false; } root = JS::ubi::Node(&rootList); } else { + maybeNoGC.emplace(cx); root = JS::ubi::Node(start); } - JS::AutoCheckCannotGC noGC(cx); + JS::AutoCheckCannotGC& noGC = maybeNoGC.ref(); JS::ubi::NodeSet targets; diff --git a/js/src/debugger/Debugger.cpp b/js/src/debugger/Debugger.cpp index 2a83ca4c6db5..89524c7d62b0 100644 --- a/js/src/debugger/Debugger.cpp +++ b/js/src/debugger/Debugger.cpp @@ -5779,15 +5779,15 @@ class MOZ_STACK_CLASS Debugger::ObjectQuery { { // We can't tolerate the GC moving things around while we're // searching the heap. Check that nothing we do causes a GC. + Maybe maybeNoGC; RootedObject dbgObj(cx, dbg->object); - JS::ubi::RootList rootList(cx); - auto [ok, nogc] = rootList.init(dbgObj); - if (!ok) { + JS::ubi::RootList rootList(cx, maybeNoGC); + if (!rootList.init(dbgObj)) { ReportOutOfMemory(cx); return false; } - Traversal traversal(cx, *this, nogc); + Traversal traversal(cx, *this, maybeNoGC.ref()); traversal.wantNames = false; return traversal.addStart(JS::ubi::Node(&rootList)) && diff --git a/js/src/debugger/DebuggerMemory.cpp b/js/src/debugger/DebuggerMemory.cpp index 617331252782..f2c9d0e1168a 100644 --- a/js/src/debugger/DebuggerMemory.cpp +++ b/js/src/debugger/DebuggerMemory.cpp @@ -413,14 +413,14 @@ bool DebuggerMemory::CallData::takeCensus() { } { - JS::ubi::RootList rootList(cx); - auto [ok, nogc] = rootList.init(dbgObj); - if (!ok) { + Maybe maybeNoGC; + JS::ubi::RootList rootList(cx, maybeNoGC); + if (!rootList.init(dbgObj)) { ReportOutOfMemory(cx); return false; } - JS::ubi::CensusTraversal traversal(cx, handler, nogc); + JS::ubi::CensusTraversal traversal(cx, handler, maybeNoGC.ref()); traversal.wantNames = false; if (!traversal.addStart(JS::ubi::Node(&rootList)) || diff --git a/js/src/devtools/rootAnalysis/CFG.js b/js/src/devtools/rootAnalysis/CFG.js index f8e71fc81824..c1beafe08227 100644 --- a/js/src/devtools/rootAnalysis/CFG.js +++ b/js/src/devtools/rootAnalysis/CFG.js @@ -8,8 +8,6 @@ "use strict"; -var TRACING = false; - // Find all points (positions within the code) of the body given by the list of // bodies and the blockId to match (which will specify an outer function or a // loop within it), recursing into loops if needed. @@ -37,154 +35,6 @@ function findAllPoints(bodies, blockId, bits) return points; } -// Visitor of a graph of vertexes and sixgill-generated edges, -// where the edges represent the actual computation happening. -// -// Uses the syntax `var Visitor = class { ... }` rather than `class Visitor` -// to allow reloading this file with the JS debugger. -var Visitor = class { - constructor(bodies) { - this.visited_bodies = new Map(); - for (const body of bodies) { - this.visited_bodies.set(body, new Map()); - } - } - - // Returns whether we should keep going after seeing this - // pair. Also records it as visited. - visit(body, ppoint, info) { - const visited = this.visited_bodies.get(body); - const existing = visited.get(ppoint); - const action = this.next_action(existing, info); - const merged = this.merge_info(existing, info); - visited.set(ppoint, merged); - return [action, merged]; - } - - // Default implementation does a basic "only visit nodes once" search. - // (Whether this is BFS/DFS/other is determined by the caller.) - - // Override if you need to revisit nodes. Valid actions are "continue", - // "prune", and "done". "continue" means continue with the search. "prune" - // means stop at this node, only continue on other edges. "done" means the - // whole search is complete even if unvisited nodes remain. - next_action(prev, current) { return prev ? "prune" : "continue"; } - - // Update the info at a node. If this is the first time the node has been - // seen, `prev` will be undefined. `current` will be the info computed by - // `extend_path`. The node will be updated with the return value. - merge_info(prev, current) { return true; } - - // Prepend `edge` to the info stored at the successor node, returning - // the updated info value. This should be overridden by pretty much any - // subclass, as a traversal's semantics are largely determined by this method. - extend_path(edge, body, ppoint, successor_path) { return true; } -}; - -function findMatchingBlock(bodies, blockId) { - for (const body of bodies) { - if (sameBlockId(body.BlockId, blockId)) { - return body; - } - } - assert(false); -} - -// Perform a mostly breadth-first search through the graph of . -// This is only mostly breadth-first because the visitor decides whether to -// stop searching when it sees an already-visited node. It can choose to -// re-visit a node in order to find "better" paths that include a node more -// than once. -// -// The return value depends on how the search finishes. If a 'done' action -// is returned by visitor.visit(), use the information returned by -// that call. If the search completes without reaching the entry point of -// the function (the "root"), return null. If the search manages to reach -// the root, return the value of the `result_if_reached_root` parameter. -// -// This allows this function to be used in different ways. If the visitor -// associates a value with each node that chains onto its successors -// (or predecessors in the "upwards" search order), then this will return -// a complete path through the graph. But this can also be used to test -// whether a condition holds (eg "the exit point is reachable after -// calling SomethingImportant()"), in which case no path is needed and the -// visitor will cause the return value to be a simple boolean (or null -// if it terminates the search before reaching the root.) -// -// The information returned by the visitor for a node is often called -// `path` in the code below, even though it may not represent a path. -// -function BFS_upwards(start_body, start_ppoint, bodies, visitor, - initial_successor_info={}, - result_if_reached_root=null) -{ - const work = [[start_body, start_ppoint, null, initial_successor_info]]; - if (TRACING) { - printErr(`BFS start at ${blockIdentifier(start_body)}:${start_ppoint}`); - } - - let reached_root = false; - while (work.length > 0) { - const [body, ppoint, edgeToAdd, successor_path] = work.shift(); - if (TRACING) { - printErr(`prepending edge from ${ppoint} to state '${successor_path}'`); - } - let path = visitor.extend_path(edgeToAdd, body, ppoint, successor_path); - - const [action, merged_path] = visitor.visit(body, ppoint, path); - if (action === "done") { - return merged_path; - } - if (action === "prune") { - // Do not push anything else to the work queue, but continue processing - // other branches. - continue; - } - assert(action == "continue"); - path = merged_path; - - const predecessors = getPredecessors(body); - for (const edge of (predecessors[ppoint] || [])) { - if (edge.Kind == "Loop") { - // Propagate the search into the exit point of the loop body. - const loopBody = findMatchingBlock(bodies, edge.BlockId); - const loopEnd = loopBody.Index[1]; - work.push([loopBody, loopEnd, null, path]); - // Don't continue to predecessors here without going through - // the loop. (The points in this body that enter the loop will - // be traversed when we reach the entry point of the loop.) - } else { - work.push([body, edge.Index[0], edge, path]); - } - } - - // Check for hitting the entry point of a loop body. - if (ppoint == body.Index[0] && body.BlockId.Kind == "Loop") { - // Propagate to outer body parents that enter the loop body. - for (const parent of (body.BlockPPoint || [])) { - const parentBody = findMatchingBlock(bodies, parent.BlockId); - work.push([parentBody, parent.Index, null, path]); - } - - // This point is also preceded by the *end* of this loop, for the - // previous iteration. - work.push([body, body.Index[1], null, path]); - } - - // Check for reaching the root of the function. - if (body === start_body && ppoint == body.Index[0]) { - reached_root = true; - } - } - - // The search space was exhausted without finding a 'done' state. That - // might be because all search paths were pruned before reaching the entry - // point of the function, in which case reached_root will be false. (If - // reached_root is true, then we may still not have visited the entire - // graph, if some paths were pruned but at least one made it to the root.) - return reached_root ? result_if_reached_root : null; -} - // Given the CFG for the constructor call of some RAII, return whether the // given edge is the matching destructor call. function isMatchingDestructor(constructor, edge) @@ -328,42 +178,6 @@ function pointsInRAIIScope(bodies, body, constructorEdge, bits) { return points; } -function isImmobileValue(exp) { - if (exp.Kind == "Int" && exp.String == "0") { - return true; - } - return false; -} - -function expressionIsVariableAddress(exp, variable) -{ - while (exp.Kind == "Fld") - exp = exp.Exp[0]; - return exp.Kind == "Var" && sameVariable(exp.Variable, variable); -} - -function edgeTakesVariableAddress(edge, variable, body) -{ - if (ignoreEdgeUse(edge, variable, body)) - return false; - if (ignoreEdgeAddressTaken(edge)) - return false; - switch (edge.Kind) { - case "Assign": - return expressionIsVariableAddress(edge.Exp[1], variable); - case "Call": - if ("PEdgeCallArguments" in edge) { - for (var exp of edge.PEdgeCallArguments.Exp) { - if (expressionIsVariableAddress(exp, variable)) - return true; - } - } - return false; - default: - return false; - } -} - // Look at an invocation of a virtual method or function pointer contained in a // field, and return the static type of the invocant (or the containing struct, // for a function pointer field.) @@ -402,495 +216,6 @@ function getFieldCallInstanceCSU(edge, field) } } -function expressionUsesVariable(exp, variable) -{ - if (exp.Kind == "Var" && sameVariable(exp.Variable, variable)) - return true; - if (!("Exp" in exp)) - return false; - for (var childExp of exp.Exp) { - if (expressionUsesVariable(childExp, variable)) - return true; - } - return false; -} - -function expressionUsesVariableContents(exp, variable) -{ - if (!("Exp" in exp)) - return false; - for (var childExp of exp.Exp) { - if (childExp.Kind == 'Drf') { - if (expressionUsesVariable(childExp, variable)) - return true; - } else if (expressionUsesVariableContents(childExp, variable)) { - return true; - } - } - return false; -} - -// Detect simple |return nullptr;| statements. -function isReturningImmobileValue(edge, variable) -{ - if (variable.Kind == "Return") { - if (edge.Exp[0].Kind == "Var" && sameVariable(edge.Exp[0].Variable, variable)) { - if (isImmobileValue(edge.Exp[1])) - return true; - } - } - return false; -} - -// If the edge uses the given variable's value, return the earliest point at -// which the use is definite. Usually, that means the source of the edge -// (anything that reaches that source point will end up using the variable, but -// there may be other ways to reach the destination of the edge.) -// -// Return values are implicitly used at the very last point in the function. -// This makes a difference: if an RAII class GCs in its destructor, we need to -// start looking at the final point in the function, not one point back from -// that, since that would skip over the GCing call. -// -// Note that this returns true only if the variable's incoming value is used. -// So this would return false for 'obj': -// -// obj = someFunction(); -// -// but these would return true: -// -// obj = someFunction(obj); -// obj->foo = someFunction(); -// -function edgeUsesVariable(edge, variable, body) -{ - if (ignoreEdgeUse(edge, variable, body)) - return 0; - - if (variable.Kind == "Return" && body.Index[1] == edge.Index[1] && body.BlockId.Kind == "Function") { - // The last point in the function body is treated as using the return - // value. This is the only time the destination point is returned - // rather than the source point. - return edge.Index[1]; - } - - var src = edge.Index[0]; - - switch (edge.Kind) { - - case "Assign": { - // Detect `Return := nullptr`. - if (isReturningImmobileValue(edge, variable)) - return 0; - const [lhs, rhs] = edge.Exp; - // Detect `lhs := ...variable...` - if (expressionUsesVariable(rhs, variable)) - return src; - // Detect `...variable... := rhs` but not `variable := rhs`. The latter - // overwrites the previous value of `variable` without using it. - if (expressionUsesVariable(lhs, variable) && !expressionIsVariable(lhs, variable)) - return src; - return 0; - } - - case "Assume": - return expressionUsesVariableContents(edge.Exp[0], variable) ? src : 0; - - case "Call": { - const callee = edge.Exp[0]; - if (expressionUsesVariable(callee, variable)) - return src; - if ("PEdgeCallInstance" in edge) { - if (expressionUsesVariable(edge.PEdgeCallInstance.Exp, variable)) { - if (edgeStartsValueLiveRange(edge, variable)) { - // If the variable is being constructed, then the incoming - // value is not used here; it didn't exist before - // construction. (The analysis doesn't get told where - // variables are defined, so must infer it from - // construction. If the variable does not have a - // constructor, its live range may be larger than it really - // ought to be if it is defined within a loop body, but - // that is conservative.) - } else { - return src; - } - } - } - if ("PEdgeCallArguments" in edge) { - for (var exp of edge.PEdgeCallArguments.Exp) { - if (expressionUsesVariable(exp, variable)) - return src; - } - } - if (edge.Exp.length == 1) - return 0; - - // Assigning call result to a variable. - const lhs = edge.Exp[1]; - if (expressionUsesVariable(lhs, variable) && !expressionIsVariable(lhs, variable)) - return src; - return 0; - } - - case "Loop": - return 0; - - case "Assembly": - return 0; - - default: - assert(false); - } -} - -function expressionIsVariable(exp, variable) -{ - return exp.Kind == "Var" && sameVariable(exp.Variable, variable); -} - -function expressionIsMethodOnVariable(exp, variable) -{ - // This might be calling a method on a base class, in which case exp will - // be an unnamed field of the variable instead of the variable itself. - while (exp.Kind == "Fld" && exp.Field.Name[0].startsWith("field:")) - exp = exp.Exp[0]; - - return exp.Kind == "Var" && sameVariable(exp.Variable, variable); -} - -// Return whether the edge starts the live range of a variable's value, by setting -// it to some new value. Examples of starting obj's live range: -// -// obj = foo; -// obj = foo(); -// obj = foo(obj); // uses previous value but then sets to new value -// SomeClass obj(true, 1); // constructor -// -function edgeStartsValueLiveRange(edge, variable) -{ - // Direct assignments start live range of lhs: var = value - if (edge.Kind == "Assign") { - const [lhs, rhs] = edge.Exp; - return (expressionIsVariable(lhs, variable) && - !isReturningImmobileValue(edge, variable)); - } - - if (edge.Kind != "Call") - return false; - - // Assignments of call results start live range: var = foo() - if (1 in edge.Exp) { - var lhs = edge.Exp[1]; - if (expressionIsVariable(lhs, variable)) - return true; - } - - // Constructor calls start live range of instance: SomeClass var(...) - if ("PEdgeCallInstance" in edge) { - var instance = edge.PEdgeCallInstance.Exp; - - // Kludge around incorrect dereference on some constructor calls. - if (instance.Kind == "Drf") - instance = instance.Exp[0]; - - if (!expressionIsVariable(instance, variable)) - return false; - - var callee = edge.Exp[0]; - if (callee.Kind != "Var") - return false; - - assert(callee.Variable.Kind == "Func"); - var calleeName = readable(callee.Variable.Name[0]); - - // Constructor calls include the text 'Name::Name(' or 'Name<...>::Name('. - var openParen = calleeName.indexOf('('); - if (openParen < 0) - return false; - calleeName = calleeName.substring(0, openParen); - - var lastColon = calleeName.lastIndexOf('::'); - if (lastColon < 0) - return false; - var constructorName = calleeName.substr(lastColon + 2); - calleeName = calleeName.substr(0, lastColon); - - var lastTemplateOpen = calleeName.lastIndexOf('<'); - if (lastTemplateOpen >= 0) - calleeName = calleeName.substr(0, lastTemplateOpen); - - if (calleeName.endsWith(constructorName)) - return true; - } - - return false; -} - -// Return whether an edge "clears out" a variable's value. A simple example -// would be -// -// var = nullptr; -// -// for analyses for which nullptr is a "safe" value (eg GC rooting hazards; you -// can't get in trouble by holding a nullptr live across a GC.) A more complex -// example is a Maybe that gets reset: -// -// Maybe nogc; -// nogc.emplace(cx); -// nogc.reset(); -// gc(); // <-- not a problem; nogc is invalidated by prev line -// nogc.emplace(cx); -// foo(nogc); -// -// Yet another example is a UniquePtr being passed by value, which means the -// receiver takes ownership: -// -// UniquePtr uobj(obj); -// foo(uobj); -// gc(); -// -function edgeEndsValueLiveRange(edge, variable, body) -{ - // var = nullptr; - if (edge.Kind == "Assign") { - const [lhs, rhs] = edge.Exp; - return expressionIsVariable(lhs, variable) && isImmobileValue(rhs); - } - - if (edge.Kind != "Call") - return false; - - var callee = edge.Exp[0]; - - if (edge.Type.Kind == 'Function' && - edge.Exp[0].Kind == 'Var' && - edge.Exp[0].Variable.Kind == 'Func' && - edge.Exp[0].Variable.Name[1] == 'MarkVariableAsGCSafe' && - edge.Exp[0].Variable.Name[0].includes("JS::detail::MarkVariableAsGCSafe") && - expressionIsVariable(edge.PEdgeCallArguments.Exp[0], variable)) - { - // explicit JS_HAZ_VARIABLE_IS_GC_SAFE annotation - return true; - } - - if (edge.Type.Kind == 'Function' && - edge.Exp[0].Kind == 'Var' && - edge.Exp[0].Variable.Kind == 'Func' && - edge.Exp[0].Variable.Name[1] == 'move' && - edge.Exp[0].Variable.Name[0].includes('std::move(') && - expressionIsVariable(edge.PEdgeCallArguments.Exp[0], variable) && - edge.Exp[1].Kind == 'Var' && - edge.Exp[1].Variable.Kind == 'Temp') - { - // temp = std::move(var) - // - // If var is a UniquePtr, and we pass it into something that takes - // ownership, then it should be considered to be invalid. Example: - // - // consume(std::move(var)); - // - // where consume takes a UniquePtr. This will compile to something like - // - // UniquePtr* __temp_1 = &std::move(var); - // UniquePtr&& __temp_2(*temp_1); // move constructor - // consume(__temp_2); - // ~UniquePtr(__temp_2); - // - // The line commented with "// move constructor" is a result of passing - // a UniquePtr as a parameter. If consume() took a UniquePtr&& - // directly, this would just be: - // - // UniquePtr* __temp_1 = &std::move(var); - // consume(__temp_1); - // - // which is not guaranteed to move from the reference. It might just - // ignore the parameter. We can't predict what consume(UniquePtr&&) - // will do. We do know that UniquePtr(UniquePtr&& other) moves out of - // `other`. - // - // The std::move() technically is irrelevant, but because we only care - // about bare variables, it has to be used, which is fortunate because - // the UniquePtr&& constructor operates on a temporary, not the - // variable we care about. - - const lhs = edge.Exp[1].Variable; - if (basicBlockEatsVariable(lhs, body, edge.Index[1])) - return true; - } - - if (edge.Type.Kind == 'Function' && - edge.Type.TypeFunctionCSU && - edge.PEdgeCallInstance && - expressionIsMethodOnVariable(edge.PEdgeCallInstance.Exp, variable)) - { - const typeName = edge.Type.TypeFunctionCSU.Type.Name; - const m = typeName.match(/^(((\w|::)+?)(\w+)). - const ctorName = `${namespace}${classname}::${classname}()`; - if (callee.Kind == 'Var' && - typesWithSafeConstructors.has(type) && - callee.Variable.Name[0].includes(ctorName)) - { - return true; - } - - // special-case: UniquePtr::reset() and similar. - if (callee.Kind == 'Var' && - type in resetterMethods && - resetterMethods[type].has(callee.Variable.Name[1])) - { - return true; - } - } - } - - // special-case: passing UniquePtr by value. - if (edge.Type.Kind == 'Function' && - edge.Type.TypeFunctionArgument && - edge.PEdgeCallArguments) - { - for (const i in edge.Type.TypeFunctionArgument) { - const param = edge.Type.TypeFunctionArgument[i]; - if (param.Type.Kind != 'CSU') - continue; - if (!param.Type.Name.startsWith("mozilla::UniquePtr<")) - continue; - const arg = edge.PEdgeCallArguments.Exp[i]; - if (expressionIsVariable(arg, variable)) { - return true; - } - } - } - - return false; -} - -function edgeMovesVariable(edge, variable) -{ - if (edge.Kind != 'Call') - return false; - const callee = edge.Exp[0]; - if (callee.Kind == 'Var' && - callee.Variable.Kind == 'Func') - { - const { Variable: { Name: [ fullname, shortname ] } } = callee; - const [ mangled, unmangled ] = splitFunction(fullname); - // Match a UniquePtr move constructor. - if (unmangled.match(/::UniquePtr<[^>]*>::UniquePtr\((\w+::)*UniquePtr<[^>]*>&&/)) - return true; - } - - return false; -} - -// Scan forward through the basic block in 'body' starting at 'startpoint', -// looking for a call that passes 'variable' to a move constructor that -// "consumes" it (eg UniquePtr::UniquePtr(UniquePtr&&)). -function basicBlockEatsVariable(variable, body, startpoint) -{ - const successors = getSuccessors(body); - let point = startpoint; - while (point in successors) { - // Only handle a single basic block. If it forks, stop looking. - const edges = successors[point]; - if (edges.length != 1) { - return false; - } - const edge = edges[0]; - - if (edgeMovesVariable(edge, variable)) { - return true; - } - - // edgeStartsValueLiveRange will find places where 'variable' is given - // a new value. Never observed in practice, since this function is only - // called with a temporary resulting from std::move(), which is used - // immediately for a call. But just to be robust to future uses: - if (edgeStartsValueLiveRange(edge, variable)) { - return false; - } - - point = edge.Index[1]; - } - - return false; -} - -function edgeIsNonReleasingDtor(body, edge, calleeName, functionBodies) { - if (edge.Kind !== "Call") { - return false; - } - if (!isRefcountedDtor(calleeName)) { - return false; - } - - let callee = edge.Exp[0]; - while (callee.Kind === "Drf") { - callee = callee.Exp[0]; - } - - const instance = edge.PEdgeCallInstance.Exp; - if (instance.Kind !== "Var") { - // TODO: handle field destructors - return false; - } - - // Test whether the dtor call is dominated by operations on the variable - // that mean it will not go to a zero refcount in the dtor: either because - // it's already dead (eg r.forget() was called) or because it can be proven - // to have a ref count of greater than 1. This is implemented by looking - // for the reverse: find a path scanning backwards from the dtor call where - // the variable is used in any way that does *not* ensure that it is - // trivially destructible. - - const variable = instance.Variable; - - const visitor = new class extends Visitor { - // Do not revisit nodes. For new nodes, relay the decision made by - // extend_path. - next_action(seen, current) { return seen ? "prune" : current; } - - // We don't revisit, so always use the new. - merge_info(seen, current) { return current; } - - // Return the action to take from this node. - extend_path(edge, body, ppoint, successor_path) { - if (!edge) { - // Dummy edge to join two points. - return "continue"; - } - - if (!edgeUsesVariable(edge, variable, body)) { - // Nothing of interest on this edge, keep searching. - return "continue"; - } - - if (edgeEndsValueLiveRange(edge, variable, body)) { - // This path is safe! - return "prune"; - } - - // Unsafe. Found a use that might set the variable to a - // nonzero refcount. - return "done"; - } - }(functionBodies); - - // Searching upwards from a destructor call, return the opposite of: is - // there a path to a use or the start of the function that does NOT hit a - // safe assignment like refptr.forget() first? - // - // In graph terms: return whether the destructor call is dominated by forget() calls (or similar). - return !BFS_upwards( - body, edge.Index[0], functionBodies, visitor, "start", - true // Return value if we reach the root without finding a non-forget() use. - ); -} - // gcc uses something like "__dt_del " for virtual destructors that it // generates. function isSyntheticVirtualDestructor(funcName) { diff --git a/js/src/devtools/rootAnalysis/analyze.py b/js/src/devtools/rootAnalysis/analyze.py index 896ab022a63c..30e727e42c71 100755 --- a/js/src/devtools/rootAnalysis/analyze.py +++ b/js/src/devtools/rootAnalysis/analyze.py @@ -120,21 +120,11 @@ JOBS = { "{analysis_scriptdir}/computeCallgraph.js", "{typeInfo}", Output("rawcalls"), - Output("rawEdges"), "{i}", "{n}", ], "multi-output": True, - "outputs": ["rawcalls.{i}.of.{n}", "gcEdges.{i}.of.{n}"], - }, - "mergeJSON": { - "command": [ - "{js}", - "{analysis_scriptdir}/mergeJSON.js", - MultiInput("{rawEdges}"), - Output("gcEdges"), - ], - "outputs": ["gcEdges.json"], + "outputs": ["rawcalls.{i}.of.{n}"], }, "gcFunctions": { "command": [ @@ -145,12 +135,14 @@ JOBS = { Output("callgraph"), Output("gcFunctions"), Output("gcFunctions_list"), + Output("gcEdges"), Output("limitedFunctions_list"), ], "outputs": [ "callgraph.txt", "gcFunctions.txt", "gcFunctions.lst", + "gcEdges.txt", "limitedFunctions.lst", ], }, @@ -440,7 +432,6 @@ steps = [ "gcTypes", "rawcalls", "gcFunctions", - "mergeJSON", "allFunctions", "hazards", "gather-hazards", diff --git a/js/src/devtools/rootAnalysis/analyzeRoots.js b/js/src/devtools/rootAnalysis/analyzeRoots.js index b66a66cec353..8565f1b84384 100644 --- a/js/src/devtools/rootAnalysis/analyzeRoots.js +++ b/js/src/devtools/rootAnalysis/analyzeRoots.js @@ -16,75 +16,51 @@ var sourceRoot = (os.getenv('SOURCE') || '') + '/'; var functionName; var functionBodies; -try { - var options = parse_options([ - { - name: "--function", - type: 'string', - }, - { - name: "-f", - type: "string", - dest: "function", - }, - { - name: "gcFunctions", - default: "gcFunctions.lst" - }, - { - name: "gcEdges", - default: "gcEdges.json" - }, - { - name: "limitedFunctions", - default: "limitedFunctions.lst" - }, - { - name: "gcTypes", - default: "gcTypes.txt" - }, - { - name: "typeInfo", - default: "typeInfo.txt" - }, - { - name: "batch", - type: "number", - default: 1 - }, - { - name: "numBatches", - type: "number", - default: 1 - }, - { - name: "tmpfile", - default: "tmp.txt" - }, - ]); -} catch (e) { - printErr(e); - printErr("Usage: analyzeRoots.js [-f function_name] [start end [tmpfile]]"); - quit(1); +if (typeof scriptArgs[0] != 'string' || typeof scriptArgs[1] != 'string') + throw "Usage: analyzeRoots.js [-f function_name] [start end [tmpfile]]"; + +var theFunctionNameToFind; +if (scriptArgs[0] == '--function' || scriptArgs[0] == '-f') { + theFunctionNameToFind = scriptArgs[1]; + scriptArgs = scriptArgs.slice(2); } + +var gcFunctionsFile = scriptArgs[0] || "gcFunctions.lst"; +var gcEdgesFile = scriptArgs[1] || "gcEdges.txt"; +var limitedFunctionsFile = scriptArgs[2] || "limitedFunctions.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"; + var gcFunctions = {}; -var text = snarf(options.gcFunctions).split("\n"); +var text = snarf("gcFunctions.lst").split("\n"); assert(text.pop().length == 0); for (const line of text) gcFunctions[mangled(line)] = readable(line); -var limitedFunctions = JSON.parse(snarf(options.limitedFunctions)); +var limitedFunctions = JSON.parse(snarf(limitedFunctionsFile)); text = null; -var typeInfo = loadTypeInfo(options.typeInfo); +var typeInfo = loadTypeInfo(typeInfoFile); -var gcEdges = JSON.parse(os.file.readFile(options.gcEdges)); +var gcEdges = {}; +text = snarf(gcEdgesFile).split('\n'); +assert(text.pop().length == 0); +for (const line of text) { + var [ block, edge, func ] = line.split(" || "); + if (!(block in gcEdges)) + gcEdges[block] = {} + gcEdges[block][edge] = func; +} +text = null; var match; var gcThings = {}; var gcPointers = {}; -text = snarf(options.gcTypes).split("\n"); +text = snarf(gcTypesFile).split("\n"); for (var line of text) { if (match = /^GCThing: (.*)/.exec(line)) gcThings[match[1]] = true; @@ -119,6 +95,425 @@ function isUnrootedType(type) return false; } +function expressionUsesVariable(exp, variable) +{ + if (exp.Kind == "Var" && sameVariable(exp.Variable, variable)) + return true; + if (!("Exp" in exp)) + return false; + for (var childExp of exp.Exp) { + if (expressionUsesVariable(childExp, variable)) + return true; + } + return false; +} + +function expressionUsesVariableContents(exp, variable) +{ + if (!("Exp" in exp)) + return false; + for (var childExp of exp.Exp) { + if (childExp.Kind == 'Drf') { + if (expressionUsesVariable(childExp, variable)) + return true; + } else if (expressionUsesVariableContents(childExp, variable)) { + return true; + } + } + return false; +} + +function isImmobileValue(exp) { + if (exp.Kind == "Int" && exp.String == "0") { + return true; + } + return false; +} + +// Detect simple |return nullptr;| statements. +function isReturningImmobileValue(edge, variable) +{ + if (variable.Kind == "Return") { + if (edge.Exp[0].Kind == "Var" && sameVariable(edge.Exp[0].Variable, variable)) { + if (isImmobileValue(edge.Exp[1])) + return true; + } + } + return false; +} + +// If the edge uses the given variable's value, return the earliest point at +// which the use is definite. Usually, that means the source of the edge +// (anything that reaches that source point will end up using the variable, but +// there may be other ways to reach the destination of the edge.) +// +// Return values are implicitly used at the very last point in the function. +// This makes a difference: if an RAII class GCs in its destructor, we need to +// start looking at the final point in the function, not one point back from +// that, since that would skip over the GCing call. +// +// Note that this returns true only if the variable's incoming value is used. +// So this would return false for 'obj': +// +// obj = someFunction(); +// +// but these would return true: +// +// obj = someFunction(obj); +// obj->foo = someFunction(); +// +function edgeUsesVariable(edge, variable, body) +{ + if (ignoreEdgeUse(edge, variable, body)) + return 0; + + if (variable.Kind == "Return" && body.Index[1] == edge.Index[1] && body.BlockId.Kind == "Function") + return edge.Index[1]; // Last point in function body uses the return value. + + var src = edge.Index[0]; + + switch (edge.Kind) { + + case "Assign": { + // Detect `Return := nullptr`. + if (isReturningImmobileValue(edge, variable)) + return 0; + const [lhs, rhs] = edge.Exp; + // Detect `lhs := ...variable...` + if (expressionUsesVariable(rhs, variable)) + return src; + // Detect `...variable... := rhs` but not `variable := rhs`. The latter + // overwrites the previous value of `variable` without using it. + if (expressionUsesVariable(lhs, variable) && !expressionIsVariable(lhs, variable)) + return src; + return 0; + } + + case "Assume": + return expressionUsesVariableContents(edge.Exp[0], variable) ? src : 0; + + case "Call": { + const callee = edge.Exp[0]; + if (expressionUsesVariable(callee, variable)) + return src; + if ("PEdgeCallInstance" in edge) { + if (expressionUsesVariable(edge.PEdgeCallInstance.Exp, variable)) { + if (edgeStartsValueLiveRange(edge, variable)) { + // If the variable is being constructed, then the incoming + // value is not used here; it didn't exist before + // construction. (The analysis doesn't get told where + // variables are defined, so must infer it from + // construction. If the variable does not have a + // constructor, its live range may be larger than it really + // ought to be if it is defined within a loop body, but + // that is conservative.) + } else { + return src; + } + } + } + if ("PEdgeCallArguments" in edge) { + for (var exp of edge.PEdgeCallArguments.Exp) { + if (expressionUsesVariable(exp, variable)) + return src; + } + } + if (edge.Exp.length == 1) + return 0; + + // Assigning call result to a variable. + const lhs = edge.Exp[1]; + if (expressionUsesVariable(lhs, variable) && !expressionIsVariable(lhs, variable)) + return src; + return 0; + } + + case "Loop": + return 0; + + case "Assembly": + return 0; + + default: + assert(false); + } +} + +function expressionIsVariableAddress(exp, variable) +{ + while (exp.Kind == "Fld") + exp = exp.Exp[0]; + return exp.Kind == "Var" && sameVariable(exp.Variable, variable); +} + +function edgeTakesVariableAddress(edge, variable, body) +{ + if (ignoreEdgeUse(edge, variable, body)) + return false; + if (ignoreEdgeAddressTaken(edge)) + return false; + switch (edge.Kind) { + case "Assign": + return expressionIsVariableAddress(edge.Exp[1], variable); + case "Call": + if ("PEdgeCallArguments" in edge) { + for (var exp of edge.PEdgeCallArguments.Exp) { + if (expressionIsVariableAddress(exp, variable)) + return true; + } + } + return false; + default: + return false; + } +} + +function expressionIsVariable(exp, variable) +{ + return exp.Kind == "Var" && sameVariable(exp.Variable, variable); +} + +function expressionIsMethodOnVariable(exp, variable) +{ + // This might be calling a method on a base class, in which case exp will + // be an unnamed field of the variable instead of the variable itself. + while (exp.Kind == "Fld" && exp.Field.Name[0].startsWith("field:")) + exp = exp.Exp[0]; + + return exp.Kind == "Var" && sameVariable(exp.Variable, variable); +} + +// Return whether the edge starts the live range of a variable's value, by setting +// it to some new value. Examples of starting obj's live range: +// +// obj = foo; +// obj = foo(); +// obj = foo(obj); // uses previous value but then sets to new value +// SomeClass obj(true, 1); // constructor +// +function edgeStartsValueLiveRange(edge, variable) +{ + // Direct assignments start live range of lhs: var = value + if (edge.Kind == "Assign") { + const [lhs, rhs] = edge.Exp; + return (expressionIsVariable(lhs, variable) && + !isReturningImmobileValue(edge, variable)); + } + + if (edge.Kind != "Call") + return false; + + // Assignments of call results start live range: var = foo() + if (1 in edge.Exp) { + var lhs = edge.Exp[1]; + if (expressionIsVariable(lhs, variable)) + return true; + } + + // Constructor calls start live range of instance: SomeClass var(...) + if ("PEdgeCallInstance" in edge) { + var instance = edge.PEdgeCallInstance.Exp; + + // Kludge around incorrect dereference on some constructor calls. + if (instance.Kind == "Drf") + instance = instance.Exp[0]; + + if (!expressionIsVariable(instance, variable)) + return false; + + var callee = edge.Exp[0]; + if (callee.Kind != "Var") + return false; + + assert(callee.Variable.Kind == "Func"); + var calleeName = readable(callee.Variable.Name[0]); + + // Constructor calls include the text 'Name::Name(' or 'Name<...>::Name('. + var openParen = calleeName.indexOf('('); + if (openParen < 0) + return false; + calleeName = calleeName.substring(0, openParen); + + var lastColon = calleeName.lastIndexOf('::'); + if (lastColon < 0) + return false; + var constructorName = calleeName.substr(lastColon + 2); + calleeName = calleeName.substr(0, lastColon); + + var lastTemplateOpen = calleeName.lastIndexOf('<'); + if (lastTemplateOpen >= 0) + calleeName = calleeName.substr(0, lastTemplateOpen); + + if (calleeName.endsWith(constructorName)) + return true; + } + + return false; +} + +function edgeMovesVariable(edge, variable) +{ + if (edge.Kind != 'Call') + return false; + const callee = edge.Exp[0]; + if (callee.Kind == 'Var' && + callee.Variable.Kind == 'Func') + { + const { Variable: { Name: [ fullname, shortname ] } } = callee; + const [ mangled, unmangled ] = splitFunction(fullname); + // Match a UniquePtr move constructor. + if (unmangled.match(/::UniquePtr<[^>]*>::UniquePtr\((\w+::)*UniquePtr<[^>]*>&&/)) + return true; + } + + return false; +} + +// Scan forward through the given 'body', starting at 'startpoint', looking for +// a call that passes 'variable' to a move constructor that "consumes" it (eg +// UniquePtr::UniquePtr(UniquePtr&&)). +function bodyEatsVariable(variable, body, startpoint) +{ + const successors = getSuccessors(body); + const work = [startpoint]; + while (work.length > 0) { + const point = work.shift(); + if (!(point in successors)) + continue; + for (const edge of successors[point]) { + if (edgeMovesVariable(edge, variable)) + return true; + // edgeStartsValueLiveRange will find places where 'variable' is given a + // new value. Never observed in practice, since this function is + // only called with a temporary resulting from std::move(), which + // is used immediately for a call. But just to be robust to future + // uses: + if (!edgeStartsValueLiveRange(edge, variable)) + work.push(edge.Index[1]); + } + } + return false; +} + +// Return whether an edge "clears out" a variable's value. A simple example +// would be +// +// var = nullptr; +// +// for analyses for which nullptr is a "safe" value (eg GC rooting hazards; you +// can't get in trouble by holding a nullptr live across a GC.) A more complex +// example is a Maybe that gets reset: +// +// Maybe nogc; +// nogc.emplace(cx); +// nogc.reset(); +// gc(); // <-- not a problem; nogc is invalidated by prev line +// nogc.emplace(cx); +// foo(nogc); +// +// Yet another example is a UniquePtr being passed by value, which means the +// receiver takes ownership: +// +// UniquePtr uobj(obj); +// foo(uobj); +// gc(); +// +function edgeEndsValueLiveRange(edge, variable, body) +{ + // var = nullptr; + if (edge.Kind == "Assign") { + const [lhs, rhs] = edge.Exp; + return expressionIsVariable(lhs, variable) && isImmobileValue(rhs); + } + + if (edge.Kind != "Call") + return false; + + var callee = edge.Exp[0]; + + if (edge.Type.Kind == 'Function' && + edge.Exp[0].Kind == 'Var' && + edge.Exp[0].Variable.Kind == 'Func' && + edge.Exp[0].Variable.Name[1] == 'MarkVariableAsGCSafe' && + edge.Exp[0].Variable.Name[0].includes("JS::detail::MarkVariableAsGCSafe") && + expressionIsVariable(edge.PEdgeCallArguments.Exp[0], variable)) + { + // explicit JS_HAZ_VARIABLE_IS_GC_SAFE annotation + return true; + } + + if (edge.Type.Kind == 'Function' && + edge.Exp[0].Kind == 'Var' && + edge.Exp[0].Variable.Kind == 'Func' && + edge.Exp[0].Variable.Name[1] == 'move' && + edge.Exp[0].Variable.Name[0].includes('std::move(') && + expressionIsVariable(edge.PEdgeCallArguments.Exp[0], variable) && + edge.Exp[1].Kind == 'Var' && + edge.Exp[1].Variable.Kind == 'Temp') + { + // temp = std::move(var) + // + // If var is a UniquePtr, and we pass it into something that takes + // ownership, then it should be considered to be invalid. It really + // ought to be invalidated at the point of the function call that calls + // the move constructor, but given that we're creating a temporary here + // just for the purpose of passing it in, this edge is good enough. + const lhs = edge.Exp[1].Variable; + if (bodyEatsVariable(lhs, body, edge.Index[1])) + return true; + } + + if (edge.Type.Kind == 'Function' && + edge.Type.TypeFunctionCSU && + edge.PEdgeCallInstance && + expressionIsMethodOnVariable(edge.PEdgeCallInstance.Exp, variable)) + { + const typeName = edge.Type.TypeFunctionCSU.Type.Name; + const m = typeName.match(/^(((\w|::)+?)(\w+)). + const ctorName = `${namespace}${classname}::${classname}()`; + if (callee.Kind == 'Var' && + typesWithSafeConstructors.has(type) && + callee.Variable.Name[0].includes(ctorName)) + { + return true; + } + + // special-case: UniquePtr::reset() and similar. + if (callee.Kind == 'Var' && + type in resetterMethods && + resetterMethods[type].has(callee.Variable.Name[1])) + { + return true; + } + } + } + + // special-case: passing UniquePtr by value. + if (edge.Type.Kind == 'Function' && + edge.Type.TypeFunctionArgument && + edge.PEdgeCallArguments) + { + for (const i in edge.Type.TypeFunctionArgument) { + const param = edge.Type.TypeFunctionArgument[i]; + if (param.Type.Kind != 'CSU') + continue; + if (!param.Type.Name.startsWith("mozilla::UniquePtr<")) + continue; + const arg = edge.PEdgeCallArguments.Exp[i]; + if (expressionIsVariable(arg, variable)) { + return true; + } + } + } + + return false; +} + function edgeCanGC(edge) { if (edge.Kind != "Call") @@ -155,57 +550,10 @@ function edgeCanGC(edge) return false; } -// Search upwards through a function's control flow graph (CFG) to find a path containing: -// -// - a use of a variable, preceded by -// -// - a function call that can GC, preceded by -// -// - a use of the variable that shows that the live range starts at least that -// far back, preceded by -// -// - an informative use of the variable (which might be the same use), one that -// assigns to it a value that might contain a GC pointer (or is the start of -// the function for parameters or 'this'.) This is not necessary for -// correctness, it just makes it easier to understand why something might be -// a hazard. The output of the analysis will include the whole path from the -// informative use to the post-GC use, to make the problem as understandable -// as possible. -// -// A canonical example might be: -// -// void foo() { -// JS::Value* val = lookupValue(); <-- informative use -// if (!val.isUndefined()) { <-- any use -// GC(); <-- GC call -// } -// putValue(val); <-- a use after a GC -// } -// -// The search is performed on an underlying CFG that we traverse in -// breadth-first order (to find the shortest path). We build a path starting -// from an empty path and conditionally lengthening and improving it according -// to the computation occurring on each incoming edge. (If that path so far -// does not have a GC call and we traverse an edge with a GC call, then we -// lengthen the path by that edge and record it as including a GC call.) The -// resulting path may include a point or edge more than once! For example, in: -// -// void foo(JS::Value val) { -// for (int i = 0; i < N; i++) { -// GC(); -// val = processValue(val); -// } -// } -// -// the path would start at the point after processValue(), go through the GC(), -// then back to the processValue() (for the call in the previous loop -// iteration). -// -// While searching, each point is annotated with a path node corresponding to -// the best path found to that node so far. When a later search ends up at the -// same point, the best path node is kept. (But the path that it heads may -// include an earlier path node for the same point, as in the case above.) -// +// Search recursively through predecessors from the use of a variable's value, +// returning whether a GC call is reachable (in the reverse direction; this +// means that the variable use is reachable from the GC call, and therefore the +// variable is live after the GC call), along with some additional information. // What info we want depends on whether the variable turns out to be live // across a GC call. We are looking for both hazards (unrooted variables live // across GC calls) and unnecessary roots (rooted variables that have no GC @@ -218,286 +566,200 @@ function edgeCanGC(edge) // // If so: // -// - 'successor': a path from the GC call to a use of the variable after the GC -// call, chained through 'successor' field in the returned edge descriptor +// - 'why': a path from the GC call to a use of the variable after the GC +// call, chained through a 'why' field in the returned edge descriptor // // - 'gcInfo': a direct pointer to the GC call edge // -function findGCBeforeValueUse(start_body, start_point, suppressed_bits, variable) +function findGCBeforeValueUse(start_body, start_point, suppressed, variable) { - const isGCSuppressed = Boolean(suppressed_bits & ATTR_GC_SUPPRESSED); - // Scan through all edges preceding an unrooted variable use, using an - // explicit worklist, looking for a GC call and a preceding point where the - // variable is known to be live. A worklist contains an incoming edge - // together with a description of where it or one of its successors GC'd - // (if any). + // 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). - class Path { - get ProgressProperties() { return ["informativeUse", "anyUse", "gcInfo"]; } + var bodies_visited = new Map(); - constructor(successor_path, body, ppoint) { - Object.assign(this, {body, ppoint}); - if (successor_path !== undefined) { - this.successor = successor_path; - for (const prop of this.ProgressProperties) { - if (prop in successor_path) { - this[prop] = successor_path[prop]; + 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; + + // 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; + } + visited.set(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. + if ("BlockPPoint" in body) { + for (var parent of body.BlockPPoint) { + var found = false; + for (var xbody of functionBodies) { + if (sameBlockId(xbody.BlockId, parent.BlockId)) { + assert(!found); + found = true; + worklist.push({body: xbody, ppoint: parent.Index, + gcInfo: gcInfo, why: entry}); + } + } + assert(found); } } + + // Also propagate to the *end* of this loop, for the previous + // iteration. + worklist.push({body: body, ppoint: body.Index[1], + gcInfo: gcInfo, why: entry}); + } else if ((variable.Kind == "Arg" || variable.Kind == "This") && 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; } } - toString() { - const trail = []; - for (let path = this; path.ppoint; path = path.successor) { - trail.push(path.ppoint); - } - return trail.join(); - } + var predecessors = getPredecessors(body); + if (!(ppoint in predecessors)) + continue; - // Return -1, 0, or 1 to indicate how complete this Path is compared - // to another one. - compare(other) { - for (const prop of this.ProgressProperties) { - const a = this.hasOwnProperty(prop); - const b = other.hasOwnProperty(prop); - if (a != b) { - return a - b; - } - } - return 0; - } - }; - - // In case we never find an informative use, keep track of the best path - // found with any use. - let bestPathWithAnyUse = null; - - const visitor = new class extends Visitor { - constructor() { - super(functionBodies); - } - - // Do a BFS upwards through the CFG, starting from a use of the - // variable and searching for a path containing a GC followed by an - // initializing use of the variable (or, in forward direction, a start - // of the variable's live range, a GC within that live range, and then - // a use showing that the live range extends past the GC call.) - // Actually, possibly two uses: any use at all, and then if available - // an "informative" use that is more convincing (they may be the same). - // - // The CFG is a graph (a 'body' here is acyclic, but they can contain - // loop nodes that bridge to additional bodies for the loop, so the - // overall graph can by cyclic.) That means there may be multiple paths - // from point A to point B, and we want paths with a GC on them. This - // can be thought of as searching for a "maximal GCing" path from a use - // A to an initialization B. - // - // This is implemented as a BFS search that when it reaches a point - // that has been visited before, stops if and only if the current path - // being advanced is a less GC-ful path. The traversal pushes a - // `gcInfo` token, initially empty, up through the graph and stores the - // maximal one visited so far at every point. - // - // Note that this means we may traverse through the same point more - // than once, and so in theory this scan is superlinear -- if you visit - // every point twice, once for a non GC path and once for a GC path, it - // would be 2^n. But that is unlikely to matter, since you'd need lots - // of split/join pairs that GC on one side and not the other, and you'd - // have to visit them in an unlucky order. This could be fixed by - // updating the gcInfo for past points in a path when a GC is found, - // but it hasn't been found to matter in practice yet. - - next_action(prev, current) { - // Continue if first visit, or the new path is more complete than the old path. This - // could be enhanced at some point to choose paths with 'better' - // examples of GC (eg a call that invokes GC through concrete functions rather than going through a function pointer that is conservatively assumed to GC.) - - if (!current) { - // This search path has been terminated. - return "prune"; - } - - if (current.informativeUse) { - // We have a path with an informative use leading to a GC - // leading to the starting point. - assert(current.gcInfo); - return "done"; - } - - if (prev === undefined) { - // first visit - return "continue"; - } - - if (!prev.gcInfo && current.gcInfo) { - // More GC. - return "continue"; - } else { - return "prune"; - } - } - - merge_info(prev, current) { - // Keep the most complete path. - - if (!prev || !current) { - return prev || current; - } - - // Tie goes to the first found, since it will be shorter when doing a BFS-like search. - return prev.compare(current) >= 0 ? prev : current; - } - - extend_path(edge, body, ppoint, successor_path) { - // Clone the successor path node and then tack on the new point. Other values - // will be updated during the rest of this function, according to what is - // happening on the edge. - const path = new Path(successor_path, body, ppoint); - if (edge === null) { - // Artificial edge to connect loops to their surrounding nodes in the outer body. - // Does not influence "completeness" of path. - return path; - } - - assert(ppoint == edge.Index[0]); + for (var edge of predecessors[ppoint]) { + var source = edge.Index[0]; if (edgeEndsValueLiveRange(edge, variable, body)) { - // Terminate the search through this point. - return null; + // Terminate the search through this point; we thought we were + // within the live range, but it turns out that the variable + // was set to a value that we don't care about. + continue; } - const edge_starts = edgeStartsValueLiveRange(edge, variable); - const edge_uses = edgeUsesVariable(edge, variable, body); + var edge_kills = edgeStartsValueLiveRange(edge, variable); + var edge_uses = edgeUsesVariable(edge, variable, body); - if (edge_starts || edge_uses) { - if (!body.minimumUse || ppoint < body.minimumUse) - body.minimumUse = ppoint; + if (edge_kills || edge_uses) { + if (!body.minimumUse || source < body.minimumUse) + body.minimumUse = source; } - if (edge_starts) { + if (edge_kills) { // This is a beginning of the variable's live range. If we can // reach a GC call from here, then we're done -- we have a path - // from the beginning of the live range, through the GC call, to a - // use after the GC call that proves its live range extends at - // least that far. - if (path.gcInfo) { - path.anyUse = path.anyUse || edge; - path.informativeUse = path.informativeUse || edge; - return path; - } + // from the beginning of the live range, through the GC call, + // 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 }; - // Otherwise, truncate this particular branch of the search at this - // edge -- there is no GC after this use, and traversing the edge - // would lead to a different live range. - return null; + // Otherwise, keep searching through the graph, but truncate + // this particular branch of the search at this edge. + continue; } - // The value is live across this edge. Check whether this edge can GC (if we don't have a GC yet on this path.) - const had_gcInfo = Boolean(path.gcInfo); - if (!path.gcInfo && !(body.attrs[ppoint] & ATTR_GC_SUPPRESSED) && !isGCSuppressed) { + var src_gcInfo = gcInfo; + var src_preGCLive = preGCLive; + if (!gcInfo && !(body.attrs[source] & ATTR_GC_SUPPRESSED) && !(suppressed & ATTR_GC_SUPPRESSED)) { var gcName = edgeCanGC(edge, body); - if (gcName) { - path.gcInfo = {name:gcName, body, ppoint}; + if (gcName) + src_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 even if we + // don't find an assignment. + + if (src_gcInfo) { + src_preGCLive = true; + if (edge.Kind == 'Assign') + return {body:body, ppoint:source, gcInfo:src_gcInfo, why:entry}; } } - // Beginning of function? - if (ppoint == body.Index[0] && body.BlockId.Kind != "Loop") { - if (path.gcInfo && (variable.Kind == "Arg" || variable.Kind == "This")) { - // The scope of arguments starts at the beginning of the - // function. - path.anyUse = path.informativeUse = true; - } - - if (path.anyUse) { - // We know the variable was live across the GC. We may or - // may not have found an "informative" explanation - // beginning of the live range. (This can happen if the - // live range started when a variable is used as a - // retparam.) - return path; + if (edge.Kind == "Loop") { + // Additionally propagate the search into a loop body, starting + // with the exit point. + var found = false; + for (var xbody of functionBodies) { + if (sameBlockId(xbody.BlockId, edge.BlockId)) { + assert(!found); + found = true; + worklist.push({body:xbody, ppoint:xbody.Index[1], + preGCLive: src_preGCLive, gcInfo:src_gcInfo, + why:entry}); + } } + assert(found); + // Don't continue to predecessors here without going through + // the loop. (The points in this body that enter the loop will + // be traversed when we reach the entry point of the loop.) + break; } - if (!path.gcInfo) { - // We haven't reached a GC yet, so don't start looking for uses. - return path; - } + // Propagate the search to the predecessors of this edge. + worklist.push({body:body, ppoint:source, + preGCLive: src_preGCLive, gcInfo:src_gcInfo, + why:entry}); + } + } - if (!edge_uses) { - // We have a GC. If this edge doesn't use the value, then there - // is no change to the completeness of the path. - return path; - } - - // The live range starts at least this far back, so we're done for - // the same reason as with edge_starts. 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 'successor' path to - // include the ancestor where the value was generated. So we will - // only stop 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 even if we don't find an assignment. - - // One special case: if the use of the variable is on the - // destination part of the edge (which currently only happens for - // the return value and a terminal edge in the body), and this edge - // is also GCing, then that usage happens *after* the GC and so - // should not be used for anyUse or informativeUse. This matters - // for a hazard involving a destructor GC'ing after an immobile - // return value has been assigned: - // - // GCInDestructor guard(cx); - // if (cond()) { - // return nullptr; - // } - // - // which boils down to - // - // p1 --(construct guard)--> - // p2 --(call cond)--> - // p3 --(returnval := nullptr) --> - // p4 --(destruct guard, possibly GCing)--> - // p5 - // - // The return value is considered to be live at p5. The live range - // of the return value would ordinarily be from p3->p4->p5, except - // that the nullptr assignment means it needn't be considered live - // back that far, and so the live range is *just* p5. The GC on the - // 4->5 edge happens just before that range, so the value was not - // live across the GC. - // - if (!had_gcInfo && edge_uses == edge.Index[1]) { - return path; // New GC does not cross this variable use. - } - - path.anyUse = path.anyUse || edge; - bestPathWithAnyUse = bestPathWithAnyUse || path; - if (edge.Kind == 'Assign') { - path.informativeUse = edge; // Done! Setting this terminates the search. - } - - return path; - }; - }; - - return BFS_upwards(start_body, start_point, functionBodies, visitor, new Path()) || bestPathWithAnyUse; + return null; } function variableLiveAcrossGC(suppressed, variable) @@ -580,8 +842,8 @@ function unsafeVariableAddressTaken(suppressed, variable) // given function and store it. function loadPrintedLines(functionName) { - assert(!os.system("xdbfind src_body.xdb '" + functionName + "' > " + options.tmpfile)); - var lines = snarf(options.tmpfile).split('\n'); + assert(!os.system("xdbfind src_body.xdb '" + functionName + "' > " + tmpfile)); + var lines = snarf(tmpfile).split('\n'); for (var body of functionBodies) body.lines = []; @@ -644,15 +906,14 @@ function printEntryTrace(functionName, entry) if (!functionBodies[0].lines) loadPrintedLines(functionName); - while (entry.successor) { + while (entry) { var ppoint = entry.ppoint; var lineText = findLocation(entry.body, ppoint, {"brief": true}); var edgeText = ""; - if (entry.successor && entry.successor.body == entry.body) { - // If the next point in the trace is in the same block, look for an - // edge between them. - var next = entry.successor.ppoint; + if (entry.why && entry.why.body == entry.body) { + // If the next point in the trace is in the same block, look for an edge between them. + var next = entry.why.ppoint; if (!entry.body.edgeTable) { var table = {}; @@ -686,7 +947,7 @@ function printEntryTrace(functionName, entry) } print(" " + lineText + (edgeText.length ? ": " + edgeText : "")); - entry = entry.successor; + entry = entry.why; } } @@ -756,8 +1017,7 @@ function processBodies(functionName, wholeBodyAttrs) // Hopefully these will be simplified at some point (see bug 1517829), but // for now we special-case functions in the mozilla::dom namespace that // contain locals with types ending in "Argument". Or - // Maybe. Or Maybe>. It's - // a harsh world. + // Maybe. It's a harsh world. const ignoreVars = new Set(); if (functionName.match(/mozilla::dom::/)) { const vars = functionBodies[0].DefineVariable.filter( @@ -766,11 +1026,9 @@ function processBodies(functionName, wholeBodyAttrs) v => [ v.Variable.Name[0], v.Type.Name ] ); - const holders = vars.filter( - ([n, t]) => n.match(/^arg\d+_holder$/) && - (t.includes("Argument") || t.includes("Rooter"))); + const holders = vars.filter(([n, t]) => n.match(/^arg\d+_holder$/) && t.match(/Argument\b/)); for (const [holder,] of holders) { - ignoreVars.add(holder); // Ignore the holder. + ignoreVars.add(holder); // Ignore the older. ignoreVars.add(holder.replace("_holder", "")); // Ignore the "managed" arg. } } @@ -804,7 +1062,6 @@ function processBodies(functionName, wholeBodyAttrs) } else if (isUnrootedType(variable.Type)) { var result = variableLiveAcrossGC(suppressed, variable.Variable); if (result) { - assert(result.gcInfo); var lineText = findLocation(result.gcInfo.body, result.gcInfo.ppoint); if (annotations.has('Expect Hazards')) { print("\nThis is expected, but '" + functionName + "'" + @@ -848,7 +1105,7 @@ function processBodies(functionName, wholeBodyAttrs) } } -if (options.batch == 1) +if (batch == 1) print("Time: " + new Date); var xdb = xdbLibrary(); @@ -857,8 +1114,8 @@ xdb.open("src_body.xdb"); var minStream = xdb.min_data_stream()|0; var maxStream = xdb.max_data_stream()|0; -var start = batchStart(options.batch, options.numBatches, minStream, maxStream); -var end = batchLast(options.batch, options.numBatches, minStream, maxStream); +var start = batchStart(batch, numBatches, minStream, maxStream); +var end = batchLast(batch, numBatches, minStream, maxStream); function process(name, json) { functionName = name; @@ -877,9 +1134,6 @@ function process(name, json) { if (attrs) pbody.attrs[id] = attrs; } - for (const edgeAttr of gcEdges[blockIdentifier(body)] || []) { - body.attrs[edgeAttr.Index[0]] |= edgeAttr.attrs; - } } // Special case: std::swap of two refcounted values thinks it can drop the @@ -894,11 +1148,11 @@ function process(name, json) { processBodies(functionName, wholeBodyAttrs); } -if (options.function) { - var data = xdb.read_entry(options.function); +if (theFunctionNameToFind) { + var data = xdb.read_entry(theFunctionNameToFind); var json = data.readString(); debugger; - process(options.function, json); + process(theFunctionNameToFind, json); xdb.free_string(data); quit(0); } diff --git a/js/src/devtools/rootAnalysis/annotations.js b/js/src/devtools/rootAnalysis/annotations.js index 314958566e98..3198020a5464 100644 --- a/js/src/devtools/rootAnalysis/annotations.js +++ b/js/src/devtools/rootAnalysis/annotations.js @@ -17,7 +17,7 @@ var ignoreIndirectCalls = { }; // Types that when constructed with no arguments, are "safe" values (they do -// not contain GC pointers, or values with nontrivial destructors.) +// not contain GC pointers). var typesWithSafeConstructors = new Set([ "mozilla::Maybe", "mozilla::dom::Nullable", @@ -32,14 +32,8 @@ var resetterMethods = { 'js::UniquePtr': new Set(["reset"]), 'mozilla::dom::Nullable': new Set(["SetNull"]), 'mozilla::dom::TypedArray_base': new Set(["Reset"]), - 'RefPtr': new Set(["forget"]), - 'nsCOMPtr': new Set(["forget"]), }; -function isRefcountedDtor(name) { - return name.includes("::~RefPtr(") || name.includes("::~nsCOMPtr("); -} - function indirectCallCannotGC(fullCaller, fullVariable) { var caller = readable(fullCaller); diff --git a/js/src/devtools/rootAnalysis/computeCallgraph.js b/js/src/devtools/rootAnalysis/computeCallgraph.js index 3d7d95a4be46..c4737cdc7594 100644 --- a/js/src/devtools/rootAnalysis/computeCallgraph.js +++ b/js/src/devtools/rootAnalysis/computeCallgraph.js @@ -8,46 +8,23 @@ loadRelativeToScript('callgraph.js'); -var options = parse_options([ - { - name: '--function', - type: 'string' - }, - { - name: 'typeInfo_filename', - type: 'string', - default: "typeInfo.txt" - }, - { - name: 'callgraphOut_filename', - type: 'string', - default: "rawcalls.txt" - }, - { - name: 'gcEdgesOut_filename', - type: 'string', - default: "gcEdges.json" - }, - { - name: 'batch', - default: 1, - type: 'number' - }, - { - name: 'numBatches', - default: 1, - type: 'number' - }, -]); +var theFunctionNameToFind; +if (scriptArgs[0] == '--function' || scriptArgs[0] == '-f') { + theFunctionNameToFind = scriptArgs[1]; + scriptArgs = scriptArgs.slice(2); +} -var origOut = os.file.redirect(options.callgraphOut_filename); +var typeInfo_filename = scriptArgs[0] || "typeInfo.txt"; +var callgraphOut_filename = scriptArgs[1] || "rawcalls.txt"; +var batch = (scriptArgs[2]|0) || 1; +var numBatches = (scriptArgs[3]|0) || 1; + +var origOut = os.file.redirect(callgraphOut_filename); var memoized = new Map(); var unmangled2id = new Set(); -var gcEdges = {}; - // Insert a string into the name table and return the ID. Do not use for // functions, which must be handled specially. function getId(name) @@ -128,7 +105,7 @@ function getAnnotations(functionName, body) { // Scan through a function body, pulling out all annotations and calls and // recording them in callgraph.txt. -function processBody(functionName, body, functionBodies) +function processBody(functionName, body) { if (!('PEdge' in body)) return; @@ -161,13 +138,6 @@ function processBody(functionName, body, functionBodies) var edgeAttrs = body.attrs[edge.Index[0]] | 0; for (var callee of getCallees(edge)) { - // Special-case some calls when we can derive more information about them, eg - // that they are a destructor that won't do anything. - if (callee.kind === "direct" && edgeIsNonReleasingDtor(body, edge, callee.name, functionBodies)) { - const block = blockIdentifier(body); - addToKeyedList(gcEdges, block, { Index: edge.Index, attrs: ATTR_GC_SUPPRESSED | ATTR_NONRELEASING }); - } - // Individual callees may have additional attrs. The only such // bit currently is that nsISupports.{AddRef,Release} are assumed // to never GC. @@ -228,7 +198,7 @@ assert(ID.nogcfunc == functionId("(nogc-function)")); // garbage collection assert(ID.gc == functionId("(GC)")); -var typeInfo = loadTypeInfo(options.typeInfo_filename); +var typeInfo = loadTypeInfo(typeInfo_filename); loadTypes("src_comp.xdb"); @@ -286,8 +256,8 @@ printErr("Finished loading data structures"); var minStream = xdb.min_data_stream(); var maxStream = xdb.max_data_stream(); -if (options.function) { - var index = xdb.lookup_key(options.function); +if (theFunctionNameToFind) { + var index = xdb.lookup_key(theFunctionNameToFind); if (!index) { printErr("Function not found"); quit(1); @@ -307,7 +277,7 @@ function process(functionName, functionBodies) } for (var body of functionBodies) - processBody(functionName, body, functionBodies); + processBody(functionName, body); // Not strictly necessary, but add an edge from the synthetic "(js-code)" // to RunScript to allow better stacks than just randomly selecting a @@ -420,8 +390,8 @@ function process(functionName, functionBodies) printOnce(`D ${functionId("(js-code)")} ${functionId(functionName)}`); } -var start = batchStart(options.batch, options.numBatches, minStream, maxStream); -var end = batchLast(options.batch, options.numBatches, minStream, maxStream); +var start = batchStart(batch, numBatches, minStream, maxStream); +var end = batchLast(batch, numBatches, minStream, maxStream); for (var nameIndex = start; nameIndex <= end; nameIndex++) { var name = xdb.read_key(nameIndex); @@ -431,8 +401,4 @@ for (var nameIndex = start; nameIndex <= end; nameIndex++) { xdb.free_string(data); } -os.file.close(os.file.redirect(options.gcEdgesOut_filename)); - -print(JSON.stringify(gcEdges, null, 4)); - os.file.close(os.file.redirect(origOut)); diff --git a/js/src/devtools/rootAnalysis/computeGCFunctions.js b/js/src/devtools/rootAnalysis/computeGCFunctions.js index 90705051c911..02edc9399cc2 100644 --- a/js/src/devtools/rootAnalysis/computeGCFunctions.js +++ b/js/src/devtools/rootAnalysis/computeGCFunctions.js @@ -18,52 +18,31 @@ if (typeof scriptArgs[0] != 'string') var start = "Time: " + new Date; -try { - var options = parse_options([ - { - name: 'inputs', - dest: 'rawcalls_filenames', - nargs: '+' - }, - { - name: '--outputs', - type: 'bool' - }, - { - name: 'callgraph', - type: 'string', - default: 'callgraph.txt' - }, - { - name: 'gcFunctions', - type: 'string', - default: 'gcFunctions.txt' - }, - { - name: 'gcFunctionsList', - type: 'string', - default: 'gcFunctions.lst' - }, - { - name: 'limitedFunctions', - type: 'string', - default: 'limitedFunctions.lst' - }, - ]); -} catch { - printErr("Usage: computeGCFunctions.js ... --outputs "); - quit(1); -}; +var rawcalls_filenames = []; +while (scriptArgs.length) { + const arg = scriptArgs.shift(); + if (arg == '--outputs') + break; + rawcalls_filenames.push(arg); +} +if (scriptArgs.length == 0) + usage(); + +var callgraph_filename = scriptArgs[0] || "callgraph.txt"; +var gcFunctions_filename = scriptArgs[1] || "gcFunctions.txt"; +var gcFunctionsList_filename = scriptArgs[2] || "gcFunctions.lst"; +var gcEdges_filename = scriptArgs[3] || "gcEdges.txt"; +var limitedFunctionsList_filename = scriptArgs[4] || "limitedFunctions.lst"; var { gcFunctions, functions, calleesOf, limitedFunctions -} = loadCallgraph(options.rawcalls_filenames); +} = loadCallgraph(rawcalls_filenames); -printErr("Writing " + options.gcFunctions); -redirect(options.gcFunctions); +printErr("Writing " + gcFunctions_filename); +redirect(gcFunctions_filename); for (var name in gcFunctions) { for (let readable of (functions.readableName[name] || [name])) { @@ -83,8 +62,8 @@ for (var name in gcFunctions) { } } -printErr("Writing " + options.gcFunctionsList); -redirect(options.gcFunctionsList); +printErr("Writing " + gcFunctionsList_filename); +redirect(gcFunctionsList_filename); for (var name in gcFunctions) { if (name in functions.readableName) { for (var readable of functions.readableName[name]) @@ -94,10 +73,28 @@ for (var name in gcFunctions) { } } -printErr("Writing " + options.limitedFunctions); -redirect(options.limitedFunctions); +// gcEdges is a list of edges that can GC for more specific reasons than just +// calling a function that is in gcFunctions.txt. +// +// Right now, it is unused. It was meant for ~AutoRealm when it might +// wrap an exception, but anything held live across ~AC will have to be held +// live across the corresponding constructor (and hence the whole scope of the +// AC), and in that case it'll be held live across whatever could create an +// exception within the AC scope. So ~AC edges are redundant. I will leave the +// stub machinery here for now. +printErr("Writing " + gcEdges_filename); +redirect(gcEdges_filename); +for (var block in gcEdges) { + for (var edge in gcEdges[block]) { + var func = gcEdges[block][edge]; + print([ block, edge, func ].join(" || ")); + } +} + +printErr("Writing " + limitedFunctionsList_filename); +redirect(limitedFunctionsList_filename); print(JSON.stringify(limitedFunctions, null, 4)); -printErr("Writing " + options.callgraph); -redirect(options.callgraph); +printErr("Writing " + callgraph_filename); +redirect(callgraph_filename); saveCallgraph(functions, calleesOf); diff --git a/js/src/devtools/rootAnalysis/computeGCTypes.js b/js/src/devtools/rootAnalysis/computeGCTypes.js index 9fc72f6d4442..a52aed832af3 100644 --- a/js/src/devtools/rootAnalysis/computeGCTypes.js +++ b/js/src/devtools/rootAnalysis/computeGCTypes.js @@ -9,10 +9,8 @@ loadRelativeToScript('utility.js'); loadRelativeToScript('annotations.js'); -var options = parse_options([ - { name: "gcTypes", default: "gcTypes.txt" }, - { name: "typeInfo", default: "typeInfo.txt" } -]); +var gcTypes_filename = scriptArgs[0] || "gcTypes.txt"; +var typeInfo_filename = scriptArgs[1] || "typeInfo.txt"; var typeInfo = { 'GCPointers': [], @@ -490,7 +488,7 @@ function explain(csu, indent, seen) { } } -var origOut = os.file.redirect(options.gcTypes); +var origOut = os.file.redirect(gcTypes_filename); for (var csu in gcTypes) { print("GCThing: " + csu); @@ -502,7 +500,7 @@ for (var csu in gcPointers) { } // Redirect output to the typeInfo file and close the gcTypes file. -os.file.close(os.file.redirect(options.typeInfo)); +os.file.close(os.file.redirect(typeInfo_filename)); // Compute the set of types that suppress GC within their RAII scopes (eg // AutoSuppressGC, AutoSuppressGCForAnalysis). diff --git a/js/src/devtools/rootAnalysis/loadCallgraph.js b/js/src/devtools/rootAnalysis/loadCallgraph.js index eef76c24b356..d824dd01495c 100644 --- a/js/src/devtools/rootAnalysis/loadCallgraph.js +++ b/js/src/devtools/rootAnalysis/loadCallgraph.js @@ -419,11 +419,8 @@ function loadCallgraph(files) // set of mangled names (map from mangled name => {any,all,recursive_root:bool} var limitedFunctions = {}; - for (const [id, [any, all]] of Object.entries(functionAttrs)) { - if (all) { - limitedFunctions[functions.name[id]] = { attributes: all }; - } - } + for (const [id, [any, all]] of Object.entries(functionAttrs)) + limitedFunctions[functions.name[id]] = { attributes: all }; for (const [id, limits, label] of recursive_roots) { const name = functions.name[id]; diff --git a/js/src/devtools/rootAnalysis/mergeJSON.js b/js/src/devtools/rootAnalysis/mergeJSON.js deleted file mode 100644 index 56fb3a2ab1c3..000000000000 --- a/js/src/devtools/rootAnalysis/mergeJSON.js +++ /dev/null @@ -1,18 +0,0 @@ -/* 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/. */ - -/* -*- indent-tabs-mode: nil; js-indent-level: 4 -*- */ - -var infiles = [...scriptArgs]; -var outfile = infiles.pop(); - -const output = {}; -for (const filename of infiles) { - const data = JSON.parse(os.file.readFile(filename)); - Object.assign(output, data); -} - -var origOut = os.file.redirect(outfile); -print(JSON.stringify(output, null, 4)); -os.file.close(os.file.redirect(origOut)); diff --git a/js/src/devtools/rootAnalysis/run-test.py b/js/src/devtools/rootAnalysis/run-test.py index f0bafb2babf2..cd02d68939d4 100755 --- a/js/src/devtools/rootAnalysis/run-test.py +++ b/js/src/devtools/rootAnalysis/run-test.py @@ -108,7 +108,6 @@ make_dir(outroot) os.environ["HAZARD_RUN_INTERNAL_TESTS"] = "1" failed = set() -passed = set() for name in cfg.tests: name = os.path.basename(name) indir = os.path.join(testdir, name) @@ -135,9 +134,6 @@ for name in cfg.tests: raise else: print("TEST-PASSED: %s" % name) - passed.add(name) if failed: raise Exception("Failed tests: " + " ".join(failed)) - -print(f"All {len(passed)} tests passed.") diff --git a/js/src/devtools/rootAnalysis/t/hazards/source.cpp b/js/src/devtools/rootAnalysis/t/hazards/source.cpp index fc5ba8464a28..e866a789f7b4 100644 --- a/js/src/devtools/rootAnalysis/t/hazards/source.cpp +++ b/js/src/devtools/rootAnalysis/t/hazards/source.cpp @@ -367,46 +367,3 @@ class Subcell : public Cell { return f; // this->f } }; - -template -struct RefPtr { - ~RefPtr() { GC(); } - void forget() {} -}; - -Cell* refptr_test1() { - static Cell cell; - RefPtr v1; - Cell* ref_unsafe1 = &cell; - return ref_unsafe1; -} - -Cell* refptr_test2() { - static Cell cell; - RefPtr v2; - Cell* ref_safe2 = &cell; - v2.forget(); - return ref_safe2; -} - -Cell* refptr_test3() { - static Cell cell; - RefPtr v3; - Cell* ref_unsafe3 = &cell; - if (x) { - v3.forget(); - } - return ref_unsafe3; -} - -Cell* refptr_test4() { - static Cell cell; - RefPtr r; - return &cell; // hazard in return value -} - -Cell* refptr_test5() { - static Cell cell; - RefPtr r; - return nullptr; // returning immobile value, so no hazard -} diff --git a/js/src/devtools/rootAnalysis/t/hazards/test.py b/js/src/devtools/rootAnalysis/t/hazards/test.py index e6d992f05f25..aa39187c2001 100644 --- a/js/src/devtools/rootAnalysis/t/hazards/test.py +++ b/js/src/devtools/rootAnalysis/t/hazards/test.py @@ -7,6 +7,7 @@ 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 @@ -25,11 +26,10 @@ assert "cell6" not in hazmap assert "" in hazmap assert "this" in hazmap -# All hazards should be in f(), loopy(), safevals(), method(), and refptr_test{1,3,4}() +# All hazards should be in f(), loopy(), safevals(), and method() assert hazmap["cell2"].function == "Cell* f()" -haz_functions = set(haz.function for haz in hazards) -print(haz_functions) -assert len(haz_functions) == 7 +print(len(set(haz.function for haz in hazards))) +assert len(set(haz.function for haz in hazards)) == 4 # 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 @@ -39,16 +39,8 @@ assert hazmap["cell3"].GCFunction in ( "void halfSuppressedFunction()", "void unsuppressedFunction()", ) -returnval_hazards = set( - haz.function for haz in hazards if haz.variable == "" -) -assert returnval_hazards == set( - [ - "Cell* f()", - "Cell* refptr_test1()", - "Cell* refptr_test3()", - "Cell* refptr_test4()", - ] +assert hazmap[""].GCFunction.startswith( + "void GCInDestructor::~GCInDestructor()" ) assert "container1" in hazmap diff --git a/js/src/devtools/rootAnalysis/utility.js b/js/src/devtools/rootAnalysis/utility.js index 35156c6de4b1..8461b782b0e6 100644 --- a/js/src/devtools/rootAnalysis/utility.js +++ b/js/src/devtools/rootAnalysis/utility.js @@ -14,7 +14,6 @@ loadRelativeToScript('dumpCFG.js'); var ATTR_GC_SUPPRESSED = 1; var ATTR_CANSCRIPT_BOUNDED = 2; // Unimplemented var ATTR_DOM_ITERATING = 4; // Unimplemented -var ATTR_NONRELEASING = 8; // ~RefPtr of value whose refcount will not go to zero var ATTRS_NONE = 0; var ATTRS_ALL = 7; // All possible bits set @@ -75,131 +74,64 @@ function xprint(x, padding) } } -// Command-line argument parser. -// -// `parameters` is a dict of parameters specs, each of which is a dict with keys: -// -// - name: name of option, prefixed with "--" if it is named (otherwise, it -// is interpreted as a positional parameter.) -// - dest: key to store the result in, defaulting to the parameter name without -// any leading "--"" and with dashes replaced with underscores. -// - default: value of option if no value is given. Positional parameters with -// a default value are optional. If no default is given, the parameter's name -// is not included in the return value. -// - type: `bool` if it takes no argument, otherwise an argument is required. -// Named arguments default to 'bool', positional arguments to 'string'. -// - nargs: the only supported value is `+`, which means to grab all following -// arguments, up to the next named option, and store them as a list. -// -// The command line is parsed for `--foo=value` and `--bar` arguments. -// -// Return value is a dict of parameter values, keyed off of `dest` as determined -// above. An extra option named "rest" will be set to the list of all remaining -// arguments passed in. -// function parse_options(parameters, inArgs = scriptArgs) { const options = {}; - const named = {}; + const optional = {}; const positional = []; for (const param of parameters) { if (param.name.startsWith("-")) { - named[param.name] = param; - if (!param.dest) { - if (!param.name.startsWith("--")) { - throw new Error(`parameter '${param.name}' requires param.dest to be set`); - } - param.dest = param.name.substring(2).replace("-", "_"); - } + optional[param.name] = param; + param.dest = param.dest || param.name.substring(2).replace("-", "_"); } else { - if (!('default' in param) && positional.length > 0 && ('default' in positional.at(-1))) { - throw new Error(`required parameter '${param.name}' follows optional parameter`); - } - param.positional = true; positional.push(param); param.dest = param.dest || param.name.replace("-", "_"); } - if (!param.type) { - if (param.nargs === "+") { - param.type = "list"; - } else if (param.positional) { - param.type = "string"; - } else { - param.type = "bool"; - } - } - - if ('default' in param) { + param.type = param.type || 'bool'; + if ('default' in param) options[param.dest] = param.default; - } } options.rest = []; const args = [...inArgs]; - let grabbing_into = undefined; while (args.length > 0) { - let arg = args.shift(); let param; - if (arg.startsWith("-") && arg in named) { - param = named[arg]; - if (param.type !== 'bool') { - if (args.length == 0) { - throw(new Error(`${param.name} requires an argument`)); - } - arg = args.shift(); - } - } else { - const pos = arg.indexOf("="); + let pos = -1; + if (args[0] in optional) + param = optional[args[0]]; + else { + pos = args[0].indexOf("="); if (pos != -1) { - const name = arg.substring(0, pos); - param = named[name]; - if (!param) { - throw(new Error(`Unknown option '${name}'`)); - } else if (param.type === 'bool') { - throw(new Error(`--${param.name} does not take an argument`)); - } - arg = arg.substring(pos + 1); + param = optional[args[0].substring(0, pos)]; + pos++; } } - // If this isn't a --named param, and we're not accumulating into a nargs="+" param, then - // use the next positional. - if (!param && !grabbing_into && positional.length > 0) { - param = positional.shift(); - } - - // If a parameter was identified, then any old accumulator is done and we might start a new one. - if (param) { - if (param.type === 'list') { - grabbing_into = options[param.dest] = options[param.dest] || []; + if (!param) { + if (positional.length > 0) { + param = positional.shift(); + options[param.dest] = args.shift(); } else { - grabbing_into = undefined; + options.rest.push(args.shift()); } + continue; } - if (grabbing_into) { - grabbing_into.push(arg); - } else if (param) { - if (param.type === 'bool') { - options[param.dest] = true; + if (param.type != 'bool') { + if (pos != -1) { + options[param.dest] = args.shift().substring(pos); } else { - options[param.dest] = arg; + args.shift(); + if (args.length == 0) + throw(new Error(`--${param.name} requires an argument`)); + options[param.dest] = args.shift(); } } else { - options.rest.push(arg); - } - } - - for (const param of positional) { - if (!('default' in param)) { - throw(new Error(`'${param.name}' option is required`)); - } - } - - for (const param of parameters) { - if (param.nargs === '+' && options[param.dest].length == 0) { - throw(new Error(`at least one value required for option '${param.name}'`)); + if (pos != -1) + throw(new Error(`--${param.name} does not take an argument`)); + options[param.dest] = true; + args.shift(); } } @@ -254,8 +186,13 @@ function collectBodyEdges(body) function getPredecessors(body) { - if (!('predecessors' in body)) - collectBodyEdges(body); + try { + if (!('predecessors' in body)) + collectBodyEdges(body); + } catch (e) { + debugger; + printErr("body is " + body); + } return body.predecessors; } diff --git a/js/src/vm/UbiNode.cpp b/js/src/vm/UbiNode.cpp index 25e772906fef..91521dbfafbd 100644 --- a/js/src/vm/UbiNode.cpp +++ b/js/src/vm/UbiNode.cpp @@ -361,35 +361,38 @@ const char16_t Concrete::concreteTypeName[] = namespace JS { namespace ubi { -RootList::RootList(JSContext* cx, bool wantNames /* = false */) - : cx(cx), edges(), wantNames(wantNames), inited(false) {} +RootList::RootList(JSContext* cx, Maybe& noGC, + bool wantNames /* = false */) + : noGC(noGC), cx(cx), edges(), wantNames(wantNames) {} -std::pair RootList::init() { +bool RootList::init() { EdgeVectorTracer tracer(cx->runtime(), &edges, wantNames); js::TraceRuntime(&tracer); - inited = tracer.okay; - return {tracer.okay, JS::AutoCheckCannotGC(cx)}; + if (!tracer.okay) { + return false; + } + noGC.emplace(); + return true; } -std::pair RootList::init( - CompartmentSet& debuggees) { +bool RootList::init(CompartmentSet& debuggees) { EdgeVector allRootEdges; EdgeVectorTracer tracer(cx->runtime(), &allRootEdges, wantNames); ZoneSet debuggeeZones; for (auto range = debuggees.all(); !range.empty(); range.popFront()) { if (!debuggeeZones.put(range.front()->zone())) { - return {false, JS::AutoCheckCannotGC(cx)}; + return false; } } js::TraceRuntime(&tracer); if (!tracer.okay) { - return {false, JS::AutoCheckCannotGC(cx)}; + return false; } js::gc::TraceIncomingCCWs(&tracer, debuggees); if (!tracer.okay) { - return {false, JS::AutoCheckCannotGC(cx)}; + return false; } for (EdgeVector::Range r = allRootEdges.all(); !r.empty(); r.popFront()) { @@ -406,14 +409,15 @@ std::pair RootList::init( } if (!edges.append(std::move(edge))) { - return {false, JS::AutoCheckCannotGC(cx)}; + return false; } } - return {true, JS::AutoCheckCannotGC(cx)}; + noGC.emplace(); + return true; } -std::pair RootList::init(HandleObject debuggees) { +bool RootList::init(HandleObject debuggees) { MOZ_ASSERT(debuggees && JS::dbg::IsDebugger(*debuggees)); js::Debugger* dbg = js::Debugger::fromJSObject(debuggees.get()); @@ -422,13 +426,12 @@ std::pair RootList::init(HandleObject debuggees) { for (js::WeakGlobalObjectSet::Range r = dbg->allDebuggees(); !r.empty(); r.popFront()) { if (!debuggeeCompartments.put(r.front()->compartment())) { - return {false, JS::AutoCheckCannotGC(cx)}; + return false; } } - auto [ok, nogc] = init(debuggeeCompartments); - if (!ok) { - return {false, nogc}; + if (!init(debuggeeCompartments)) { + return false; } // Ensure that each of our debuggee globals are in the root list. @@ -436,14 +439,15 @@ std::pair RootList::init(HandleObject debuggees) { r.popFront()) { if (!addRoot(JS::ubi::Node(static_cast(r.front())), u"debuggee global")) { - return {false, nogc}; + return false; } } - return {true, nogc}; + return true; } bool RootList::addRoot(Node node, const char16_t* edgeName) { + MOZ_ASSERT(noGC.isSome()); MOZ_ASSERT_IF(wantNames, edgeName); UniqueTwoByteChars name; diff --git a/js/src/vm/UbiNodeShortestPaths.cpp b/js/src/vm/UbiNodeShortestPaths.cpp index 25dc28590962..b0f2cdb58123 100644 --- a/js/src/vm/UbiNodeShortestPaths.cpp +++ b/js/src/vm/UbiNodeShortestPaths.cpp @@ -46,15 +46,16 @@ static void dumpNode(const JS::ubi::Node& node) { JS_PUBLIC_API void dumpPaths(JSContext* cx, Node node, uint32_t maxNumPaths /* = 10 */) { - JS::ubi::RootList rootList(cx, true); - auto [ok, nogc] = rootList.init(); - MOZ_ASSERT(ok); + mozilla::Maybe nogc; + + JS::ubi::RootList rootList(cx, nogc, true); + MOZ_ASSERT(rootList.init()); NodeSet targets; - ok = targets.putNew(node); + bool ok = targets.putNew(node); MOZ_ASSERT(ok); - auto paths = ShortestPaths::Create(cx, nogc, maxNumPaths, &rootList, + auto paths = ShortestPaths::Create(cx, nogc.ref(), maxNumPaths, &rootList, std::move(targets)); MOZ_ASSERT(paths.isSome());