Bug 1360372 - Avoid rooting hazard when entering atoms compartment (r=sfink)

This patch avoids a rooting hazard involving the atoms compartment. There is
code in jsatom.cpp that enters the atoms compartment while unrooted pointers
are on the stack. Now that enterZoneGroup can potentially yield, this leads to
a rooting hazard. This patch avoids the hazard by using a totally separate path
that avoids enterZoneGroup when entering the atoms compartment.

MozReview-Commit-ID: ExG1ofCbN8C

js/src/jscntxt.h

@@ -201,10 +201,14 @@ struct JSContext : public JS::RootingContext,
 #endif
 
   private:
-    // If |c| or |oldCompartment| is the atoms compartment, the
+    // We distinguish between entering the atoms compartment and all other
-    // |exclusiveAccessLock| must be held.
+    // compartments. Entering the atoms compartment requires a lock. Also, we
-    inline void enterCompartment(JSCompartment* c,
+    // don't call enterZoneGroup when entering the atoms compartment since that
-                                 const js::AutoLockForExclusiveAccess* maybeLock = nullptr);
+    // can induce GC hazards.
+    inline void enterNonAtomsCompartment(JSCompartment* c);
+    inline void enterAtomsCompartment(JSCompartment* c,
+                                      const js::AutoLockForExclusiveAccess& lock);
+
     friend class js::AutoCompartment;
 
   public:

js/src/jscntxtinlines.h

@@ -451,17 +451,27 @@ JSContext::runningWithTrustedPrincipals()
 }
 
 inline void
-JSContext::enterCompartment(
+JSContext::enterNonAtomsCompartment(JSCompartment* c)
-    JSCompartment* c,
-    const js::AutoLockForExclusiveAccess* maybeLock /* = nullptr */)
 {
     enterCompartmentDepth_++;
 
-    if (!c->zone()->isAtomsZone())
+    MOZ_ASSERT(!c->zone()->isAtomsZone());
-        enterZoneGroup(c->zone()->group());
+    enterZoneGroup(c->zone()->group());
 
     c->enter();
-    setCompartment(c, maybeLock);
+    setCompartment(c, nullptr);
+}
+
+inline void
+JSContext::enterAtomsCompartment(JSCompartment* c,
+                                 const js::AutoLockForExclusiveAccess& lock)
+{
+    enterCompartmentDepth_++;
+
+    MOZ_ASSERT(c->zone()->isAtomsZone());
+
+    c->enter();
+    setCompartment(c, &lock);
 }
 
 template <typename T>
@@ -469,7 +479,7 @@ inline void
 JSContext::enterCompartmentOf(const T& target)
 {
     MOZ_ASSERT(JS::CellIsNotGray(target));
-    enterCompartment(target->compartment(), nullptr);
+    enterNonAtomsCompartment(target->compartment());
 }
 
 inline void

js/src/jscompartment.h

@@ -1053,8 +1053,11 @@ class AutoCompartment
     JSCompartment* origin() const { return origin_; }
 
   protected:
+    inline AutoCompartment(JSContext* cx, JSCompartment* target);
+
+    // Used only for entering the atoms compartment.
     inline AutoCompartment(JSContext* cx, JSCompartment* target,
-                           AutoLockForExclusiveAccess* maybeLock = nullptr);
+                           AutoLockForExclusiveAccess& lock);
 
   private:
     AutoCompartment(const AutoCompartment&) = delete;

js/src/jscompartmentinlines.h

@@ -44,14 +44,27 @@ js::AutoCompartment::AutoCompartment(JSContext* cx, const T& target)
     cx_->enterCompartmentOf(target);
 }
 
-// Protected constructor that bypasses assertions in enterCompartmentOf.
+// Protected constructor that bypasses assertions in enterCompartmentOf. Used
+// only for entering the atoms compartment.
 js::AutoCompartment::AutoCompartment(JSContext* cx, JSCompartment* target,
-                                     js::AutoLockForExclusiveAccess* maybeLock /* = nullptr */)
+                                     js::AutoLockForExclusiveAccess& lock)
   : cx_(cx),
     origin_(cx->compartment()),
-    maybeLock_(maybeLock)
+    maybeLock_(&lock)
 {
-    cx_->enterCompartment(target, maybeLock);
+    MOZ_ASSERT(target->isAtomsCompartment());
+    cx_->enterAtomsCompartment(target, lock);
+}
+
+// Protected constructor that bypasses assertions in enterCompartmentOf. Should
+// not be used to enter the atoms compartment.
+js::AutoCompartment::AutoCompartment(JSContext* cx, JSCompartment* target)
+  : cx_(cx),
+    origin_(cx->compartment()),
+    maybeLock_(nullptr)
+{
+    MOZ_ASSERT(!target->isAtomsCompartment());
+    cx_->enterNonAtomsCompartment(target);
 }
 
 js::AutoCompartment::~AutoCompartment()
@@ -61,7 +74,7 @@ js::AutoCompartment::~AutoCompartment()
 
 js::AutoAtomsCompartment::AutoAtomsCompartment(JSContext* cx,
                                                js::AutoLockForExclusiveAccess& lock)
-  : AutoCompartment(cx, cx->atomsCompartment(lock), &lock)
+  : AutoCompartment(cx, cx->atomsCompartment(lock), lock)
 {}
 
 js::AutoCompartmentUnchecked::AutoCompartmentUnchecked(JSContext* cx, JSCompartment* target)