Bug 1479962 - Add JS_HAZ_ROOTED_BASE for AutoGCRooter, which assumes all descendants are rooted, r=jonco
authorSteve Fink <sfink@mozilla.com>
Tue, 24 Jul 2018 16:52:43 -0700
changeset 831070 5b595406855ef0ab1d65e43a408a252b4a476748
parent 831069 aedd937e5d6a64b48b1458e408e2124376c15409
child 831071 8b7adcbe197982d69af6308a78a178b419c6fa16
push id118868
push userbmo:zjz@zjz.name
push dateFri, 24 Aug 2018 07:04:39 +0000
reviewersjonco
bugs1479962
milestone63.0a1
Bug 1479962 - Add JS_HAZ_ROOTED_BASE for AutoGCRooter, which assumes all descendants are rooted, r=jonco
js/public/GCAnnotations.h
js/public/RootingAPI.h
js/src/devtools/rootAnalysis/computeGCTypes.js
--- a/js/public/GCAnnotations.h
+++ b/js/public/GCAnnotations.h
@@ -21,16 +21,22 @@
 // Mark a type as a rooted pointer, suitable for use on the stack (eg all
 // Rooted<T> instantiations should have this.)
 # define JS_HAZ_ROOTED __attribute__((tag("Rooted Pointer")))
 
 // Mark a type as something that should not be held live across a GC, but which
 // is not itself a GC pointer.
 # define JS_HAZ_GC_INVALIDATED __attribute__((tag("Invalidated by GC")))
 
+// Mark a class as a base class of rooted types, eg CustomAutoRooter. All
+// descendants of this class will be considered rooted, though classes that
+// merely contain these as a field member will not be. "Inherited" by
+// templatized types with MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS
+# define JS_HAZ_ROOTED_BASE __attribute__((tag("Rooted Base")))
+
 // Mark a type that would otherwise be considered a GC Pointer (eg because it
 // contains a JS::Value field) as a non-GC pointer. It is handled almost the
 // same in the analysis as a rooted pointer, except it will not be reported as
 // an unnecessary root if used across a GC call. This should rarely be used,
 // but makes sense for something like ErrorResult, which only contains a GC
 // pointer when it holds an exception (and it does its own rooting,
 // conditionally.)
 # define JS_HAZ_NON_GC_POINTER __attribute__((tag("Suppressed GC Pointer")))
@@ -47,16 +53,17 @@
 # define JS_HAZ_CAN_RUN_SCRIPT __attribute__((tag("Can run script")))
 
 #else
 
 # define JS_HAZ_GC_THING
 # define JS_HAZ_GC_POINTER
 # define JS_HAZ_ROOTED
 # define JS_HAZ_GC_INVALIDATED
+# define JS_HAZ_ROOTED_BASE
 # define JS_HAZ_NON_GC_POINTER
 # define JS_HAZ_GC_CALL
 # define JS_HAZ_GC_SUPPRESSED
 # define JS_HAZ_CAN_RUN_SCRIPT
 
 #endif
 
 #endif /* js_GCAnnotations_h */
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -940,17 +940,17 @@ class JS_PUBLIC_API(AutoGCRooter)
      * Discriminates actual subclass of this being used. The meaning is
      * indicated by the corresponding value in the Tag enum.
      */
     Tag tag_;
 
     /* No copy or assignment semantics. */
     AutoGCRooter(AutoGCRooter& ida) = delete;
     void operator=(AutoGCRooter& ida) = delete;
