Bug 1402504 - Switch to using in-source annotations more directly, r=jonco

--HG--
extra : rebase_source : 477173b9c98df724b4a922de5ae7e3bda38cf76e

js/src/devtools/rootAnalysis/CFG.js

@@ -40,6 +40,14 @@ function isMatchingDestructor(constructor, edge)
     if (variable.Name[1].charAt(0) != '~')
         return false;
 
+    // Note that in some situations, a regular function can begin with '~', so
+    // we don't necessarily have a destructor in hand. This is probably a
+    // sixgill artifact, but in js::wasm::ModuleGenerator::~ModuleGenerator, a
+    // templatized static inline EraseIf is invoked, and it gets named ~EraseIf
+    // for some reason.
+    if (!("PEdgeCallInstance" in edge))
+        return false;
+
     var constructExp = constructor.PEdgeCallInstance.Exp;
     assert(constructExp.Kind == "Var");
 
@@ -55,7 +63,7 @@ function isMatchingDestructor(constructor, edge)
 // treat each instance separately, such as when different regions of a function
 // body were guarded by these constructors and you needed to do something
 // different with each.)
-function allRAIIGuardedCallPoints(bodies, body, isConstructor)
+function allRAIIGuardedCallPoints(typeInfo, bodies, body, isConstructor)
 {
     if (!("PEdge" in body))
         return [];
@@ -70,7 +78,7 @@ function allRAIIGuardedCallPoints(bodies, body, isConstructor)
             continue;
         var variable = callee.Variable;
         assert(variable.Kind == "Func");
-        if (!isConstructor(edge.Type, variable.Name))
+        if (!isConstructor(typeInfo, edge.Type, variable.Name))
             continue;
         if (!("PEdgeCallInstance" in edge))
             continue;

js/src/devtools/rootAnalysis/analyzeRoots.js

@@ -5,6 +5,7 @@
 loadRelativeToScript('utility.js');
 loadRelativeToScript('annotations.js');
 loadRelativeToScript('CFG.js');
+loadRelativeToScript('dumpCFG.js');
 
 var sourceRoot = (os.getenv('SOURCE') || '') + '/'
 
@@ -29,8 +30,6 @@ var batch = (scriptArgs[5]|0) || 1;
 var numBatches = (scriptArgs[6]|0) || 1;
 var tmpfile = scriptArgs[7] || "tmp.txt";
 
-GCSuppressionTypes = loadTypeInfo(typeInfoFile)["Suppress GC"] || [];
-
 var gcFunctions = {};
 var text = snarf("gcFunctions.lst").split("\n");
 assert(text.pop().length == 0);
@@ -45,6 +44,8 @@ for (var line of text) {
 }
 text = null;
 
+var typeInfo = loadTypeInfo(typeInfoFile);
+
 var gcEdges = {};
 text = snarf(gcEdgesFile).split('\n');
 assert(text.pop().length == 0);
@@ -749,7 +750,8 @@ function printEntryTrace(functionName, entry)
 
 function isRootedType(type)
 {
-    return type.Kind == "CSU" && isRootedTypeName(type.Name);
+    return type.Kind == "CSU" && ((type.Name in typeInfo.RootedPointers) ||
+                                  (type.Name in typeInfo.RootedGCThings));
 }
 
 function typeDesc(type)
@@ -841,7 +843,7 @@ function process(name, json) {
     for (var body of functionBodies)
         body.suppressed = [];
     for (var body of functionBodies) {
-        for (var [pbody, id] of allRAIIGuardedCallPoints(functionBodies, body, isSuppressConstructor))
+        for (var [pbody, id] of allRAIIGuardedCallPoints(typeInfo, functionBodies, body, isSuppressConstructor))
             pbody.suppressed[id] = true;
     }
     processBodies(functionName);

js/src/devtools/rootAnalysis/annotations.js

@@ -2,10 +2,6 @@
 
 "use strict";
 
-// RAII types within which we should assume GC is suppressed, eg
-// AutoSuppressGC.
-var GCSuppressionTypes = [];
-
 // Ignore calls made through these function pointers
 var ignoreIndirectCalls = {
     "mallocSizeOf" : true,
@@ -43,11 +39,20 @@ function indirectCallCannotGC(fullCaller, fullVariable)
         return true;
 
     // template method called during marking and hence cannot GC
-    if (name == "op" && caller.indexOf("bool js::WeakMap<Key, Value, HashPolicy>::keyNeedsMark(JSObject*)") != -1)
+    if (name == "op" && caller.includes("bool js::WeakMap<Key, Value, HashPolicy>::keyNeedsMark(JSObject*)"))
     {
         return true;
     }
 
+    // Call through a 'callback' function pointer, in a place where we're going
+    // to be throwing a JS exception.
+    if (name == "callback" && caller.includes("js::ErrorToException"))
+        return true;
+
+    // The math cache only gets called with non-GC math functions.
+    if (name == "f" && caller.includes("js::MathCache::lookup"))
+        return true;
+
     return false;
 }
 
@@ -218,6 +223,8 @@ var ignoreFunctions = {
     "uint32 js::TenuringTracer::moveObjectToTenured(JSObject*, JSObject*, int32)" : true,
     "void js::Nursery::freeMallocedBuffers()" : true,
 
+    "void js::AutoEnterOOMUnsafeRegion::crash(uint64, int8*)" : true,
+
     // It would be cool to somehow annotate that nsTHashtable<T> will use
     // nsTHashtable<T>::s_MatchEntry for its matchEntry function pointer, but
     // there is no mechanism for that. So we will just annotate a particularly
@@ -287,11 +294,11 @@ function ignoreGCFunction(mangled)
         return true;
 
     // Templatized function
-    if (fun.indexOf("void nsCOMPtr<T>::Assert_NoQueryNeeded()") >= 0)
+    if (fun.includes("void nsCOMPtr<T>::Assert_NoQueryNeeded()"))
         return true;
 
     // These call through an 'op' function pointer.
-    if (fun.indexOf("js::WeakMap<Key, Value, HashPolicy>::getDelegate(") >= 0)
+    if (fun.includes("js::WeakMap<Key, Value, HashPolicy>::getDelegate("))
         return true;
 
     // XXX modify refillFreeList<NoGC> to not need data flow analysis to understand it cannot GC.
@@ -307,9 +314,27 @@ function stripUCSAndNamespace(name)
     return name;
 }
 
-function isRootedGCTypeName(name)
+function extraRootedGCThings()
 {
-    return (name == "JSAddonId");
+    return [ 'JSAddonId' ];
+}
+
+function extraRootedPointers()
+{
+    return [
+        'ModuleValidator',
+        'JSErrorResult',
+        'WrappableJSErrorResult',
+
+        // These are not actually rooted, but are only used in the context of
+        // AutoKeepAtoms.
+        'js::frontend::TokenStream',
+        'js::frontend::TokenStream::Position',
+
+        'mozilla::ErrorResult',
+        'mozilla::IgnoredErrorResult',
+        'mozilla::dom::binding_detail::FastErrorResult',
+    ];
 }
 
 function isRootedGCPointerTypeName(name)
@@ -319,33 +344,16 @@ function isRootedGCPointerTypeName(name)
     if (name.startsWith('MaybeRooted<'))
         return /\(js::AllowGC\)1u>::RootType/.test(name);
 
-    if (name == "ErrorResult" ||
-        name == "JSErrorResult" ||
-        name == "WrappableJSErrorResult" ||
-        name == "binding_detail::FastErrorResult" ||
-        name == "IgnoredErrorResult" ||
-        name == "frontend::TokenStream" ||
-        name == "frontend::TokenStream::Position" ||
-        name == "ModuleValidator")
-    {
-        return true;
-    }
-
     return name.startsWith('Rooted') || name.startsWith('PersistentRooted');
 }
 
-function isRootedTypeName(name)
-{
-    return isRootedGCTypeName(name) || isRootedGCPointerTypeName(name);
-}
-
 function isUnsafeStorage(typeName)
 {
     typeName = stripUCSAndNamespace(typeName);
     return typeName.startsWith('UniquePtr<');
 }
 
-function isSuppressConstructor(edgeType, varName)
+function isSuppressConstructor(typeInfo, edgeType, varName)
 {
     // Check whether this could be a constructor
     if (edgeType.Kind != 'Function')
@@ -357,7 +365,7 @@ function isSuppressConstructor(edgeType, varName)
 
     // Check whether the type is a known suppression type.
     var type = edgeType.TypeFunctionCSU.Type.Name;
-    if (GCSuppressionTypes.indexOf(type) == -1)
+    if (!(type in typeInfo.GCSuppressors))
         return false;
 
     // And now make sure this is the constructor, not some other method on a

js/src/devtools/rootAnalysis/computeCallgraph.js

@@ -128,7 +128,7 @@ function processBody(functionName, body)
     }
 }
 
