Bug 1118469 - Disallow storing unrooted pointer in memory pointed to by a UniquePtr, r=terrence

--HG--
extra : rebase_source : 43a15ea4d37948c10c24de5c79ea3194b17fb599

js/src/devtools/rootAnalysis/analyze.py

@@ -65,7 +65,7 @@ def print_command(command, outfile=None, env=None):
 
 def generate_hazards(config, outfilename):
     jobs = []
-    for i in range(config['jobs']):
+    for i in range(int(config['jobs'])):
         command = fill(('%(js)s',
                         '%(analysis_scriptdir)s/analyzeRoots.js',
                         '%(gcFunctions_list)s',
@@ -90,7 +90,7 @@ def generate_hazards(config, outfilename):
         raise subprocess.CalledProcessError(final_status, 'analyzeRoots.js')
 
     with open(outfilename, 'w') as output:
-        command = ['cat'] + [ 'rootingHazards.%s' % (i+1,) for i in range(config['jobs']) ]
+        command = ['cat'] + [ 'rootingHazards.%s' % (i+1,) for i in range(int(config['jobs'])) ]
         print_command(command, outfile=outfilename)
         subprocess.call(command, stdout=output)
 

js/src/devtools/rootAnalysis/annotations.js

@@ -200,7 +200,7 @@ function isRootedTypeName(name)
     return false;
 }
 
-function isRootedPointerTypeName(name)
+function stripUCSAndNamespace(name)
 {
     if (name.startsWith('struct '))
         name = name.substr(7);
@@ -216,6 +216,15 @@ function isRootedPointerTypeName(name)
         name = name.substr(4);
     if (name.startsWith('mozilla::dom::'))
         name = name.substr(14);
+    if (name.startsWith('mozilla::'))
+        name = name.substr(9);
+
+    return name;
+}
+
+function isRootedPointerTypeName(name)
+{
+    name = stripUCSAndNamespace(name);
 
     if (name.startsWith('MaybeRooted<'))
         return /\(js::AllowGC\)1u>::RootType/.test(name);
@@ -223,6 +232,12 @@ function isRootedPointerTypeName(name)
     return name.startsWith('Rooted') || name.startsWith('PersistentRooted');
 }
 
+function isUnsafeStorage(typeName)
+{
+    typeName = stripUCSAndNamespace(typeName);
+    return typeName.startsWith('UniquePtr<');
+}
+
 function isSuppressConstructor(name)
 {
     return name.indexOf("::AutoSuppressGC") != -1

js/src/devtools/rootAnalysis/computeGCTypes.js

@@ -65,63 +65,72 @@ var gcTypes = {}; // map from parent struct => Set of GC typed children
 var gcPointers = {}; // map from parent struct => Set of GC typed children
 var gcFields = {};
 
-function addGCType(typeName, child, why)
+// "typeName is a (pointer to a)*'depth' GC type because it contains a field
+// named 'child' of type 'why' (or pointer to 'why' if ptrdness == 1), which
+// itself a GCThing or GCPointer."
+function markGCType(typeName, child, why, depth, ptrdness)
 {
-    if (!why)
-        why = '<annotation>';
-    if (!child)
-        child = 'annotation';
+    // 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
+    // GCPointer, so nothing else can be tracing it and it'll die when the
+    // UniquePtr goes out of scope. So we say that a UniquePtr's memory is just
+    // as unsafe as the stack for storing GC pointers.
+    if (!ptrdness && isUnsafeStorage(typeName)) {
+        printErr("Unsafe! " + typeName);
+        // The UniquePtr itself is on the stack but when you dereference the
+        // contained pointer, you get to the unsafe memory that we are treating
+        // as if it were the stack (aka depth 0). Note that
+        // UniquePtr<UniquePtr<JSObject*>> is fine, so we don't want to just
+        // hardcode the depth.
+        ptrdness = -1;
+    }
 
-    if (isRootedTypeName(typeName))
+    depth += ptrdness;
+    if (depth > 2)
         return;
 
-    if (!(typeName in gcTypes))
-        gcTypes[typeName] = Set();
-    gcTypes[typeName].add(why);
+    if (depth == 0 && isRootedTypeName(typeName))
+        return;
+    if (depth == 1 && isRootedPointerTypeName(typeName))
+        return;
+
+    if (depth == 0) {
+        if (!(typeName in gcTypes))
+            gcTypes[typeName] = Set();
+        gcTypes[typeName].add(why);
+    } else if (depth == 1) {
+        if (!(typeName in gcPointers))
+            gcPointers[typeName] = Set();
+        gcPointers[typeName].add(why);
+    }
 
     if (!(typeName in gcFields))
         gcFields[typeName] = Map();
-    gcFields[typeName].set(why, child);
+    gcFields[typeName].set(why, [ child, ptrdness ]);
 
     if (typeName in structureParents) {
         for (var field of structureParents[typeName]) {
             var [ holderType, fieldName ] = field;
-            addGCType(holderType, typeName, fieldName);
+            markGCType(holderType, typeName, fieldName, depth, 0);
         }
     }
     if (typeName in pointerParents) {
         for (var field of pointerParents[typeName]) {
             var [ holderType, fieldName ] = field;
-            addGCPointer(holderType, typeName, fieldName);
+            markGCType(holderType, typeName, fieldName, depth, 1);
         }
     }
 }
 
-function addGCPointer(typeName, child, why)
+function addGCType(typeName, child, why, depth, ptrdness)
 {
-    if (!why)
-        why = '<annotation>';
-    if (!child)
-        child = 'annotation';
+    markGCType(typeName, 'annotation', '<annotation>', 0, 0);
+}
 
-    // Ignore types that are properly rooted.
-    if (isRootedPointerTypeName(typeName))
-        return;
-
-    if (!(typeName in gcPointers))
-        gcPointers[typeName] = Set();
-    gcPointers[typeName].add(why);
-
-    if (!(typeName in gcFields))
-        gcFields[typeName] = Map();
-    gcFields[typeName].set(why, child);
-
-    if (typeName in structureParents) {
-        for (var field of structureParents[typeName]) {
-            var [ holder, fieldName ] = field;
-            addGCPointer(holder, typeName, fieldName);
-        }
-    }
+function addGCPointer(typeName)
+{
+    markGCType(typeName, 'annotation', '<pointer-annotation>', 1, 0);
 }
 
 addGCType('JSObject');
@@ -133,6 +142,8 @@ addGCType('js::LazyScript');
 addGCType('js::ion::IonCode');
 addGCPointer('JS::Value');
 addGCPointer('jsid');
+
+// AutoCheckCannotGC should also not be held live across a GC function.
 addGCPointer('JS::AutoCheckCannotGC');
 
 function explain(csu, indent, seen) {
@@ -142,14 +153,26 @@ function explain(csu, indent, seen) {
     if (!(csu in gcFields))
         return;
     if (gcFields[csu].has('<annotation>')) {
-        print(indent + "because I said so");
+        print(indent + "which is a GCThing because I said so");
         return;
     }
-    for (var [ field, child ] of gcFields[csu]) {
+    if (gcFields[csu].has('<pointer-annotation>')) {
+        print(indent + "which is a GCPointer because I said so");
+        return;
+    }
+    for (var [ field, [ child, ptrdness ] ] of gcFields[csu]) {
         var inherit = "";
         if (field == "field:0")
             inherit = " (probably via inheritance)";
-        print(indent + "contains field '" + field + "' of type " + child + inherit);
+        var msg = indent + "contains field '" + field + "' ";
+        if (ptrdness == -1)
+            msg += "(with a pointer to unsafe storage) holding a ";
+        else if (ptrdness == 0)
+            msg += "of type ";
+        else
+            msg += "pointing to type ";
+        msg += child + inherit;
+        print(msg);
         if (!seen.has(child))
             explain(child, indent + "  ", seen);
     }

js/src/vm/Debugger.cpp

@@ -2895,13 +2895,16 @@ Debugger::construct(JSContext *cx, unsigned argc, Value *vp)
         obj->setReservedSlot(slot, proto->getReservedSlot(slot));
     obj->setReservedSlot(JSSLOT_DEBUG_MEMORY_INSTANCE, NullValue());
 
-    /* Construct the underlying C++ object. */
-    auto dbg = cx->make_unique<Debugger>(cx, obj.get());
-    if (!dbg || !dbg->init(cx))
-        return false;
+    Debugger *debugger;
+    {
+        /* Construct the underlying C++ object. */
+        auto dbg = cx->make_unique<Debugger>(cx, obj.get());
+        if (!dbg || !dbg->init(cx))
+            return false;
 
-    Debugger *debugger = dbg.release();
-    obj->setPrivate(debugger); // owns the released pointer
+        debugger = dbg.release();
+        obj->setPrivate(debugger); // owns the released pointer
+    }
 
     /* Add the initial debuggees, if any. */
     for (unsigned i = 0; i < args.length(); i++) {