Bug 1500247 - [hazards] Ignore difficult-to-handle autogenerated DOM bindings code, r=jonco
authorSteve Fink <sfink@mozilla.com>
Wed, 09 Jan 2019 10:28:30 -0800
changeset 458988 7e7f4d97d178
parent 458987 5e40a711ed0a
child 458989 87ed9a736435
push id35553
push usershindli@mozilla.com
push dateThu, 14 Feb 2019 04:41:18 +0000
treeherdermozilla-central@f0ea53f47215 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1500247
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1500247 - [hazards] Ignore difficult-to-handle autogenerated DOM bindings code, r=jonco
js/src/devtools/rootAnalysis/analyzeRoots.js
--- a/js/src/devtools/rootAnalysis/analyzeRoots.js
+++ b/js/src/devtools/rootAnalysis/analyzeRoots.js
@@ -975,25 +975,66 @@ function processBodies(functionName)
                 if (tag == 'annotate')
                     annotations.add(value);
             }
         }
     }
 
     var missingExpectedHazard = annotations.has("Expect Hazards");
 
-    for (var variable of functionBodies[0].DefineVariable) {
+    // Awful special case, hopefully temporary:
+    //
+    // The DOM bindings code generator uses "holders" to externally root
+    // variables. So for example:
+    //
+    //       StringObjectRecordOrLong arg0;
+    //       StringObjectRecordOrLongArgument arg0_holder(arg0);
+    //       arg0_holder.TrySetToStringObjectRecord(cx, args[0]);
+    //       GC();
+    //       self->PassUnion22(cx, arg0);
+    //
+    // This appears to be a rooting hazard on arg0, but it is rooted by
+    // arg0_holder if you set it to any of its union types that requires
+    // rooting.
+    //
+    // Additionally, the holder may be reported as a hazard because it's not
+    // itself a Rooted or a subclass of AutoRooter; it contains a
+    // Maybe<RecordRooter<T>> that will get emplaced if rooting is required.
+    //
+    // 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<SomethingArgument>. It's a harsh world.
+    const ignoreVars = new Set();
+    if (functionName.match(/mozilla::dom::/)) {
+        const vars = functionBodies[0].DefineVariable.filter(
+            v => v.Type.Kind == 'CSU' && v.Variable.Kind == 'Local'
+        ).map(
+            v => [ v.Variable.Name[0], v.Type.Name ]
+        );
+
+        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 older.
+            ignoreVars.add(holder.replace("_holder", "")); // Ignore the "managed" arg.
+        }
+    }
+
+    for (const variable of functionBodies[0].DefineVariable) {
         var name;
         if (variable.Variable.Kind == "This")
             name = "this";
         else if (variable.Variable.Kind == "Return")
             name = "<returnvalue>";
         else
             name = variable.Variable.Name[0];
 
+        if (ignoreVars.has(name))
+            continue;
+
         if (isRootedType(variable.Type)) {
             if (!variableLiveAcrossGC(suppressed, variable.Variable)) {
                 // The earliest use of the variable should be its constructor.
                 var lineText;
                 for (var body of functionBodies) {
                     if (body.minimumUse) {
                         var text = findLocation(body, body.minimumUse);
                         if (!lineText || locationLine(lineText) > locationLine(text))