Bug 934698 - Suppress nsISupports.{AddRef,Release} when called directly, r=bhackett
authorSteve Fink <sfink@mozilla.com>
Tue, 05 Nov 2013 09:10:31 -0800
changeset 169205 c4fc26fa3b1062ce862f9803978c3152baf8f104
parent 169204 e69975bf90027f19a454784f93baa3376fa3c704
child 169206 62601b803f21013372ffd87865a23c71030db66e
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs934698
milestone28.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 934698 - Suppress nsISupports.{AddRef,Release} when called directly, r=bhackett DONBUILD because NPOTB yet
js/src/devtools/rootAnalysis/analyzeRoots.js
js/src/devtools/rootAnalysis/annotations.js
js/src/devtools/rootAnalysis/computeCallgraph.js
js/src/devtools/rootAnalysis/explain.py
js/src/devtools/rootAnalysis/loadCallgraph.js
--- a/js/src/devtools/rootAnalysis/analyzeRoots.js
+++ b/js/src/devtools/rootAnalysis/analyzeRoots.js
@@ -231,17 +231,19 @@ function edgeCanGC(edge)
             return "'" + otherName + "'";
         return null;
     }
     assert(callee.Kind == "Drf");
     if (callee.Exp[0].Kind == "Fld") {
         var field = callee.Exp[0].Field;
         var csuName = field.FieldCSU.Type.Name;
         var fullFieldName = csuName + "." + field.Name[0];
-        return fieldCallCannotGC(csuName, fullFieldName) ? null : fullFieldName;
+        if (fieldCallCannotGC(csuName, fullFieldName))
+            return null;
+        return (fullFieldName in suppressedFunctions) ? null : fullFieldName;
     }
     assert(callee.Exp[0].Kind == "Var");
     var calleeName = callee.Exp[0].Variable.Name[0];
     return indirectCallCannotGC(functionName, calleeName) ? null : "*" + calleeName;
 }
 
 function variableUseFollowsGC(suppressed, variable, worklist)
 {
--- a/js/src/devtools/rootAnalysis/annotations.js
+++ b/js/src/devtools/rootAnalysis/annotations.js
@@ -64,18 +64,16 @@ var ignoreClasses = {
 };
 
 // Ignore calls through TYPE.FIELD, where TYPE is the class or struct name containing
 // a function pointer field named FIELD.
 var ignoreCallees = {
     "js::Class.trace" : true,
     "js::Class.finalize" : true,
     "JSRuntime.destroyPrincipals" : true,
-    "nsISupports.AddRef" : true,
-    "nsISupports.Release" : true, // makes me a bit nervous; this is a bug but can happen
     "nsIGlobalObject.GetGlobalJSObject" : true, // virtual but no implementation can GC
     "nsAXPCNativeCallContext.GetJSContext" : true,
     "js::jit::MDefinition.op" : true, // macro generated virtuals just return a constant
     "js::jit::MDefinition.opName" : true, // macro generated virtuals just return a constant
     "js::jit::LInstruction.getDef" : true, // virtual but no implementation can GC
     "js::jit::IonCache.kind" : true, // macro generated virtuals just return a constant
     "icu_50::UObject.__deleting_dtor" : true, // destructors in ICU code can't cause GC
     "mozilla::CycleCollectedJSRuntime.DescribeCustomObjects" : true, // During tracing, cannot GC.
--- a/js/src/devtools/rootAnalysis/computeCallgraph.js
+++ b/js/src/devtools/rootAnalysis/computeCallgraph.js
@@ -44,29 +44,31 @@ function processCSU(csuName, csu)
             var key = csuName + ":" + field.Field[0].Name[0];
             if (!(key in classFunctions))
                 classFunctions[key] = [];
             classFunctions[key].push(name);
         }
     }
 }
 
