Bug 1482423 part 1 - Assert compartments don't contain both system/non-system realms. r=luke
authorJan de Mooij <jdemooij@mozilla.com>
Sat, 11 Aug 2018 13:12:49 +0200
changeset 431102 9f73af1f3b2c6928a18cc01d64f55b75aa3c4bba
parent 431101 54fe148fced876aeb78488afdc2faf6de8c4a401
child 431103 e6b953e50fbf75c58fb122520e0abd09662b1019
push id34424
push useraciure@mozilla.com
push dateSat, 11 Aug 2018 21:54:22 +0000
treeherdermozilla-central@4bf146738292 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1482423
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1482423 part 1 - Assert compartments don't contain both system/non-system realms. r=luke
js/src/gc/GC.cpp
js/src/jit-test/tests/realms/basic.js
js/src/jsfriendapi.cpp
js/src/shell/js.cpp
js/src/vm/HelperThreads.cpp
js/src/vm/Realm.cpp
js/src/vm/Realm.h
js/src/vm/SelfHosting.cpp
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -8079,21 +8079,23 @@ js::NewRealm(JSContext* cx, JSPrincipals
         compHolder = cx->make_unique<JS::Compartment>(zone);
         if (!compHolder || !compHolder->init(cx))
             return nullptr;
 
         comp = compHolder.get();
     }
 
     UniquePtr<Realm> realm(cx->new_<Realm>(comp, options));
-    if (!realm || !realm->init(cx))
+    if (!realm || !realm->init(cx, principals))
         return nullptr;
 
-    // Set up the principals.
-    JS::SetRealmPrincipals(realm.get(), principals);
+    // Make sure we don't put system and non-system realms in the same
+    // compartment.
+    if (!compHolder)
+        MOZ_RELEASE_ASSERT(realm->isSystem() == IsSystemCompartment(comp));
 
     AutoLockGC lock(rt);
 
     // Reserve space in the Vectors before we start mutating them.
     if (!comp->realms().reserve(comp->realms().length() + 1) ||
         (compHolder && !zone->compartments().reserve(zone->compartments().length() + 1)) ||
         (zoneHolder && !rt->gc.zones().reserve(rt->gc.zones().length() + 1)))
     {
--- a/js/src/jit-test/tests/realms/basic.js
+++ b/js/src/jit-test/tests/realms/basic.js
@@ -19,8 +19,27 @@ function testCrossRealmProto() {
 
         var a = Reflect.construct(Array, [], g.Object);
         assertEq(Array.isArray(a), true);
         assertEq(objectGlobal(a), this);
         assertEq(a.__proto__, g.Object.prototype);
     }
 }
 testCrossRealmProto();
+
+function testSystemNonSystemRealms() {
+    var systemRealm = newGlobal({systemPrincipal: true});
+    var ex;
+    try {
+        var nonSystemRealm = newGlobal({sameCompartmentAs: systemRealm, principal: 10});
+    } catch(e) {
+        ex = e;
+    }
+    assertEq(ex.toString().includes("non-system realms"), true);
+    ex = null;
+    try {
+        systemRealm = newGlobal({systemPrincipal: true, sameCompartmentAs: this});
+    } catch(e) {
+        ex = e;
+    }
+    assertEq(ex.toString().includes("non-system realms"), true);
+}
+testSystemNonSystemRealms();
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -173,40 +173,35 @@ JS::GetRealmPrincipals(JS::Realm* realm)
 
 JS_FRIEND_API(void)
 JS::SetRealmPrincipals(JS::Realm* realm, JSPrincipals* principals)
 {
     // Short circuit if there's no change.
     if (principals == realm->principals())
         return;
 
-    // Any realm with the trusted principals -- and there can be
-    // multiple -- is a system realm.
+    // We'd like to assert that our new principals is always same-origin
+    // with the old one, but JSPrincipals doesn't give us a way to do that.
+    // But we can at least assert that we're not switching between system
+    // and non-system.
     const JSPrincipals* trusted = realm->runtimeFromMainThread()->trustedPrincipals();
     bool isSystem = principals && principals == trusted;
+    MOZ_RELEASE_ASSERT(realm->isSystem() == isSystem);
 
     // Clear out the old principals, if any.
     if (realm->principals()) {
         JS_DropPrincipals(TlsContext.get(), realm->principals());
         realm->setPrincipals(nullptr);
-        // We'd like to assert that our new principals is always same-origin
-        // with the old one, but JSPrincipals doesn't give us a way to do that.
-        // But we can at least assert that we're not switching between system
-        // and non-system.
-        MOZ_ASSERT(realm->isSystem() == isSystem);
     }
 
     // Set up the new principals.
     if (principals) {
         JS_HoldPrincipals(principals);
         realm->setPrincipals(principals);
     }
-
-    // Update the system flag.
-    realm->setIsSystem(isSystem);
 }
 
 JS_FRIEND_API(JSPrincipals*)
 JS_GetScriptPrincipals(JSScript* script)
 {
     return script->principals();
 }
 
