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 313025 2356dac937b763c3b4918a0f82d65741c1a28905
parent 313024 d2d69b3328ffdc44ebfedc1f7ff29d96f0b48139
child 313026 80dccdd8c94ae0f47a9c037bc56e4c4ed79ebfbb
push id20479
push userkwierso@gmail.com
push dateThu, 08 Sep 2016 01:08:46 +0000
treeherderfx-team@fb7c6b034329 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1300548
milestone51.0a1
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) {