-};
+} JS_HAZ_ROOTED_BASE;
 
 namespace detail {
 
 /*
  * For pointer types, the TraceKind for tracing is based on the list it is
  * in (selected via MapTypeToRootKind), so no additional storage is
  * required here. Non-pointer types, however, share the same list, so the
  * function to call for tracing is stored adjacent to the struct. Since C++
--- a/js/src/devtools/rootAnalysis/computeGCTypes.js
+++ b/js/src/devtools/rootAnalysis/computeGCTypes.js
@@ -10,16 +10,17 @@ var typeInfo_filename = scriptArgs[1] ||
 
 var typeInfo = {
     'GCPointers': [],
     'GCThings': [],
     'NonGCTypes': {}, // unused
     'NonGCPointers': {},
     'RootedGCThings': {},
     'RootedPointers': {},
+    'RootedBases': {'JS::AutoGCRooter': true},
 
     // RAII types within which we should assume GC is suppressed, eg
     // AutoSuppressGC.
     'GCSuppressors': {},
 };
 
 var gDescriptors = new Map; // Map from descriptor string => Set of typeName
 
@@ -31,66 +32,65 @@ var subClasses = {}; // Map from struct 
 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;
 
 var rootedPointers = {};
 
 function processCSU(csu, body)
 {
+    for (let { 'Name': [ annType, tag ] } of (body.Annotation || [])) {
+        if (annType != 'Tag')
+            continue;
+
+        if (tag == 'GC Pointer')
+            typeInfo.GCPointers.push(csu);
+        else if (tag == 'Invalidated by GC')
+            typeInfo.GCPointers.push(csu);
+        else if (tag == 'GC Thing')
+            typeInfo.GCThings.push(csu);
+        else if (tag == 'Suppressed GC Pointer')
+            typeInfo.NonGCPointers[csu] = true;
+        else if (tag == 'Rooted Pointer')
+            typeInfo.RootedPointers[csu] = true;
+        else if (tag == 'Rooted Base')
+            typeInfo.RootedBases[csu] = true;
+        else if (tag == 'Suppress GC')
+            typeInfo.GCSuppressors[csu] = true;
+    }
+
     for (let { 'Base': base } of (body.CSUBaseClass || []))
         addBaseClass(csu, base);
 
     for (let field of (body.DataField || [])) {
         var type = field.Field.Type;
         var fieldName = field.Field.Name[0];
         if (type.Kind == "Pointer") {
             var target = type.Type;
             if (target.Kind == "CSU")
                 addNestedPointer(csu, target.Name, fieldName);
         }
         if (type.Kind == "Array") {
             var target = type.Type;
             if (target.Kind == "CSU")
                 addNestedStructure(csu, target.Name, fieldName);
         }
-        if (type.Kind == "CSU") {
-            // Ignore nesting in classes which are AutoGCRooters. We only consider
-            // types with fields that may not be properly rooted.
-            if (type.Name == "JS::AutoGCRooter" || type.Name == "JS::CustomAutoRooter")
-                return;
+        if (type.Kind == "CSU")
             addNestedStructure(csu, type.Name, fieldName);
-        }
-    }
-
-    for (let { 'Name': [ annType, tag ] } of (body.Annotation || [])) {
-        if (annType != 'Tag')
-            continue;
-
-        if (tag == 'GC Pointer')
-            typeInfo.GCPointers.push(csu);
-        else if (tag == 'Invalidated by GC')
-            typeInfo.GCPointers.push(csu);
-        else if (tag == 'GC Thing')
-            typeInfo.GCThings.push(csu);
-        else if (tag == 'Suppressed GC Pointer')
-            typeInfo.NonGCPointers[csu] = true;
-        else if (tag == 'Rooted Pointer')
-            typeInfo.RootedPointers[csu] = true;
-        else if (tag == 'Suppress GC')
-            typeInfo.GCSuppressors[csu] = true;
     }
 }
 
 // csu.field is of type inner
 function addNestedStructure(csu, inner, field)
 {
     if (!(inner in structureParents))
         structureParents[inner] = [];
 
+    // Skip fields that are really base classes, to avoid duplicating the base
+    // fields; addBaseClass already added a "base-N" name.
     if (field.match(/^field:\d+$/) && (csu in baseClasses) && (baseClasses[csu].indexOf(inner) != -1))
         return;
 
     structureParents[inner].push([ csu, field ]);
 }
 
 function addBaseClass(csu, base) {
     if (!(csu in baseClasses))
@@ -135,16 +135,26 @@ for (const typename of extraRootedPointe
 
 // Now that we have the whole hierarchy set up, add all the types and propagate
 // info.
 for (const csu of typeInfo.GCThings)
     addGCType(csu);
 for (const csu of typeInfo.GCPointers)
     addGCPointer(csu);
 
+// Everything that inherits from a "Rooted Base" is considered to be rooted.
+// This is for things like CustomAutoRooter and its subclasses.
+var basework = Object.keys(typeInfo.RootedBases);
+while (basework.length) {
+    const base = basework.pop();
+    typeInfo.RootedPointers[base] = true;
+    if (base in subClasses)
+        basework.push(...subClasses[base]);
+}
+
 // "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)
 {
     // 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
@@ -225,17 +235,17 @@ function addGCType(typeName, child, why,
 
 function addGCPointer(typeName)
 {
     markGCType(typeName, '<pointer-annotation>', '(annotation)', 1, 0, "");
 }
 
 // Call a function for a type and every type that contains the type in a field
 // or as a base class (which internally is pretty much the same thing --
-// sublcasses are structs beginning with the base class and adding on their
+// subclasses are structs beginning with the base class and adding on their
 // local fields.)
 function foreachContainingStruct(typeName, func, seen = new Set())
 {
     function recurse(container, typeName) {
         if (seen.has(typeName))
             return;
         seen.add(typeName);