Bug 1259850 - In-source annotations for GC suppression, r=terrence
☠☠ backed out by 69518db96a4d ☠ ☠
authorSteve Fink <sfink@mozilla.com>
Thu, 19 May 2016 12:53:29 -0700
changeset 340767 129559d4ac621b3801e41ce10db1cb4b1a6786da
parent 340766 d00b9c8a7984a8e117eb191280b08b1aed879710
child 340768 5762a8fba027bb667a621deb50540ddd5a884193
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [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 - In-source annotations for GC suppression, r=terrence MozReview-Commit-ID: HaSt3RVV6CM
js/public/GCAPI.h
js/public/GCAnnotations.h
js/src/devtools/rootAnalysis/analyze.py
js/src/devtools/rootAnalysis/analyzeRoots.js
js/src/devtools/rootAnalysis/annotations.js
js/src/devtools/rootAnalysis/computeCallgraph.js
js/src/devtools/rootAnalysis/computeGCTypes.js
js/src/devtools/rootAnalysis/utility.js
js/src/jsapi-tests/testGCStoreBufferRemoval.cpp
js/src/jsgc.h
--- a/js/public/GCAPI.h
+++ b/js/public/GCAPI.h
@@ -546,17 +546,17 @@ class JS_PUBLIC_API(AutoAssertNoAlloc)
  *       that the hazard analysis is correct for that code, rather than relying
  *       on this class.
  */
 class JS_PUBLIC_API(AutoSuppressGCAnalysis) : public AutoAssertNoAlloc
 {
   public:
     AutoSuppressGCAnalysis() : AutoAssertNoAlloc() {}
     explicit AutoSuppressGCAnalysis(JSRuntime* rt) : AutoAssertNoAlloc(rt) {}
-};
+} JS_HAZ_GC_SUPPRESSED;
 
 /**
  * Assert that code is only ever called from a GC callback, disable the static
  * rooting hazard analysis and assert if any allocation that could potentially
  * trigger a GC occurs while this guard object is live.
  *
  * This is useful to make the static analysis ignore code that runs in GC
  * callbacks.
--- a/js/public/GCAnnotations.h
+++ b/js/public/GCAnnotations.h
@@ -34,20 +34,23 @@
 // pointer when it holds an exception (and it does its own rooting,
 // conditionally.)
 # define JS_HAZ_NON_GC_POINTER __attribute__((tag("Suppressed GC Pointer")))
 
 // Mark a function as something that runs a garbage collection, potentially
 // invalidating GC pointers.
 # define JS_HAZ_GC_CALL __attribute__((tag("GC Call")))
 
+# define JS_HAZ_GC_SUPPRESSED __attribute__((tag("Suppress GC")))
+
 #else
 
 # define JS_HAZ_GC_THING
 # define JS_HAZ_GC_POINTER
 # define JS_HAZ_ROOTED
 # define JS_HAZ_GC_INVALIDATED
 # define JS_HAZ_NON_GC_POINTER
 # define JS_HAZ_GC_CALL
+# define JS_HAZ_GC_SUPPRESSED
 
 #endif
 
 #endif /* js_GCAnnotations_h */
--- a/js/src/devtools/rootAnalysis/analyze.py
+++ b/js/src/devtools/rootAnalysis/analyze.py
@@ -67,16 +67,17 @@ def generate_hazards(config, outfilename
     jobs = []
     for i in range(int(config['jobs'])):
         command = fill(('%(js)s',
                         '%(analysis_scriptdir)s/analyzeRoots.js',
                         '%(gcFunctions_list)s',
                         '%(gcEdges)s',
                         '%(suppressedFunctions_list)s',
                         '%(gcTypes)s',
+                        '%(typeInfo)s',
                         str(i+1), '%(jobs)s',
                         'tmp.%s' % (i+1,)),
                        config)
         outfile = 'rootingHazards.%s' % (i+1,)
         output = open(outfile, 'w')
         if config['verbose']:
             print_command(command, outfile=outfile, env=env(config))
         jobs.append((command, Popen(command, stdout=output, env=env(config))))
@@ -108,27 +109,28 @@ JOBS = { 'dbs':
                '.'),
               ()),
 
          'list-dbs':
              (('ls', '-l'),
               ()),
 
          'callgraph':
