Bug 1259850 - Refactor findVirtualFunctions and improve comments, r=terrence
☠☠ backed out by 69518db96a4d ☠ ☠
authorSteve Fink <sfink@mozilla.com>
Fri, 25 Mar 2016 15:25:56 -0700
changeset 338751 c6615c7b00838e1d212c6007b4a54b20d71f7ae0
parent 338750 196ac1f813f9a9489d24ba0c70c1dfb20b404489
child 338752 266befcb8acd57bc12a5949d6c873521d7ea6167
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1259850
milestone49.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 1259850 - Refactor findVirtualFunctions and improve comments, r=terrence No more passing a 2nd return value back through a mutable 1-element array. MozReview-Commit-ID: IUcyrq93KXT
js/src/devtools/rootAnalysis/computeCallgraph.js
--- a/js/src/devtools/rootAnalysis/computeCallgraph.js
+++ b/js/src/devtools/rootAnalysis/computeCallgraph.js
@@ -60,31 +60,39 @@ function nearestAncestorMethods(csu, met
     if (superclasses.has(csu)) {
         for (var parent of superclasses.get(csu))
             functions.update(nearestAncestorMethods(parent, method));
     }
 
     return functions;
 }
 
-function findVirtualFunctions(initialCSU, field, suppressed)
+// Return [ instantations, suppressed ], where instantiations is a Set of all
+// possible implementations of 'field' given static type 'initialCSU', plus
+// null if arbitrary other implementations are possible, and suppressed is true
+// if we the method is assumed to be non-GC'ing by annotation.
+function findVirtualFunctions(initialCSU, field)
 {
     var worklist = [initialCSU];
     var functions = new Set();
 
-    // 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).
+    // Loop through all methods of initialCSU (by looking at all methods of ancestor csus).
+    //
+    // If field is nsISupports::AddRef or ::Release, return an empty list and a
+    // boolean that says we assert that it cannot GC.
+    //
+    // If this is a method that is annotated to be dangerous (eg, it could be
+    // overridden with an implementation that could GC), then use null as a
+    // signal value that it should be considered to GC, even though we'll also
+    // collect all of the instantiations for other purposes.
+
     while (worklist.length) {
         var csu = worklist.pop();
-        if (isSuppressedVirtualMethod(csu, field)) {
-            suppressed[0] = true;
-            return new Set();
-        }
+        if (isSuppressedVirtualMethod(csu, field))
+            return [ new Set(), true ];
         if (isOverridableField(initialCSU, csu, field)) {
             // We will still resolve the virtual function call, because it's
             // nice to have as complete a callgraph as possible for other uses.
             // But push a token saying that we can run arbitrary code.
             functions.add(null);
         }
 
         if (superclasses.has(csu))
@@ -106,17 +114,17 @@ function findVirtualFunctions(initialCSU
 
         if (classFunctions.has(key))
             functions.update(classFunctions.get(key));
 
         if (subclasses.has(csu))
             worklist.push(...subclasses.get(csu));
     }
 
-    return functions;
+    return [ functions, false ];
 }
 
 var memoized = new Map();
 var memoizedCount = 0;
 
 function memo(name)
 {
     if (!memoized.has(name)) {
@@ -143,52 +151,52 @@ function getCallees(edge)
         assert(callee.Variable.Kind == "Func");
         callees.push({'kind': 'direct', 'name': callee.Variable.Name[0]});
     } 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;
+            var functions;
             if ("FieldInstanceFunction" in field) {
-                var suppressed = [ false ];
-                functions = findVirtualFunctions(csuName, fieldName, suppressed);
-                if (suppressed[0]) {
+                let suppressed;
+                [ functions, suppressed ] = findVirtualFunctions(csuName, fieldName, suppressed);
+                if (suppressed) {
                     // Field call known to not GC; mark it as suppressed so
                     // direct invocations will be ignored
                     callees.push({'kind': "field", 'csu': csuName, 'field': fieldName,
                                   'suppressed': true});
                 }
+            } else {
+                functions = new Set([null]); // field call
             }
-            if (functions) {
-                // Known set of virtual call targets. Treat them as direct
-                // calls to all possible resolved types, but also record edges
-                // from this field call to each final callee. When the analysis
-                // is checking whether an edge can GC and it sees an unrooted
-                // pointer held live across this field call, it will know
-                // whether any of the direct callees can GC or not.
-                var targets = [];
-                var fullyResolved = true;
-                for (var name of functions) {
-                    if (name === null) {
-                        // virtual call on an nsISupports object
-                        callees.push({'kind': "field", 'csu': csuName, 'field': fieldName});
-                        fullyResolved = false;
-                    } else {
-                        callees.push({'kind': "direct", 'name': name});
-                        targets.push({'kind': "direct", 'name': name});
-                    }
+
+            // Known set of virtual call targets. Treat them as direct calls to
+            // all possible resolved types, but also record edges from this
+            // field call to each final callee. When the analysis is checking
+            // whether an edge can GC and it sees an unrooted pointer held live
+            // across this field call, it will know whether any of the direct
+            // callees can GC or not.
+            var targets = [];
+            var fullyResolved = true;
+            for (var name of functions) {
+                if (name === null) {
+                    // Unknown set of call targets, meaning either a function
+                    // pointer call ("field call") or a virtual method that can
+                    // be overridden in extensions.
+                    callees.push({'kind': "field", 'csu': csuName, 'field': fieldName});
+                    fullyResolved = false;
+                } else {
+                    callees.push({'kind': "direct", 'name': name});
+                    targets.push({'kind': "direct", 'name': name});
                 }
-                if (fullyResolved)
-                    callees.push({'kind': "resolved-field", 'csu': csuName, 'field': fieldName, 'callees': targets});
-            } else {
-                // Unknown set of call targets. Non-virtual field call.
-                callees.push({'kind': "field", 'csu': csuName, 'field': fieldName});
             }
+            if (fullyResolved)
+                callees.push({'kind': "resolved-field", 'csu': csuName, 'field': fieldName, 'callees': targets});
         } else if (callee.Exp[0].Kind == "Var") {
             // indirect call through a variable.
             callees.push({'kind': "indirect", 'variable': callee.Exp[0].Variable.Name[0]});
         } else {
             // unknown call target.
             callees.push({'kind': "unknown"});
         }
     }
@@ -223,17 +231,16 @@ function getAnnotations(body)
 
     return all_annotations;
 }
 
 function getTags(functionName, body) {
     var tags = new Set();
     var annotations = getAnnotations(body);
     if (functionName in annotations) {
-        print("crawling through");
         for (var [ annName, annValue ] of annotations[functionName]) {
             if (annName == 'Tag')
                 tags.add(annValue);
         }
     }
     return tags;
 }