Bug 841356 - GC: Some minor rooting fixes r=sfink

js/src/ion/Bailouts.cpp

@@ -247,8 +247,6 @@ ConvertFrames(JSContext *cx, IonActivation *activation, IonBailoutIterator &it)
     }
 #endif
 
-    SnapshotIterator iter(it);
-
     // Set a flag to avoid bailing out on every iteration or function call. Ion can
     // compile and run the script again after an invalidation.
     it.ionScript()->setBailoutExpected();
@@ -296,6 +294,8 @@ ConvertFrames(JSContext *cx, IonActivation *activation, IonBailoutIterator &it)
     if (it.isConstructing())
         fp->setConstructing();
 
+    SnapshotIterator iter(it);
+
     while (true) {
         IonSpew(IonSpew_Bailouts, " restoring frame");
         fp->initFromBailout(cx, iter);

js/src/jsapi.cpp

@@ -1582,7 +1582,7 @@ JS_TransplantObject(JSContext *cx, JSObject *origobjArg, JSObject *targetArg)
         // destination, then we know that we won't find a wrapper in the
         // destination's cross compartment map and that the same
         // object will continue to work.
-        if (!origobj->swap(cx, target))
+        if (!JSObject::swap(cx, origobj, target))
             MOZ_CRASH();
         newIdentity = origobj;
     } else if (WrapperMap::Ptr p = destination->lookupWrapper(origv)) {
@@ -1596,7 +1596,7 @@ JS_TransplantObject(JSContext *cx, JSObject *origobjArg, JSObject *targetArg)
         destination->removeWrapper(p);
         NukeCrossCompartmentWrapper(cx, newIdentity);
 
-        if (!newIdentity->swap(cx, target))
+        if (!JSObject::swap(cx, newIdentity, target))
             MOZ_CRASH();
     } else {
         // Otherwise, we use |target| for the new identity object.
@@ -1615,7 +1615,7 @@ JS_TransplantObject(JSContext *cx, JSObject *origobjArg, JSObject *targetArg)
         if (!JS_WrapObject(cx, newIdentityWrapper.address()))
             MOZ_CRASH();
         JS_ASSERT(Wrapper::wrappedObject(newIdentityWrapper) == newIdentity);
-        if (!origobj->swap(cx, newIdentityWrapper))
+        if (!JSObject::swap(cx, origobj, newIdentityWrapper))
             MOZ_CRASH();
         origobj->compartment()->putWrapper(ObjectValue(*newIdentity), origv);
     }