@@ -345,22 +340,20 @@ JS_FRIEND_API(JS::Zone*)
 js::GetRealmZone(JS::Realm* realm)
 {
     return realm->zone();
 }
 
 JS_FRIEND_API(bool)
 js::IsSystemCompartment(JS::Compartment* comp)
 {
-    // Note: for now we assume a single realm per compartment. This API will
-    // hopefully go away once Gecko supports same-compartment realms. Another
-    // option is to return comp->zone()->isSystem here, but we'd have to make
-    // sure that's equivalent.
-    MOZ_RELEASE_ASSERT(comp->realms().length() == 1);
-
+    // Realms in the same compartment must either all be system realms or
+    // non-system realms. We assert this in NewRealm and SetRealmPrincipals,
+    // but do an extra sanity check here.
+    MOZ_ASSERT(comp->realms()[0]->isSystem() == comp->realms().back()->isSystem());
     return comp->realms()[0]->isSystem();
 }
 
 JS_FRIEND_API(bool)
 js::IsSystemRealm(JS::Realm* realm)
 {
     return realm->isSystem();
 }
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -5309,29 +5309,47 @@ NewGlobal(JSContext* cx, unsigned argc, 
         if (v.isObject())
             creationOptions.setExistingCompartment(UncheckedUnwrap(&v.toObject()));
 
         if (!JS_GetProperty(cx, opts, "disableLazyParsing", &v))
             return false;
         if (v.isBoolean())
             behaviors.setDisableLazyParsing(v.toBoolean());
 
+        if (!JS_GetProperty(cx, opts, "systemPrincipal", &v))
+            return false;
+        if (v.isBoolean()) {
+            principals = &ShellPrincipals::fullyTrusted;
+            JS_HoldPrincipals(principals);
+        }
+
         if (!JS_GetProperty(cx, opts, "principal", &v))
             return false;
         if (!v.isUndefined()) {
             uint32_t bits;
             if (!ToUint32(cx, v, &bits))
                 return false;
             principals = cx->new_<ShellPrincipals>(bits);
             if (!principals)
                 return false;
             JS_HoldPrincipals(principals);
         }
     }
 
+    if (creationOptions.compartmentSpecifier() == JS::CompartmentSpecifier::ExistingCompartment) {
+        JS::Compartment* comp = creationOptions.compartment();
+        bool isSystem = principals && principals == cx->runtime()->trustedPrincipals();
+        if (isSystem != IsSystemCompartment(comp)) {
+            JS_ReportErrorASCII(cx,
+                                "Cannot create system and non-system realms in the "
+                                "same compartment");
+            return false;
+        }
+    }
+
     RootedObject global(cx, NewGlobalObject(cx, options, principals));
     if (principals)
         JS_DropPrincipals(cx, principals);
     if (!global)
         return false;
 
     if (!JS_WrapObject(cx, &global))
         return false;
