Bug 1466458 part 2 - Refactor Realm::enterRealmDepth_ to account for Realms entered from JIT code. r=luke
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 07 Jun 2018 12:18:43 +0200
changeset 805262 60828108f4b8492d4de455e949be190b8e173af9
parent 805261 b968ba27d88c395cd6221545fbd412543cbedce2
child 805263 52c23f361adb710197c59fcb3f75dc7145b8ea09
push id112610
push userbmo:gl@mozilla.com
push dateThu, 07 Jun 2018 15:48:24 +0000
reviewersluke
bugs1466458
milestone62.0a1
Bug 1466458 part 2 - Refactor Realm::enterRealmDepth_ to account for Realms entered from JIT code. r=luke
js/src/gc/GC.cpp
js/src/vm/Debugger.cpp
js/src/vm/JSCompartment.cpp
js/src/vm/JSCompartment.h
js/src/vm/JSContext-inl.h
js/src/vm/JSFunction.cpp
js/src/vm/Stopwatch.cpp
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -4224,17 +4224,17 @@ RelazifyFunctions(Zone* zone, AllocKind 
 
 static bool
 ShouldCollectZone(Zone* zone, JS::gcreason::Reason reason)
 {
     // If we are repeating a GC because we noticed dead compartments haven't
     // been collected, then only collect zones containing those compartments.
     if (reason == JS::gcreason::COMPARTMENT_REVIVED) {
         for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next()) {
-            if (comp->scheduledForDestruction)
+            if (comp->gcState.scheduledForDestruction)
                 return true;
         }
 
         return false;
     }
 
     // Otherwise we only collect scheduled zones.
     if (!zone->isGCScheduled())
@@ -4293,24 +4293,27 @@ GCRuntime::prepareZonesForCollection(JS:
         zone->setPreservingCode(false);
     }
 
     // Discard JIT code more aggressively if the process is approaching its
     // executable code limit.
     bool canAllocateMoreCode = jit::CanLikelyAllocateMoreExecutableMemory();
 
     for (CompartmentsIter c(rt); !c.done(); c.next()) {
-        c->scheduledForDestruction = false;
-        c->maybeAlive = false;
+        c->gcState.scheduledForDestruction = false;
+        c->gcState.maybeAlive = false;
+        c->gcState.hasEnteredRealm = false;
         for (RealmsInCompartmentIter r(c); !r.done(); r.next()) {
             r->unmark();
             if (r->shouldTraceGlobal() || !r->zone()->isGCScheduled())
-                c->maybeAlive = true;
+                c->gcState.maybeAlive = true;
             if (shouldPreserveJITCode(r, currentTime, reason, canAllocateMoreCode))
                 r->zone()->setPreservingCode(true);
+            if (r->hasBeenEnteredIgnoringJit())
+                c->gcState.hasEnteredRealm = true;
         }
     }
 
     if (!cleanUpEverything && canAllocateMoreCode) {
         jit::JitActivationIterator activation(rt->mainContextFromOwnThread());
         if (!activation.done())
             activation->compartment()->zone()->setPreservingCode(true);
     }
@@ -4520,40 +4523,40 @@ GCRuntime::markCompartments()
      * allocation and read barriers during JS_TransplantObject and the like.
      */
 
     /* Propagate the maybeAlive flag via cross-compartment edges. */
 
     Vector<JSCompartment*, 0, js::SystemAllocPolicy> workList;
 
     for (CompartmentsIter comp(rt); !comp.done(); comp.next()) {
-        if (comp->maybeAlive) {
+        if (comp->gcState.maybeAlive) {
             if (!workList.append(comp))
                 return;
         }
     }
 
     while (!workList.empty()) {
         JSCompartment* comp = workList.popCopy();
         for (JSCompartment::NonStringWrapperEnum e(comp); !e.empty(); e.popFront()) {
             JSCompartment* dest = e.front().mutableKey().compartment();
-            if (dest && !dest->maybeAlive) {
-                dest->maybeAlive = true;
+            if (dest && !dest->gcState.maybeAlive) {
+                dest->gcState.maybeAlive = true;
                 if (!workList.append(dest))
                     return;
             }
         }
     }
 
     /* Set scheduleForDestruction based on maybeAlive. */
 
     for (GCCompartmentsIter comp(rt); !comp.done(); comp.next()) {
-        MOZ_ASSERT(!comp->scheduledForDestruction);
-        if (!comp->maybeAlive && !comp->zone()->isAtomsZone())
-            comp->scheduledForDestruction = true;
+        MOZ_ASSERT(!comp->gcState.scheduledForDestruction);
+        if (!comp->gcState.maybeAlive && !comp->zone()->isAtomsZone())
+            comp->gcState.scheduledForDestruction = true;
     }
 }
 
 void
 GCRuntime::updateMallocCountersOnGCStart()
 {
     // Update the malloc counters for any zones we are collecting.
     for (GCZonesIter zone(rt, WithAtoms); !zone.done(); zone.next())
@@ -6935,17 +6938,17 @@ GCRuntime::resetIncrementalGC(gc::AbortR
 
         break;
       }
 
       case State::Sweep: {
         marker.reset();
 
         for (CompartmentsIter c(rt); !c.done(); c.next())
-            c->scheduledForDestruction = false;
+            c->gcState.scheduledForDestruction = false;
 
         /* Finish sweeping the current sweep group, then abort. */
         abortSweepAfterCurrentGroup = true;
 
         /* Don't perform any compaction after sweeping. */
         bool wasCompacting = isCompacting;
         isCompacting = false;
 
@@ -7663,17 +7666,17 @@ GCRuntime::shouldRepeatForDeadZone(JS::g
 {
     MOZ_ASSERT_IF(reason == JS::gcreason::COMPARTMENT_REVIVED, !isIncremental);
     MOZ_ASSERT(!isIncrementalGCInProgress());
 
     if (!isIncremental)
         return false;
 
     for (CompartmentsIter c(rt); !c.done(); c.next()) {
-        if (c->scheduledForDestruction)
+        if (c->gcState.scheduledForDestruction)
             return true;
     }
 
     return false;
 }
 
 void
 GCRuntime::collect(bool nonincrementalByAPI, SliceBudget budget, JS::gcreason::Reason reason)
@@ -8092,17 +8095,17 @@ gc::MergeRealms(Realm* source, Realm* ta
 void
 GCRuntime::mergeRealms(Realm* source, Realm* target)
 {
     // The source realm must be specifically flagged as mergable.  This
     // also implies that the realm is not visible to the debugger.
     MOZ_ASSERT(source->creationOptions().mergeable());
     MOZ_ASSERT(source->creationOptions().invisibleToDebugger());
 
-    MOZ_ASSERT(!source->hasBeenEntered());
+    MOZ_ASSERT(!source->hasBeenEnteredIgnoringJit());
     MOZ_ASSERT(source->zone()->compartments().length() == 1);
 
     JSContext* cx = rt->mainContextFromOwnThread();
 
     MOZ_ASSERT(!source->zone()->wasGCStarted());
     JS::AutoAssertNoGC nogc(cx);
 
     AutoTraceSession session(rt);
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -3747,17 +3747,17 @@ Debugger::addDebuggee(JSContext* cx, uns
 /* static */ bool
 Debugger::addAllGlobalsAsDebuggees(JSContext* cx, unsigned argc, Value* vp)
 {
     THIS_DEBUGGER(cx, argc, vp, "addAllGlobalsAsDebuggees", args, dbg);
     for (ZonesIter zone(cx->runtime(), SkipAtoms); !zone.done(); zone.next()) {
         for (RealmsInZoneIter r(zone); !r.done(); r.next()) {
             if (r == dbg->object->realm() || r->creationOptions().invisibleToDebugger())
                 continue;
-            r->compartment()->scheduledForDestruction = false;
+            r->compartment()->gcState.scheduledForDestruction = false;
             GlobalObject* global = r->maybeGlobal();
             if (global) {
                 Rooted<GlobalObject*> rg(cx, global);
                 if (!dbg->addDebuggeeGlobal(cx, rg))
                     return false;
             }
         }
     }
@@ -4986,17 +4986,17 @@ Debugger::findAllGlobals(JSContext* cx, 
         // Accumulate the list of globals before wrapping them, because
         // wrapping can GC and collect realms from under us, while iterating.
         JS::AutoCheckCannotGC nogc;
 
         for (RealmsIter r(cx->runtime()); !r.done(); r.next()) {
             if (r->creationOptions().invisibleToDebugger())
                 continue;
 
-            r->compartment()->scheduledForDestruction = false;
+            r->compartment()->gcState.scheduledForDestruction = false;
 
             GlobalObject* global = r->maybeGlobal();
 
             if (cx->runtime()->isSelfHostingGlobal(global))
                 continue;
 
             if (global) {
                 /*
--- a/js/src/vm/JSCompartment.cpp
+++ b/js/src/vm/JSCompartment.cpp
@@ -70,16 +70,18 @@ Realm::Realm(JSCompartment* comp, const 
     MOZ_ASSERT_IF(creationOptions_.mergeable(),
                   creationOptions_.invisibleToDebugger());
 
     runtime_->numRealms++;
 }
 
 Realm::~Realm()
 {
+    MOZ_ASSERT(!hasBeenEnteredIgnoringJit());
+
     // Write the code coverage information in a file.
     JSRuntime* rt = runtimeFromMainThread();
     if (rt->lcovOutput().isEnabled())
         rt->lcovOutput().writeLCovResult(lcovOutput);
 
 #ifdef DEBUG
     // Avoid assertion destroying the unboxed layouts list if the embedding
     // leaked GC things.
--- a/js/src/vm/JSCompartment.h
+++ b/js/src/vm/JSCompartment.h
@@ -568,22 +568,33 @@ struct JSCompartment
      * The objects in the list are either cross-compartment wrappers, or
      * debugger wrapper objects.  The list link is either in the second extra
      * slot for the former, or a special slot for the latter.
      */
     JSObject* gcIncomingGrayPointers = nullptr;
 
     void* data = nullptr;
 
-    // These flags help us to discover if a compartment that shouldn't be alive
-    // manages to outlive a GC. Note that these flags have to be on the
-    // compartment, not the realm, because same-compartment realms can have
-    // cross-realm pointers without wrappers.
-    bool scheduledForDestruction = false;
-    bool maybeAlive = true;
+    // Fields set and used by the GC. Be careful, may be stale after we return
+    // to the mutator.
+    struct {
+        // These flags help us to discover if a compartment that shouldn't be
+        // alive manages to outlive a GC. Note that these flags have to be on
+        // the compartment, not the realm, because same-compartment realms can
+        // have cross-realm pointers without wrappers.
+        bool scheduledForDestruction = false;
+        bool maybeAlive = true;
+
+        // During GC, we may set this to |true| if we entered a realm in this
+        // compartment. Note that (without a stack walk) we don't know exactly
+        // *which* realms, because Realm::enterRealmDepthIgnoringJit_ does not
+        // account for cross-Realm calls in JIT code updating cx->realm_. See
+        // also the enterRealmDepthIgnoringJit_ comment.
+        bool hasEnteredRealm = false;
+    } gcState;
 
     JS::Zone* zone() { return zone_; }
     const JS::Zone* zone() const { return zone_; }
 
     JSRuntime* runtimeFromMainThread() const {
         MOZ_ASSERT(js::CurrentThreadCanAccessRuntime(runtime_));
         return runtime_;
     }
@@ -810,17 +821,31 @@ class JS::Realm : public JS::shadow::Rea
     // cx->enableAccessValidation is true, then we assert that *validAccessPtr
     // is true before running any code in this realm.
     bool* validAccessPtr_ = nullptr;
 
     js::ReadBarriered<js::ArgumentsObject*> mappedArgumentsTemplate_ { nullptr };
     js::ReadBarriered<js::ArgumentsObject*> unmappedArgumentsTemplate_ { nullptr };
     js::ReadBarriered<js::NativeObject*> iterResultTemplate_ { nullptr };
 
-    unsigned enterRealmDepth_ = 0;
+    // There are two ways to enter a realm:
+    //
+    // (1) AutoRealm (and JSAutoRealm, JS::EnterRealm)
+    // (2) When calling a cross-realm (but same-compartment) function in JIT
+    //     code.
+    //
+    // This field only accounts for (1), to keep the JIT code as simple as
+    // possible.
+    //
+    // An important invariant is that the JIT can only switch to a different
+    // realm within the same compartment, so whenever that happens there must
+    // always be a same-compartment realm with enterRealmDepthIgnoringJit_ > 0.
+    // This lets us set JSCompartment::hasEnteredRealm without walking the
+    // stack.
+    unsigned enterRealmDepthIgnoringJit_ = 0;
 
     enum {
         IsDebuggee = 1 << 0,
         DebuggerObservesAllExecution = 1 << 1,
         DebuggerObservesAsmJS = 1 << 2,
         DebuggerObservesCoverage = 1 << 3,
         DebuggerObservesBinarySource = 1 << 4,
         DebuggerNeedsDelazification = 1 << 5
@@ -1014,26 +1039,30 @@ class JS::Realm : public JS::shadow::Rea
     }
 
     // Whether the given name is in [[VarNames]].
     bool isInVarNames(JS::Handle<JSAtom*> name) {
         return varNames_.has(name);
     }
 
     void enter() {
-        enterRealmDepth_++;
+        enterRealmDepthIgnoringJit_++;
     }
     void leave() {
-        enterRealmDepth_--;
+        MOZ_ASSERT(enterRealmDepthIgnoringJit_ > 0);
+        enterRealmDepthIgnoringJit_--;
     }
-    bool hasBeenEntered() const {
-        return enterRealmDepth_ > 0;
+    bool hasBeenEnteredIgnoringJit() const {
+        return enterRealmDepthIgnoringJit_ > 0;
     }
     bool shouldTraceGlobal() const {
-        return hasBeenEntered();
+        // If we entered this realm in JIT code, there must be a script and
+        // function on the stack for this realm, so the global will definitely
+        // be traced and it's safe to return false here.
+        return hasBeenEnteredIgnoringJit();
     }
 
     bool hasAllocationMetadataBuilder() const {
         return allocationMetadataBuilder_;
     }
     const js::AllocationMetadataBuilder* getAllocationMetadataBuilder() const {
         return allocationMetadataBuilder_;
     }
@@ -1300,18 +1329,28 @@ class JS::Realm : public JS::shadow::Rea
 namespace js {
 
 // We only set the maybeAlive flag for objects and scripts. It's assumed that,
 // if a compartment is alive, then it will have at least some live object or
 // script it in. Even if we get this wrong, the worst that will happen is that
 // scheduledForDestruction will be set on the compartment, which will cause
 // some extra GC activity to try to free the compartment.
 template<typename T> inline void SetMaybeAliveFlag(T* thing) {}
-template<> inline void SetMaybeAliveFlag(JSObject* thing) {thing->compartment()->maybeAlive = true;}
-template<> inline void SetMaybeAliveFlag(JSScript* thing) {thing->compartment()->maybeAlive = true;}
+
+template<> inline void
+SetMaybeAliveFlag(JSObject* thing)
+{
+    thing->compartment()->gcState.maybeAlive = true;
+}
+
+template<> inline void
+SetMaybeAliveFlag(JSScript* thing)
+{
+    thing->compartment()->gcState.maybeAlive = true;
+}
 
 } // namespace js
 
 inline js::Handle<js::GlobalObject*>
 JSContext::global() const
 {
     /*
      * It's safe to use |unsafeGet()| here because any realm that is
--- a/js/src/vm/JSContext-inl.h
+++ b/js/src/vm/JSContext-inl.h
@@ -512,18 +512,18 @@ JSContext::leaveAtomsZone(JS::Realm* old
     setRealm(oldRealm);
 }
 
 inline void
 JSContext::setRealm(JS::Realm* realm)
 {
     // Both the current and the new realm should be properly marked as
     // entered at this point.
-    MOZ_ASSERT_IF(realm_, realm_->hasBeenEntered());
-    MOZ_ASSERT_IF(realm, realm->hasBeenEntered());
+    MOZ_ASSERT_IF(realm_, realm_->hasBeenEnteredIgnoringJit());
+    MOZ_ASSERT_IF(realm, realm->hasBeenEnteredIgnoringJit());
 
     // This thread must have exclusive access to the zone.
     MOZ_ASSERT_IF(realm, CurrentThreadCanAccessZone(realm->zone()));
 
     realm_ = realm;
     zone_ = realm ? realm->zone() : nullptr;
     arenas_ = zone_ ? &zone_->arenas : nullptr;
 }
--- a/js/src/vm/JSFunction.cpp
+++ b/js/src/vm/JSFunction.cpp
@@ -1674,20 +1674,24 @@ void
 JSFunction::maybeRelazify(JSRuntime* rt)
 {
     // Try to relazify functions with a non-lazy script. Note: functions can be
     // marked as interpreted despite having no script yet at some points when
     // parsing.
     if (!hasScript() || !u.scripted.s.script_)
         return;
 
-    // Don't relazify functions in realms that are active.
+    // Don't relazify functions in compartments that are active.
     Realm* realm = this->realm();
-    if (realm->hasBeenEntered() && !rt->allowRelazificationForTesting)
-        return;
+    if (!rt->allowRelazificationForTesting) {
+        if (realm->compartment()->gcState.hasEnteredRealm)
+            return;
+
+        MOZ_ASSERT(!realm->hasBeenEnteredIgnoringJit());
+    }
 
     // The caller should have checked we're not in the self-hosting zone (it's
     // shared with worker runtimes so relazifying functions in it will race).
     MOZ_ASSERT(!realm->isSelfHostingRealm());
 
     // Don't relazify if the realm is being debugged.
     if (realm->isDebuggee())
         return;
--- a/js/src/vm/Stopwatch.cpp
+++ b/js/src/vm/Stopwatch.cpp
@@ -232,17 +232,17 @@ AutoStopwatch::AutoStopwatch(JSContext* 
   , isMonitoringJank_(false)
   , isMonitoringCPOW_(false)
   , cyclesStart_(0)
   , CPOWTimeStart_(0)
 {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
 
     JSCompartment* compartment = cx_->compartment();
-    if (MOZ_UNLIKELY(compartment->scheduledForDestruction))
+    if (MOZ_UNLIKELY(compartment->gcState.scheduledForDestruction))
         return;
 
     JSRuntime* runtime = cx_->runtime();
     iteration_ = runtime->performanceMonitoring().iteration();
 
     const PerformanceGroupVector* groups = cx_->realm()->performanceMonitoring.getGroups(cx);
     if (!groups) {
       // Either the embedding has not provided any performance
@@ -271,17 +271,17 @@ AutoStopwatch::AutoStopwatch(JSContext* 
 AutoStopwatch::~AutoStopwatch()
 {
     if (groups_.length() == 0) {
         // We are not in charge of monitoring anything.
         return;
     }
 
     JSCompartment* compartment = cx_->compartment();
-    if (MOZ_UNLIKELY(compartment->scheduledForDestruction))
+    if (MOZ_UNLIKELY(compartment->gcState.scheduledForDestruction))
         return;
 
     JSRuntime* runtime = cx_->runtime();
     if (MOZ_UNLIKELY(iteration_ != runtime->performanceMonitoring().iteration())) {
         // We have entered a nested event loop at some point.
         // Any information we may have is obsolete.
         return;
     }