-GCSuppressionTypes = loadTypeInfo(typeInfo_filename)["Suppress GC"] || [];
+var typeInfo = loadTypeInfo(typeInfo_filename);
 
 loadTypes("src_comp.xdb");
 
@@ -155,7 +155,7 @@ function process(functionName, functionBodies)
         body.suppressed = [];
 
     for (var body of functionBodies) {
-        for (var [pbody, id] of allRAIIGuardedCallPoints(functionBodies, body, isSuppressConstructor))
+        for (var [pbody, id] of allRAIIGuardedCallPoints(typeInfo, functionBodies, body, isSuppressConstructor))
             pbody.suppressed[id] = true;
     }
 

js/src/devtools/rootAnalysis/computeGCTypes.js

@@ -8,12 +8,16 @@ loadRelativeToScript('annotations.js');
 var gcTypes_filename = scriptArgs[0] || "gcTypes.txt";
 var typeInfo_filename = scriptArgs[1] || "typeInfo.txt";
 
-var annotations = {
+var typeInfo = {
     'GCPointers': [],
     'GCThings': [],
     'NonGCTypes': {}, // unused
     'NonGCPointers': {},
+    'RootedGCThings': {},
     'RootedPointers': {},
+
+    // RAII types within which we should assume GC is suppressed, eg
+    // AutoSuppressGC.
     'GCSuppressors': {},
 };
 
@@ -22,6 +26,7 @@ var gDescriptors = new Map; // Map from descriptor string => Set of typeName
 var structureParents = {}; // Map from field => list of <parent, fieldName>
 var pointerParents = {}; // Map from field => list of <parent, fieldName>
 var baseClasses = {}; // Map from struct name => list of base class name strings
+var subClasses = {}; // Map from struct name => list of subclass  name strings
 
 var gcTypes = {}; // map from parent struct => Set of GC typed children
 var gcPointers = {}; // map from parent struct => Set of GC typed children
@@ -61,17 +66,17 @@ function processCSU(csu, body)
             continue;
 
         if (tag == 'GC Pointer')
-            annotations.GCPointers.push(csu);
+            typeInfo.GCPointers.push(csu);
         else if (tag == 'Invalidated by GC')
-            annotations.GCPointers.push(csu);
+            typeInfo.GCPointers.push(csu);
         else if (tag == 'GC Thing')
-            annotations.GCThings.push(csu);
+            typeInfo.GCThings.push(csu);
         else if (tag == 'Suppressed GC Pointer')
-            annotations.NonGCPointers[csu] = true;
+            typeInfo.NonGCPointers[csu] = true;
         else if (tag == 'Rooted Pointer')
-            annotations.RootedPointers[csu] = true;
+            typeInfo.RootedPointers[csu] = true;
         else if (tag == 'Suppress GC')
-            annotations.GCSuppressors[csu] = true;
+            typeInfo.GCSuppressors[csu] = true;
     }
 }
 