@@ -1671,7 +1671,7 @@ js_TransplantObjectWithWrapper(JSContext *cx,
         destination->removeWrapper(p);
         NukeCrossCompartmentWrapper(cx, newWrapper);
 
-        if (!newWrapper->swap(cx, targetwrapper))
+        if (!JSObject::swap(cx, newWrapper, targetwrapper))
             MOZ_CRASH();
     } else {
         // Otherwise, use the passed-in wrapper as the same-compartment wrapper.
@@ -1698,7 +1698,7 @@ js_TransplantObjectWithWrapper(JSContext *cx,
         // After the swap we have a possibly-live object that isn't dangerous,
         // and a possibly-dangerous object that isn't live.
         RootedObject reflectorGuts(cx, NewDeadProxyObject(cx, JS_GetGlobalForObject(cx, origobj)));
-        if (!reflectorGuts || !origobj->swap(cx, reflectorGuts))
+        if (!reflectorGuts || !JSObject::swap(cx, origobj, reflectorGuts))
             MOZ_CRASH();
 
         // Turn origwrapper into a CCW to the new object.
@@ -1706,7 +1706,7 @@ js_TransplantObjectWithWrapper(JSContext *cx,
         if (!JS_WrapObject(cx, wrapperGuts.address()))
             MOZ_CRASH();
         JS_ASSERT(Wrapper::wrappedObject(wrapperGuts) == targetobj);
-        if (!origwrapper->swap(cx, wrapperGuts))
+        if (!JSObject::swap(cx, origwrapper, wrapperGuts))
             MOZ_CRASH();
         origwrapper->compartment()->putWrapper(ObjectValue(*targetobj),
                                                ObjectValue(*origwrapper));

js/src/jscntxt.cpp

@@ -284,6 +284,7 @@ js::CloneFunctionAtCallsite(JSContext *cx, HandleFunction fun, HandleScript scri
         return NULL;
 
     Key key;
+    SkipRoot skipKey(cx, &key); /* Stop the analysis complaining about unrooted key. */
     key.script = script;
     key.offset = pc - script->code;
     key.original = fun;

js/src/jsobj.cpp

@@ -2012,29 +2012,26 @@ JSObject::TradeGuts(JSContext *cx, JSObject *a, JSObject *b, TradeGutsReserved &
 
 /* Use this method with extreme caution. It trades the guts of two objects. */
 bool
-JSObject::swap(JSContext *cx, JSObject *other_)
+JSObject::swap(JSContext *cx, HandleObject a, HandleObject b)
 {
-    RootedObject self(cx, this);
+    AutoMarkInDeadZone adc1(a->zone());
-    RootedObject other(cx, other_);
+    AutoMarkInDeadZone adc2(b->zone());
-
-    AutoMarkInDeadZone adc1(self->zone());
-    AutoMarkInDeadZone adc2(other->zone());
 
     // Ensure swap doesn't cause a finalizer to not be run.
-    JS_ASSERT(IsBackgroundFinalized(getAllocKind()) ==
+    JS_ASSERT(IsBackgroundFinalized(a->getAllocKind()) ==
-              IsBackgroundFinalized(other->getAllocKind()));
+              IsBackgroundFinalized(b->getAllocKind()));
-    JS_ASSERT(compartment() == other->compartment());
+    JS_ASSERT(a->compartment() == b->compartment());
 
-    unsigned r = NotifyGCPreSwap(this, other);
+    unsigned r = NotifyGCPreSwap(a, b);
 
     TradeGutsReserved reserved(cx);
-    if (!ReserveForTradeGuts(cx, this, other, reserved)) {
+    if (!ReserveForTradeGuts(cx, a, b, reserved)) {
-        NotifyGCPostSwap(other, this, r);
+        NotifyGCPostSwap(b, a, r);
         return false;
     }
-    TradeGuts(cx, this, other, reserved);
+    TradeGuts(cx, a, b, reserved);
 
-    NotifyGCPostSwap(this, other, r);
+    NotifyGCPostSwap(a, b, r);
     return true;
 }
 

js/src/jsobj.h

@@ -929,7 +929,7 @@ class JSObject : public js::ObjectImpl
 
     static bool thisObject(JSContext *cx, const js::Value &v, js::Value *vp);
 
-    bool swap(JSContext *cx, JSObject *other);
+    static bool swap(JSContext *cx, JS::HandleObject a, JS::HandleObject b);
 
     inline void initArrayClass();
 

js/src/jswrapper.cpp

@@ -1154,7 +1154,7 @@ js::RemapWrapper(JSContext *cx, JSObject *wobjArg, JSObject *newTargetArg)
         // Now, because we need to maintain object identity, we do a brain
         // transplant on the old object so that it contains the contents of the
         // new one.
-        if (!wobj->swap(cx, tobj))
+        if (!JSObject::swap(cx, wobj, tobj))
             MOZ_CRASH();
     }
 

js/src/vm/ObjectImpl.cpp

@@ -630,6 +630,7 @@ js::GetProperty(JSContext *cx, Handle<ObjectImpl*> obj, Handle<ObjectImpl*> rece
         }
 
         PropDesc desc;
+        PropDesc::AutoRooter rootDesc(cx, &desc);
         if (!GetOwnProperty(cx, current, pid, resolveFlags, &desc))
             return false;
 

js/src/vm/Stack.cpp

@@ -1118,11 +1118,11 @@ bool
 ContextStack::pushBailoutArgs(JSContext *cx, const ion::IonBailoutIterator &it, InvokeArgsGuard *iag)
 {
     unsigned argc = it.numActualArgs();
-    ion::SnapshotIterator s(it);
 
     if (!pushInvokeArgs(cx, argc, iag, DONT_REPORT_ERROR))
         return false;
 
+    ion::SnapshotIterator s(it);
     JSFunction *fun = it.callee();
     iag->setCallee(ObjectValue(*fun));