@@ -7283,23 +7301,25 @@ JS_FN_HELP("parseBin", BinParse, 1, 0,
 "         scripts, even if it's a top-level script that will only run once\n"
 "         (defaults to using them directly in scripts that will only run\n"
 "         once).\n"
 "      invisibleToDebugger: If true, the global will be invisible to the\n"
 "         debugger (default false)\n"
 "      disableLazyParsing: If true, don't create lazy scripts for functions\n"
 "         (default false).\n"
 "      principal: if present, its value converted to a number must be an\n"
-"         integer that fits in 32 bits; use that as the new compartment's\n"
+"         integer that fits in 32 bits; use that as the new realm's\n"
 "         principal. Shell principals are toys, meant only for testing; one\n"
 "         shell principal subsumes another if its set bits are a superset of\n"
 "         the other's. Thus, a principal of 0 subsumes nothing, while a\n"
 "         principals of ~0 subsumes all other principals. The absence of a\n"
 "         principal is treated as if its bits were 0xffff, for subsumption\n"
-"         purposes. If this property is omitted, supply no principal."),
+"         purposes. If this property is omitted, supply no principal.\n"
+"      systemPrincipal: If true, use the shell's trusted principals for the\n"
+"         new realm. This creates a realm that's marked as a 'system' realm."),
 
     JS_FN_HELP("nukeCCW", NukeCCW, 1, 0,
 "nukeCCW(wrapper)",
 "  Nuke a CrossCompartmentWrapper, which turns it into a DeadProxyObject."),
 
     JS_FN_HELP("nukeAllCCWs", NukeAllCCWs, 0, 0,
 "nukeAllCCWs()",
 "  Like nukeCCW, but for all CrossCompartmentWrappers targeting the current compartment."),
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -763,26 +763,18 @@ CreateGlobalForOffThreadParse(JSContext*
 
     creationOptions.setInvisibleToDebugger(true)
                    .setMergeable(true)
                    .setNewCompartmentAndZone();
 
     // Don't falsely inherit the host's global trace hook.
     creationOptions.setTrace(nullptr);
 
-    JSObject* obj = JS_NewGlobalObject(cx, &parseTaskGlobalClass, nullptr,
-                                       JS::DontFireOnNewGlobalHook, realmOptions);
-    if (!obj)
-        return nullptr;
-
-    Rooted<GlobalObject*> global(cx, &obj->as<GlobalObject>());
-
-    JS::SetRealmPrincipals(global->realm(), currentRealm->principals());
-
-    return global;
+    return JS_NewGlobalObject(cx, &parseTaskGlobalClass, currentRealm->principals(),
+                              JS::DontFireOnNewGlobalHook, realmOptions);
 }
 
 static bool
 QueueOffThreadParseTask(JSContext* cx, ParseTask* task)
 {
     AutoLockHelperThreadState lock;
 
     bool mustWait = OffThreadParsingMustWaitForGC(cx->runtime());
--- a/js/src/vm/Realm.cpp
+++ b/js/src/vm/Realm.cpp
@@ -99,17 +99,17 @@ ObjectRealm::init(JSContext* cx)
         return false;
 
     iteratorSentinel_ = std::move(sentinel);
     enumerators = iteratorSentinel_.get();
     return true;
 }
 
 bool
