Bug 1300548 - Fix the iterator cache to not reuse iterators in different compartments. r=jonco
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 07 Sep 2016 16:55:06 +0200
changeset 313050 2356dac937b763c3b4918a0f82d65741c1a28905
parent 313049 d2d69b3328ffdc44ebfedc1f7ff29d96f0b48139
child 313051 80dccdd8c94ae0f47a9c037bc56e4c4ed79ebfbb
push id30669
push userkwierso@gmail.com
push dateThu, 08 Sep 2016 00:56:12 +0000
treeherdermozilla-central@77940cbf0c2a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1300548
milestone51.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 1300548 - Fix the iterator cache to not reuse iterators in different compartments. r=jonco
js/src/jit-test/tests/basic/bug1300548.js
js/src/jit/CodeGenerator.cpp
js/src/jit/CompileWrappers.cpp
js/src/jit/CompileWrappers.h
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/jsiter.cpp
js/src/vm/Caches.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug1300548.js
@@ -0,0 +1,13 @@
+var g1 = newGlobal();
+var g2 = newGlobal({sameZoneAs: g1});
+function f() {
+    var o = Object.create(null);
+    for (var p in o) {};
+}
+g1.eval(f.toSource());
+g2.eval(f.toSource());
+
+for (var i=0; i<10; i++) {
+    g1.eval("f()");
+    g2.eval("f()");
+}
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -8780,17 +8780,17 @@ CodeGenerator::visitIteratorStart(LItera
     const Register temp1 = ToRegister(lir->temp1());
     const Register temp2 = ToRegister(lir->temp2());
     const Register niTemp = ToRegister(lir->temp3()); // Holds the NativeIterator object.
 
     // Iterators other than for-in should use LCallIteratorStart.
     MOZ_ASSERT(flags == JSITER_ENUMERATE);
 
     // Fetch the most recent iterator and ensure it's not nullptr.
-    masm.loadPtr(AbsoluteAddress(GetJitContext()->runtime->addressOfLastCachedNativeIterator()), output);
+    masm.loadPtr(AbsoluteAddress(gen->compartment->addressOfLastCachedNativeIterator()), output);
     masm.branchTestPtr(Assembler::Zero, output, output, ool->entry());
 
     // Load NativeIterator.
     masm.loadObjPrivate(output, JSObject::ITER_CLASS_NFIXED_SLOTS, niTemp);
 
     // Ensure the |active| and |unreusable| bits are not set.
     masm.branchTest32(Assembler::NonZero, Address(niTemp, offsetof(NativeIterator, flags)),
                       Imm32(JSITER_ACTIVE|JSITER_UNREUSABLE), ool->entry());
--- a/js/src/jit/CompileWrappers.cpp
+++ b/js/src/jit/CompileWrappers.cpp
@@ -69,22 +69,16 @@ CompileRuntime::addressOfIonBailAfter()
 #endif
 
 const void*
 CompileRuntime::addressOfActivation()
 {
     return runtime()->addressOfActivation();
 }
 
-const void*
-CompileRuntime::addressOfLastCachedNativeIterator()
-{
-    return &static_cast<JSContext*>(runtime())->caches.nativeIterCache.last;
-}
-
 #ifdef JS_GC_ZEAL
 const void*
 CompileRuntime::addressOfGCZealModeBits()
 {
     return runtime()->gc.addressOfZealModeBits();
 }
 #endif
 
@@ -244,16 +238,22 @@ CompileCompartment::runtime()
 
 const void*
 CompileCompartment::addressOfEnumerators()
 {
     return &compartment()->enumerators;
 }
 
 const void*
+CompileCompartment::addressOfLastCachedNativeIterator()
+{
+    return &compartment()->lastCachedNativeIterator;
+}
+
+const void*
 CompileCompartment::addressOfRandomNumberGenerator()
 {
     return compartment()->randomNumberGenerator.ptr();
 }
 
 const JitCompartment*
 CompileCompartment::jitCompartment()
 {
--- a/js/src/jit/CompileWrappers.h
+++ b/js/src/jit/CompileWrappers.h
@@ -46,19 +46,16 @@ class CompileRuntime
 #ifdef DEBUG
     // rt->runtime()->addressOfIonBailAfter;
     const void* addressOfIonBailAfter();
 #endif
 
     // &runtime()->activation_
     const void* addressOfActivation();
 
-    // &GetJitContext()->runtime->nativeIterCache.last
-    const void* addressOfLastCachedNativeIterator();
-
 #ifdef JS_GC_ZEAL
     const void* addressOfGCZealModeBits();
 #endif
 
     const void* addressOfInterruptUint32();
 
     // We have to bake JSContext* into JIT code, but this pointer shouldn't be
     // used/dereferenced on the background thread so we return it as void*.
@@ -112,16 +109,17 @@ class CompileCompartment
   public:
     static CompileCompartment* get(JSCompartment* comp);
 
     CompileZone* zone();
     CompileRuntime* runtime();
 
     const void* addressOfEnumerators();
     const void* addressOfRandomNumberGenerator();
+    const void* addressOfLastCachedNativeIterator();
 
     const JitCompartment* jitCompartment();
 
     const GlobalObject* maybeGlobal();
 
     bool hasAllocationMetadataBuilder();
 
     // Mirror CompartmentOptions.
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -75,16 +75,17 @@ JSCompartment::JSCompartment(Zone* zone,
     nonSyntacticLexicalEnvironments_(nullptr),
     gcIncomingGrayPointers(nullptr),
     debugModeBits(0),
     watchpointMap(nullptr),
     scriptCountsMap(nullptr),
     debugScriptMap(nullptr),
     debugEnvs(nullptr),
     enumerators(nullptr),
+    lastCachedNativeIterator(nullptr),
     compartmentStats_(nullptr),
     scheduledForDestruction(false),
     maybeAlive(true),
     jitCompartment_(nullptr),
     mappedArgumentsTemplate_(nullptr),
     unmappedArgumentsTemplate_(nullptr),
     lcovOutput()
 {
@@ -855,19 +856,19 @@ JSCompartment::fixupCrossCompartmentWrap
         // Trace the wrappers in the map to update their edges to their referents.
         comp->traceOutgoingCrossCompartmentWrappers(trc);
     }
 }
 
 void
 JSCompartment::fixupAfterMovingGC()
 {
+    purge();
     fixupGlobal();
     objectGroups.fixupTablesAfterMovingGC();
-    dtoaCache.purge();
     fixupScriptMapsAfterMovingGC();
 }
 
 void
 JSCompartment::fixupGlobal()
 {
     GlobalObject* global = *global_.unsafeGet();
     if (global)
@@ -926,16 +927,17 @@ JSCompartment::checkScriptMapsAfterMovin
     }
 }
 #endif
 
 void
 JSCompartment::purge()
 {
     dtoaCache.purge();
+    lastCachedNativeIterator = nullptr;
 }
 
 void
 JSCompartment::clearTables()
 {
     global_.set(nullptr);
 
     // No scripts should have run in this compartment. This is used when
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -797,16 +797,19 @@ struct JSCompartment
     js::DebugEnvironments* debugEnvs;
 
     /*
      * List of potentially active iterators that may need deleted property
      * suppression.
      */
     js::NativeIterator* enumerators;
 
+    /* Native iterator most recently started. */
+    js::PropertyIteratorObject* lastCachedNativeIterator;
+
   private:
     /* Used by memory reporters and invalid otherwise. */
     JS::CompartmentStats* compartmentStats_;
 
   public:
     // This should only be called when it is non-null, i.e. during memory
     // reporting.
     JS::CompartmentStats& compartmentStats() {
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -818,16 +818,22 @@ CanCacheIterableObject(JSContext* cx, JS
         }
     }
     return true;
 }
 
 bool
 js::GetIterator(JSContext* cx, HandleObject obj, unsigned flags, MutableHandleObject objp)
 {
+#ifdef DEBUG
+    auto assertCompartment = mozilla::MakeScopeExit([&] {
+        assertSameCompartment(cx, objp);
+    });
+#endif
+
     if (obj->is<PropertyIteratorObject>() || obj->is<LegacyGeneratorObject>()) {
         objp.set(obj);
         return true;
     }
 
     // We should only call the enumerate trap for "for-in".
     // Or when we call GetIterator from the Proxy [[Enumerate]] hook.
     // JSITER_ENUMERATE is just an optimization and the same
@@ -837,18 +843,17 @@ js::GetIterator(JSContext* cx, HandleObj
             return Proxy::enumerate(cx, obj, objp);
     }
 
     Vector<ReceiverGuard, 8> guards(cx);
     uint32_t key = 0;
     if (flags == JSITER_ENUMERATE) {
         // Check to see if this is the same as the most recent object which was
         // iterated over.
-        PropertyIteratorObject* last = cx->caches.nativeIterCache.last;
-        if (last) {
+        if (PropertyIteratorObject* last = cx->compartment()->lastCachedNativeIterator) {
             NativeIterator* lastni = last->getNativeIterator();
             if (!(lastni->flags & (JSITER_ACTIVE|JSITER_UNREUSABLE)) &&
                 CanCompareIterableObjectToCache(obj) &&
                 ReceiverGuard(obj) == lastni->guard_array[0])
             {
                 JSObject* proto = obj->staticPrototype();
                 if (CanCompareIterableObjectToCache(proto) &&
                     ReceiverGuard(proto) == lastni->guard_array[1] &&
@@ -877,31 +882,31 @@ js::GetIterator(JSContext* cx, HandleObj
                 key = (key + (key << 16)) ^ guard.hash();
                 if (!guards.append(guard))
                     return false;
 
                 pobj = pobj->staticPrototype();
             } while (pobj);
         }
 
-        PropertyIteratorObject* iterobj = cx->caches.nativeIterCache.get(key);
-        if (iterobj) {
+        if (PropertyIteratorObject* iterobj = cx->caches.nativeIterCache.get(key)) {
             NativeIterator* ni = iterobj->getNativeIterator();
             if (!(ni->flags & (JSITER_ACTIVE|JSITER_UNREUSABLE)) &&
                 ni->guard_key == key &&
                 ni->guard_length == guards.length() &&
                 Compare(reinterpret_cast<ReceiverGuard*>(ni->guard_array),
-                        guards.begin(), ni->guard_length))
+                        guards.begin(), ni->guard_length) &&
+                iterobj->compartment() == cx->compartment())
             {
                 objp.set(iterobj);
 
                 UpdateNativeIterator(ni, obj);
                 RegisterEnumerator(cx, iterobj, ni);
                 if (guards.length() == 2)
-                    cx->caches.nativeIterCache.last = iterobj;
+                    cx->compartment()->lastCachedNativeIterator = iterobj;
                 return true;
             }
         }
     }
 
   miss:
     if (!GetCustomIterator(cx, obj, flags, objp))
         return false;
@@ -925,17 +930,17 @@ js::GetIterator(JSContext* cx, HandleObj
 
     PropertyIteratorObject* iterobj = &objp->as<PropertyIteratorObject>();
 
     /* Cache the iterator object if possible. */
     if (guards.length())
         cx->caches.nativeIterCache.set(key, iterobj);
 
     if (guards.length() == 2)
-        cx->caches.nativeIterCache.last = iterobj;
+        cx->compartment()->lastCachedNativeIterator = iterobj;
     return true;
 }
 
 JSObject*
 js::GetIteratorObject(JSContext* cx, HandleObject obj, uint32_t flags)
 {
     RootedObject iterator(cx);
     if (!GetIterator(cx, obj, flags, &iterator))
--- a/js/src/vm/Caches.h
+++ b/js/src/vm/Caches.h
@@ -121,27 +121,21 @@ class NativeIterCache
     /* Cached native iterators. */
     PropertyIteratorObject* data[SIZE];
 
     static size_t getIndex(uint32_t key) {
         return size_t(key) % SIZE;
     }
 
   public:
-    /* Native iterator most recently started. */
-    PropertyIteratorObject* last;
-
-    NativeIterCache()
-      : last(nullptr)
-    {
+    NativeIterCache() {
         mozilla::PodArrayZero(data);
     }
 
     void purge() {
-        last = nullptr;
         mozilla::PodArrayZero(data);
     }
 
     PropertyIteratorObject* get(uint32_t key) const {
         return data[getIndex(key)];
     }
 
     void set(uint32_t key, PropertyIteratorObject* iterobj) {