-function findVirtualFunctions(csu, field)
+function findVirtualFunctions(csu, field, suppressed)
 {
     var worklist = [csu];
 
     // Virtual call targets on subclasses of nsISupports may be incomplete,
     // if the interface is scriptable. Just treat all indirect calls on
     // nsISupports objects as potentially GC'ing, except AddRef/Release
     // which should never enter the JS engine (even when calling dtors).
     while (worklist.length) {
         var csu = worklist.pop();
         if (csu == "nsISupports") {
-            if (field == "AddRef" || field == "Release")
+            if (field == "AddRef" || field == "Release") {
+                suppressed[0] = true;
                 return [];
+            }
             return null;
         }
         if (csu in superclasses) {
             for (var superclass of superclasses[csu])
                 worklist.push(superclass);
         }
     }
 
@@ -127,18 +129,26 @@ function getCallees(edge)
         }
     } else {
         assert(callee.Kind == "Drf");
         if (callee.Exp[0].Kind == "Fld") {
             var field = callee.Exp[0].Field;
             var fieldName = field.Name[0];
             var csuName = field.FieldCSU.Type.Name;
             var functions = null;
-            if ("FieldInstanceFunction" in field)
-                functions = findVirtualFunctions(csuName, fieldName);
+            if ("FieldInstanceFunction" in field) {
+                var suppressed = [ false ];
+                functions = findVirtualFunctions(csuName, fieldName, suppressed);
+                if (suppressed[0]) {
+                    // Field call known to not GC; mark it as suppressed so
+                    // direct invocations will be ignored
+                    callees.push({'kind': "field", 'csu': csuName, 'field': fieldName,
+                                  'suppressed': true});
+                }
+            }
             if (functions) {
                 // Known set of virtual call targets.
                 for (var name of functions)
                     callees.push({'kind': "direct", 'name': name});
             } else {
                 // Unknown set of call targets. Non-virtual field call,
                 // or virtual call on an nsISupports object.
                 callees.push({'kind': "field", 'csu': csuName, 'field': fieldName});
@@ -168,24 +178,25 @@ function processBody(caller, body)
 {
     if (!('PEdge' in body))
         return;
 
     lastline = null;
     for (var edge of body.PEdge) {
         if (edge.Kind != "Call")
             continue;
-        var suppressText = "";
+        var edgeSuppressed = false;
         var seen = seenCallees;
         if (edge.Index[0] in body.suppressed) {
-            suppressText = "SUPPRESS_GC ";
+            edgeSuppressed = true;
             seen = seenSuppressedCallees;
         }
-        var prologue = suppressText + memo(caller) + " ";
         for (var callee of getCallees(edge)) {
+            var prologue = (edgeSuppressed || callee.suppressed) ? "SUPPRESS_GC " : "";
+            prologue += memo(caller) + " ";
             if (callee.kind == 'direct') {
                 if (!(callee.name in seen)) {
                     seen[name] = true;
                     printOnce("D " + prologue + memo(callee.name));
                 }
             } else if (callee.kind == 'field') {
                 var { csu, field } = callee;
                 printOnce("F " + prologue + "CLASS " + csu + " FIELD " + field);
--- a/js/src/devtools/rootAnalysis/explain.py
+++ b/js/src/devtools/rootAnalysis/explain.py
@@ -43,19 +43,21 @@ try:
                 continue
 
             m = re.match(r'^Function.*takes unsafe address of unrooted', line)
             if m:
                 num_refs += 1
                 print >>refs, line
                 continue
 
-            m = re.match(r"^Function.*has unrooted.*of type.*live across GC call '(.*?)'", line)
+            m = re.match(r"^Function.*has unrooted.*of type.*live across GC call ('?)(.*?)('?) at \S+:\d+$", line)
             if m:
-                current_gcFunction = m.group(1)
+                # Function names are surrounded by single quotes. Field calls
+                # are unquoted.
+                current_gcFunction = m.group(2)
                 hazardousGCFunctions.setdefault(current_gcFunction, []).append(line)
                 hazardOrder.append((current_gcFunction, len(hazardousGCFunctions[current_gcFunction]) - 1))
                 num_hazards += 1
                 continue
 
             if current_gcFunction:
                 if not line.strip():
                     # Blank line => end of this hazard
@@ -79,17 +81,20 @@ try:
                         explanation = line
                 elif current_func:
                     explanation += line
             if current_func:
                 gcExplanations[current_func] = explanation
 
             for gcFunction, index in hazardOrder:
                 gcHazards = hazardousGCFunctions[gcFunction]
-                print >>hazards, (gcHazards[index] + gcExplanations[gcFunction])
+                if gcFunction in gcExplanations:
+                    print >>hazards, (gcHazards[index] + gcExplanations[gcFunction])
+                else:
+                    print >>hazards, gcHazards[index]
 
 except IOError as e:
     print 'Failed: %s' % str(e)
 
 print("Wrote %s" % args.hazards)
 print("Wrote %s" % args.extra)
 print("Wrote %s" % args.refs)
 print("Found %d hazards and %d unsafe references" % (num_hazards, num_refs))
--- a/js/src/devtools/rootAnalysis/loadCallgraph.js
+++ b/js/src/devtools/rootAnalysis/loadCallgraph.js
@@ -34,16 +34,18 @@ function addCallEdge(caller, callee, sup
         callerGraph[callee] = [];
     callerGraph[callee].push({caller:caller, suppressed:suppressed});
 }
 
 var functionNames = [""];
 
 function loadCallgraph(file)
 {
+    var suppressedFieldCalls = {};
+
     var textLines = snarf(file).split('\n');
     for (var line of textLines) {
         var match;
         if (match = /^\#(\d+) (.*)/.exec(line)) {
             assert(functionNames.length == match[1]);
             functionNames.push(match[2]);
             continue;
         }
@@ -57,17 +59,19 @@ function loadCallgraph(file)
             var caller = functionNames[match[1]];
             var name = match[2];
             if (!indirectCallCannotGC(caller, name) && !suppressed)
                 addGCFunction(caller, "IndirectCall: " + name);
         } else if (match = /^F (\d+) CLASS (.*?) FIELD (.*)/.exec(line)) {
             var caller = functionNames[match[1]];
             var csu = match[2];
             var fullfield = csu + "." + match[3];
-            if (!fieldCallCannotGC(csu, fullfield) && !suppressed)
+            if (suppressed)
+                suppressedFieldCalls[fullfield] = true;
+            else if (!fieldCallCannotGC(csu, fullfield))
                 addGCFunction(caller, "FieldCall: " + fullfield);
         } else if (match = /^D (\d+) (\d+)/.exec(line)) {
             var caller = functionNames[match[1]];
             var callee = functionNames[match[2]];
             addCallEdge(caller, callee, suppressed);
         }
     }
 
@@ -95,16 +99,20 @@ function loadCallgraph(file)
         }
     }
 
     for (var name in gcFunctions) {
         if (name in suppressedFunctions)
             delete gcFunctions[name];
     }
 
+    for (var name in suppressedFieldCalls) {
+        suppressedFunctions[name] = true;
+    }
+
     for (var gcName of [ 'jsgc.cpp:void Collect(JSRuntime*, uint8, int64, uint32, uint32)',
                          'void js::MinorGC(JSRuntime*, uint32)' ])
     {
         assert(gcName in callerGraph);
         addGCFunction(gcName, "GC");
     }
 
     var worklist = [];