-Realm::init(JSContext* cx)
+Realm::init(JSContext* cx, JSPrincipals* principals)
 {
     /*
      * As a hack, we clear our timezone cache every time we create a new realm.
      * This ensures that the cache is always relatively fresh, but shouldn't
      * interfere with benchmarks that create tons of date objects (unless they
      * also create tons of iframes, which seems unlikely).
      */
     JS::ResetTimeZone();
@@ -119,16 +119,24 @@ Realm::init(JSContext* cx)
 
     if (!savedStacks_.init() ||
         !varNames_.init())
     {
         ReportOutOfMemory(cx);
         return false;
     }
 
+    if (principals) {
+        // Any realm with the trusted principals -- and there can be
+        // multiple -- is a system realm.
+        isSystem_ = (principals == cx->runtime()->trustedPrincipals());
+        JS_HoldPrincipals(principals);
+        principals_ = principals;
+    }
+
     return true;
 }
 
 jit::JitRuntime*
 JSRuntime::createJitRuntime(JSContext* cx)
 {
     MOZ_ASSERT(!jitRuntime_);
 
--- a/js/src/vm/Realm.h
+++ b/js/src/vm/Realm.h
@@ -446,17 +446,17 @@ class JS::Realm : public JS::shadow::Rea
 
     Realm(const Realm&) = delete;
     void operator=(const Realm&) = delete;
 
   public:
     Realm(JS::Compartment* comp, const JS::RealmOptions& options);
     ~Realm();
 
-    MOZ_MUST_USE bool init(JSContext* cx);
+    MOZ_MUST_USE bool init(JSContext* cx, JSPrincipals* principals);
     void destroy(js::FreeOp* fop);
     void clearTables();
 
     void addSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf,
                                 size_t* tiAllocationSiteTables,
                                 size_t* tiArrayTypeTables,
                                 size_t* tiObjectTypeTables,
                                 size_t* realmObject,
@@ -495,16 +495,17 @@ class JS::Realm : public JS::shadow::Rea
     /* Whether to preserve JIT code on non-shrinking GCs. */
     bool preserveJitCode() { return creationOptions_.preserveJitCode(); }
 
     bool isSelfHostingRealm() const {
         return isSelfHostingRealm_;
     }
     void setIsSelfHostingRealm() {
         isSelfHostingRealm_ = true;
+        isSystem_ = true;
     }
 
     /* The global object for this realm.
      *
      * Note: the global_ field is null briefly during GC, after the global
      * object is collected; but when that happens the Realm is destroyed during
      * the same GC.)
      *
@@ -674,29 +675,16 @@ class JS::Realm : public JS::shadow::Rea
         // `principals_`.
         performanceMonitoring.unlink();
         principals_ = principals;
     }
 
     bool isSystem() const {
         return isSystem_;
     }
-    void setIsSystem(bool isSystem) {
-        if (isSystem_ == isSystem)
-            return;
-
-        // If we change `isSystem*(`, we need to unlink immediately this realm
-        // from its PerformanceGroup. For one thing, the performance data we
-        // collect should not be improperly associated to a group to which we
-        // do not belong anymore. For another thing, we use `isSystem()` as part
-        // of the key to map realms to a `PerformanceGroup`, so if we do not
-        // unlink now, this will be too late once we have updated `isSystem_`.
-        performanceMonitoring.unlink();
-        isSystem_ = isSystem;
-    }
 
     // Used to approximate non-content code when reporting telemetry.
     bool isProbablySystemCode() const {
         return isSystem_;
     }
 
     static const size_t IterResultObjectValueSlot = 0;
     static const size_t IterResultObjectDoneSlot = 1;
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -2796,17 +2796,16 @@ JSRuntime::createSelfHostingGlobal(JSCon
 
     AutoRealmUnchecked ar(cx, realm);
     Rooted<GlobalObject*> shg(cx, GlobalObject::createInternal(cx, &shgClass));
     if (!shg)
         return nullptr;
 
     cx->runtime()->selfHostingGlobal_ = shg;
     realm->setIsSelfHostingRealm();
-    realm->setIsSystem(true);
 
     if (!GlobalObject::initSelfHostingBuiltins(cx, shg, intrinsic_functions))
         return nullptr;
 
     JS_FireOnNewGlobalObject(cx, shg);
 
     return shg;
 }