-             (('%(js)s', '%(analysis_scriptdir)s/computeCallgraph.js'),
+             (('%(js)s', '%(analysis_scriptdir)s/computeCallgraph.js', '%(typeInfo)s'),
               'callgraph.txt'),
 
          'gcFunctions':
              (('%(js)s', '%(analysis_scriptdir)s/computeGCFunctions.js', '%(callgraph)s',
                '[gcFunctions]', '[gcFunctions_list]', '[gcEdges]', '[suppressedFunctions_list]'),
               ('gcFunctions.txt', 'gcFunctions.lst', 'gcEdges.txt', 'suppressedFunctions.lst')),
 
          'gcTypes':
-             (('%(js)s', '%(analysis_scriptdir)s/computeGCTypes.js',),
-              'gcTypes.txt'),
+             (('%(js)s', '%(analysis_scriptdir)s/computeGCTypes.js',
+               '[gcTypes]', '[typeInfo]'),
+              ('gcTypes.txt', 'typeInfo.txt')),
 
          'allFunctions':
              (('%(sixgill_bin)s/xdbkeys', 'src_body.xdb',),
               'allFunctions.txt'),
 
          'hazards':
              (generate_hazards, 'rootingHazards.txt'),
 
@@ -254,18 +256,18 @@ if 'ANALYZED_OBJDIR' in os.environ:
 
 if 'SOURCE' in os.environ:
     data['source'] = os.environ['SOURCE']
 if not data.get('source') and data.get('sixgill_bin'):
     path = subprocess.check_output(['sh', '-c', data['sixgill_bin'] + '/xdbkeys file_source.xdb | grep jsapi.cpp'])
     data['source'] = path.replace("/js/src/jsapi.cpp", "")
 
 steps = [ 'dbs',
+          'gcTypes',
           'callgraph',
-          'gcTypes',
           'gcFunctions',
           'allFunctions',
           'hazards',
           'explain' ]
 
 if args.list:
     for step in steps:
         command, outfilename = JOBS[step]
--- a/js/src/devtools/rootAnalysis/analyzeRoots.js
+++ b/js/src/devtools/rootAnalysis/analyzeRoots.js
@@ -7,31 +7,34 @@ loadRelativeToScript('annotations.js');
 loadRelativeToScript('CFG.js');
 
 var sourceRoot = (os.getenv('SOURCE') || '') + '/'
 
 var functionName;
 var functionBodies;
 
 if (typeof scriptArgs[0] != 'string' || typeof scriptArgs[1] != 'string')
-    throw "Usage: analyzeRoots.js [-f function_name] <gcFunctions.lst> <gcEdges.txt> <suppressedFunctions.lst> <gcTypes.txt> [start end [tmpfile]]";
+    throw "Usage: analyzeRoots.js [-f function_name] <gcFunctions.lst> <gcEdges.txt> <suppressedFunctions.lst> <gcTypes.txt> <typeInfo.txt> [start end [tmpfile]]";
 
 var theFunctionNameToFind;
 if (scriptArgs[0] == '--function') {
     theFunctionNameToFind = scriptArgs[1];
     scriptArgs = scriptArgs.slice(2);
 }
 
-var gcFunctionsFile = scriptArgs[0];
-var gcEdgesFile = scriptArgs[1];
-var suppressedFunctionsFile = scriptArgs[2];
-var gcTypesFile = scriptArgs[3];
-var batch = (scriptArgs[4]|0) || 1;
-var numBatches = (scriptArgs[5]|0) || 1;
-var tmpfile = scriptArgs[6] || "tmp.txt";
+var gcFunctionsFile = scriptArgs[0] || "gcFunctions.lst";
+var gcEdgesFile = scriptArgs[1] || "gcEdges.txt";
+var suppressedFunctionsFile = scriptArgs[2] || "suppressedFunctions.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";
+
+GCSuppressionTypes = loadTypeInfo(typeInfoFile)["Suppress GC"];
 
 var gcFunctions = {};
 var text = snarf("gcFunctions.lst").split("\n");
 assert(text.pop().length == 0);
 for (var line of text)
     gcFunctions[mangled(line)] = true;
 
 var suppressedFunctions = {};
--- a/js/src/devtools/rootAnalysis/annotations.js
+++ b/js/src/devtools/rootAnalysis/annotations.js
@@ -1,12 +1,16 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 4 -*- */
 
 "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,
     "aMallocSizeOf" : true,
     "_malloc_message" : true,
     "je_malloc_message" : true,
     "chunk_dalloc" : true,
     "chunk_alloc" : true,
@@ -181,16 +185,17 @@ var ignoreFunctions = {
 
     // FIXME!
     "NS_DebugBreak": true,
 
     // These are a little overzealous -- these destructors *can* GC if they end
     // up wrapping a pending exception. See bug 898815 for the heavyweight fix.
     "void js::AutoCompartment::~AutoCompartment(int32)" : true,
     "void JSAutoCompartment::~JSAutoCompartment(int32)" : true,
+    "void js::AutoClearTypeInferenceStateOnOOM::~AutoClearTypeInferenceStateOnOOM()" : true,
 
     // Bug 948646 - the only thing AutoJSContext's constructor calls
     // is an Init() routine whose entire body is covered with an
     // AutoSuppressGCAnalysis. AutoSafeJSContext is the same thing, just with
     // a different value for the 'aSafe' parameter.
     "void mozilla::AutoJSContext::AutoJSContext(mozilla::detail::GuardObjectNotifier*)" : true,
     "void mozilla::AutoSafeJSContext::~AutoSafeJSContext(int32)" : true,
 
@@ -317,23 +322,17 @@ function isUnsafeStorage(typeName)
 {
     typeName = stripUCSAndNamespace(typeName);
     return typeName.startsWith('UniquePtr<');
 }
 
 function isSuppressConstructor(varName)
 {
     // varName[1] contains the unqualified name
-    return [
-        "AutoSuppressGC",
-        "AutoAssertGCCallback",
-        "AutoEnterAnalysis",
-        "AutoSuppressGCAnalysis",
-        "AutoIgnoreRootingHazards"
-    ].indexOf(varName[1]) != -1;
+    return GCSuppressionTypes.indexOf(varName[1]) != -1;
 }
 
 // nsISupports subclasses' methods may be scriptable (or overridden
 // via binary XPCOM), and so may GC. But some fields just aren't going
 // to get overridden with something that can GC.
 function isOverridableField(initialCSU, csu, field)
 {
     if (csu != 'nsISupports')
--- a/js/src/devtools/rootAnalysis/computeCallgraph.js
+++ b/js/src/devtools/rootAnalysis/computeCallgraph.js
@@ -7,16 +7,18 @@ loadRelativeToScript('annotations.js');
 loadRelativeToScript('CFG.js');
 
 var theFunctionNameToFind;
 if (scriptArgs[0] == '--function') {
     theFunctionNameToFind = scriptArgs[1];
     scriptArgs = scriptArgs.slice(2);
 }
 
+var typeInfo_filename = scriptArgs[0] || "typeInfo.txt";
+
 var subclasses = new Map(); // Map from csu => set of immediate subclasses
 var superclasses = new Map(); // Map from csu => set of immediate superclasses
 var classFunctions = new Map(); // Map from "csu:name" => set of full method name
 
 var virtualResolutionsSeen = new Set();
 
 function addEntry(map, name, entry)
 {
@@ -301,16 +303,18 @@ function processBody(functionName, body)
             } else {
                 printErr("invalid " + callee.kind + " callee");
                 debugger;
             }
         }
     }
 }
 