@@ -91,6 +96,9 @@ function addBaseClass(csu, base) {
     if (!(csu in baseClasses))
         baseClasses[csu] = [];
     baseClasses[csu].push(base);
+    if (!(base in subClasses))
+        subClasses[base] = [];
+    subClasses[base].push(csu);
     var k = baseClasses[csu].length;
     addNestedStructure(csu, base, `<base-${k}>`);
 }
@@ -119,22 +127,24 @@ for (var csuIndex = minStream; csuIndex <= maxStream; csuIndex++) {
     xdb.free_string(data);
 }
 
+for (const typename of extraRootedGCThings())
+    typeInfo.RootedGCThings[typename] = true;
+
+for (const typename of extraRootedPointers())
+    typeInfo.RootedPointers[typename] = true;
+
 // Now that we have the whole hierarchy set up, add all the types and propagate
 // info.
-for (let csu of annotations.GCThings)
+for (const csu of typeInfo.GCThings)
     addGCType(csu);
-for (let csu of annotations.GCPointers)
+for (const csu of typeInfo.GCPointers)
     addGCPointer(csu);
 
-function stars(n) { return n ? '*' + stars(n-1) : '' };
-
 // "typeName is a (pointer to a)^'typePtrLevel' GC type because it contains a field
 // named 'child' of type 'why' (or pointer to 'why' if fieldPtrLevel == 1), which is
 // itself a GCThing or GCPointer."
 function markGCType(typeName, child, why, typePtrLevel, fieldPtrLevel, indent)
 {
-    //printErr(`${indent}${typeName}${stars(typePtrLevel)} may be a gctype/ptr because of its child '${child}' of type ${why}${stars(fieldPtrLevel)}`);
-
     // Some types, like UniquePtr, do not mark/trace/relocate their contained
     // pointers and so should not hold them live across a GC. UniquePtr in
     // particular should be the only thing pointing to a structure containing a
@@ -166,19 +176,22 @@ function markGCType(typeName, child, why, typePtrLevel, fieldPtrLevel, indent)
     if (ptrLevel > 2)
         return;
 
-    if (ptrLevel == 0 && isRootedGCTypeName(typeName))
+    if (isRootedGCPointerTypeName(typeName) && !(typeName in typeInfo.RootedPointers))
+        printErr("FIXME: use in-source annotation for " + typeName);
+
+    if (ptrLevel == 0 && (typeName in typeInfo.RootedGCThings))
         return;
-    if (ptrLevel == 1 && isRootedGCPointerTypeName(typeName))
+    if (ptrLevel == 1 && (isRootedGCPointerTypeName(typeName) || (typeName in typeInfo.RootedPointers)))
         return;
 
     if (ptrLevel == 0) {
-        if (typeName in annotations.NonGCTypes)
+        if (typeName in typeInfo.NonGCTypes)
             return;
         if (!(typeName in gcTypes))
             gcTypes[typeName] = new Set();
         gcTypes[typeName].add(why);
     } else if (ptrLevel == 1) {
-        if (typeName in annotations.NonGCPointers)
+        if (typeName in typeInfo.NonGCPointers)
             return;
         if (!(typeName in gcPointers))
             gcPointers[typeName] = new Set();
@@ -215,28 +228,34 @@ function addGCPointer(typeName)
     markGCType(typeName, '<pointer-annotation>', '(annotation)', 1, 0, "");
 }
 
-// Add an arbitrary descriptor to a type, and apply it recursively to all base
+// Call a function for a type and every type that contains the type in a field
-// structs and structs that contain the given typeName as a field.
+// or as a base class (which internally is pretty much the same thing --
-function addDescriptor(typeName, descriptor)
+// sublcasses are structs beginning with the base class and adding on their
+// local fields.)
+function foreachContainingStruct(typeName, func, seen = new Set())
 {
-    if (!gDescriptors.has(descriptor))
+    function recurse(container, typeName) {
-        gDescriptors.set(descriptor, new Set);
+        if (seen.has(typeName))
-    let descriptorTypes = gDescriptors.get(descriptor);
+            return;
-    if (!descriptorTypes.has(typeName)) {
+        seen.add(typeName);
-        descriptorTypes.add(typeName);
+
-        if (typeName in structureParents) {
+        func(container, typeName);
-            for (let [holder, field] of structureParents[typeName])
+
-                addDescriptor(holder, descriptor);
+        if (typeName in subClasses) {
+            for (const sub of subClasses[typeName])
+                recurse("subclass of " + typeName, sub);
         }
-        if (typeName in baseClasses) {
+        if (typeName in structureParents) {
-            for (let base of baseClasses[typeName])
+            for (const [holder, field] of structureParents[typeName])
-                addDescriptor(base, descriptor);
+                recurse(field + " : " + typeName, holder);
         }
     }
+
+    recurse('<annotation>', typeName);
 }
 
 for (var type of listNonGCPointers())
-    annotations.NonGCPointers[type] = true;
+    typeInfo.NonGCPointers[type] = true;
 
 function explain(csu, indent, seen) {
     if (!seen)
@@ -288,12 +307,14 @@ for (var csu in gcPointers) {
 // Redirect output to the typeInfo file and close the gcTypes file.
 os.file.close(os.file.redirect(typeInfo_filename));
 
-for (let csu in annotations.GCSuppressors)
+// Compute the set of types that suppress GC within their RAII scopes (eg
-    addDescriptor(csu, 'Suppress GC');
+// AutoSuppressGC, AutoSuppressGCForAnalysis).
+var seen = new Set();
+for (let csu in typeInfo.GCSuppressors)
+    foreachContainingStruct(csu,
+                            (holder, typeName) => { typeInfo.GCSuppressors[typeName] = holder },
+                            seen);
 
-for (let [descriptor, types] of gDescriptors) {
+print(JSON.stringify(typeInfo, null, 4));
-    for (let csu of types)
-        print(descriptor + "$$" + csu);
-}
 
 os.file.close(os.file.redirect(origOut));

js/src/devtools/rootAnalysis/dumpCFG.js

@@ -241,7 +241,10 @@ function str_edge(edge, env) {
 }
 
 function str(unknown) {
-  if ("Index" in unknown) {
+  if ("Name" in unknown) {
+    return str_Variable(unknown);
+  } else if ("Index" in unknown) {
+    // Note: Variable also has .Index, with a different meaning.
     return str_edge(unknown);
   } else if ("Kind" in unknown) {
     if ("BlockId" in unknown)

js/src/devtools/rootAnalysis/utility.js

@@ -267,11 +267,5 @@ function addToKeyedList(collection, key, entry)
 
 function loadTypeInfo(filename)
 {
-    var info = {};
+    return JSON.parse(os.file.readFile(filename));
-    for (var line of readFileLines_gen(filename)) {
-        line = line.replace(/\n/, "");
-        let [property, name] = line.split("$$");
-        addToKeyedList(info, property, name);
-    }
-    return info;
 }