+GCSuppressionTypes = loadTypeInfo(typeInfo_filename)["Suppress GC"];
+
 var xdb = xdbLibrary();
 xdb.open("src_comp.xdb");
 
 var minStream = xdb.min_data_stream();
 var maxStream = xdb.max_data_stream();
 
 for (var csuIndex = minStream; csuIndex <= maxStream; csuIndex++) {
     var csu = xdb.read_key(csuIndex);
--- a/js/src/devtools/rootAnalysis/computeGCTypes.js
+++ b/js/src/devtools/rootAnalysis/computeGCTypes.js
@@ -1,23 +1,29 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 4 -*- */
 
 "use strict";
 
 loadRelativeToScript('utility.js');
 loadRelativeToScript('annotations.js');
 
+var gcTypes_filename = scriptArgs[0] || "gcTypes.txt";
+var typeInfo_filename = scriptArgs[1] || "typeInfo.txt";
+
 var annotations = {
     'GCPointers': [],
     'GCThings': [],
     'NonGCTypes': {}, // unused
     'NonGCPointers': {},
     'RootedPointers': {},
+    'GCSuppressors': {},
 };
 
+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 gcTypes = {}; // map from parent struct => Set of GC typed children
 var gcPointers = {}; // map from parent struct => Set of GC typed children
 var gcFields = new Map;
 
@@ -59,16 +65,18 @@ function processCSU(csu, body)
         else if (tag == 'Invalidated by GC')
             annotations.GCPointers.push(csu);
         else if (tag == 'GC Thing')
             annotations.GCThings.push(csu);
         else if (tag == 'Suppressed GC Pointer')
             annotations.NonGCPointers[csu] = true;
         else if (tag == 'Rooted Pointer')
             annotations.RootedPointers[csu] = true;
+        else if (tag == 'Suppress GC')
+            annotations.GCSuppressors[csu] = true;
     }
 }
 
 // csu.field is of type inner
 function addNestedStructure(csu, inner, field)
 {
     if (!(inner in structureParents))
         structureParents[inner] = [];
@@ -202,16 +210,36 @@ function addGCType(typeName, child, why,
     markGCType(typeName, '<annotation>', '(annotation)', 0, 0, "");
 }
 
 function addGCPointer(typeName)
 {
     markGCType(typeName, '<pointer-annotation>', '(annotation)', 1, 0, "");
 }
 
+// Add an arbitrary descriptor to a type, and apply it recursively to all base
+// structs and structs that contain the given typeName as a field.
+function addDescriptor(typeName, descriptor)
+{
+    if (!gDescriptors.has(descriptor))
+        gDescriptors.set(descriptor, new Set);
+    let descriptorTypes = gDescriptors.get(descriptor);
+    if (!descriptorTypes.has(typeName)) {
+        descriptorTypes.add(typeName);
+        if (typeName in structureParents) {
+            for (let [holder, field] of structureParents[typeName])
+                addDescriptor(holder, descriptor);
+        }
+        if (typeName in baseClasses) {
+            for (let base of baseClasses[typeName])
+                addDescriptor(base, descriptor);
+        }
+    }
+}
+
 for (var type of listNonGCPointers())
     annotations.NonGCPointers[type] = true;
 
 function explain(csu, indent, seen) {
     if (!seen)
         seen = new Set();
     seen.add(csu);
     if (!gcFields.has(csu))
@@ -241,16 +269,31 @@ function explain(csu, indent, seen) {
         }
         msg += child;
         print(msg);
         if (!seen.has(child))
             explain(child, indent + "  ", seen);
     }
 }
 
+var origOut = os.file.redirect(gcTypes_filename);
+
 for (var csu in gcTypes) {
     print("GCThing: " + csu);
     explain(csu, "  ");
 }
 for (var csu in gcPointers) {
     print("GCPointer: " + csu);
     explain(csu, "  ");
 }
+
+// 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)
+    addDescriptor(csu, 'Suppress GC');
+
+for (let [descriptor, types] of gDescriptors) {
+    for (let csu of types)
+        print(descriptor + "$$" + csu);
+}
+
+os.file.close(os.file.redirect(origOut));
--- a/js/src/devtools/rootAnalysis/utility.js
+++ b/js/src/devtools/rootAnalysis/utility.js
@@ -186,8 +186,26 @@ function* readFileLines_gen(filename)
     if (fp.isNull())
         throw "Unable to open '" + filename + "'"
 
     while (libc.getline(linebuf.address(), bufsize.address(), fp) > 0)
         yield linebuf.readString();
     libc.fclose(fp);
     libc.free(ctypes.void_t.ptr(linebuf));
 }
+
+function addToKeyedList(collection, key, entry)
+{
+    if (!(key in collection))
+        collection[key] = [];
+    collection[key].push(entry);
+}
+
+function loadTypeInfo(filename)
+{
+    var info = {};
+    for (var line of readFileLines_gen(filename)) {
+        line = line.replace(/\n/, "");
+        let [property, name] = line.split("$$");
+        addToKeyedList(info, property, name);
+    }
+    return info;
+}
--- a/js/src/jsapi-tests/testGCStoreBufferRemoval.cpp
+++ b/js/src/jsapi-tests/testGCStoreBufferRemoval.cpp
@@ -11,17 +11,17 @@
 using namespace JS;
 using namespace js;
 
 struct AutoIgnoreRootingHazards {
     // Force a nontrivial destructor so the compiler sees the whole RAII scope
     static volatile int depth;
     AutoIgnoreRootingHazards() { depth++; }
     ~AutoIgnoreRootingHazards() { depth--; }
-};
+} JS_HAZ_GC_SUPPRESSED;
 volatile int AutoIgnoreRootingHazards::depth = 0;
 
 BEGIN_TEST(testGCStoreBufferRemoval)
 {
     // Sanity check - objects start in the nursery and then become tenured.
     JS_GC(cx->runtime());
     JS::RootedObject obj(cx, NurseryObject());
     CHECK(js::gc::IsInsideNursery(obj.get()));
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -1264,17 +1264,17 @@ MaybeVerifyBarriers(JSContext* cx, bool 
 #endif
 
 /*
  * Instances of this class set the |JSRuntime::suppressGC| flag for the duration
  * that they are live. Use of this class is highly discouraged. Please carefully
  * read the comment in vm/Runtime.h above |suppressGC| and take all appropriate
  * precautions before instantiating this class.
  */
-class MOZ_RAII AutoSuppressGC
+class MOZ_RAII JS_HAZ_GC_SUPPRESSED AutoSuppressGC
 {
     int32_t& suppressGC_;
 
   public:
     explicit AutoSuppressGC(ExclusiveContext* cx);
     explicit AutoSuppressGC(JSCompartment* comp);
     explicit AutoSuppressGC(JSRuntime* rt);