Bug 1100594 - Whitelist the function resolve hook in LookupPropertyPureInline. r=bhackett
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 20 Nov 2014 12:07:23 +0100
changeset 216635 bd55fa110906ba994c3a1b10480164c2e54d9998
parent 216634 7b7941988a83594e39548b9f855c774844b269ab
child 216636 bf36fe5a2125fc4d2b923de5adbf5de6c80479d5
push id27858
push userkwierso@gmail.com
push dateFri, 21 Nov 2014 01:35:46 +0000
treeherdermozilla-central@6309710dd71d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs1100594
milestone36.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 1100594 - Whitelist the function resolve hook in LookupPropertyPureInline. r=bhackett
js/src/jit/IonBuilder.cpp
js/src/jit/IonCaches.cpp
js/src/jsfun.cpp
js/src/jsfun.h
js/src/jsobj.cpp
js/src/jsobj.h
js/src/vm/NativeObject.cpp
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -6556,17 +6556,17 @@ ClassHasResolveHook(CompileCompartment *
         return false;
 
     if (clasp->resolve == str_resolve) {
         // str_resolve only resolves integers, not names.
         return false;
     }
 
     if (clasp->resolve == fun_resolve)
-        return FunctionHasResolveHook(comp->runtime()->names(), name);
+        return FunctionHasResolveHook(comp->runtime()->names(), NameToId(name));
 
     return true;
 }
 
 void
 IonBuilder::insertRecompileCheck()
 {
     // PJS doesn't recompile and doesn't need recompile checks.
--- a/js/src/jit/IonCaches.cpp
+++ b/js/src/jit/IonCaches.cpp
@@ -1162,17 +1162,17 @@ CanAttachNativeGetProp(typename GetPropC
 {
     if (!obj || !obj->isNative())
         return GetPropertyIC::CanAttachNone;
 
     // The lookup needs to be universally pure, otherwise we risk calling hooks out
     // of turn. We don't mind doing this even when purity isn't required, because we
     // only miss out on shape hashification, which is only a temporary perf cost.
     // The limits were arbitrarily set, anyways.
-    if (!LookupPropertyPure(obj, NameToId(name), holder.address(), shape.address()))
+    if (!LookupPropertyPure(cx, obj, NameToId(name), holder.address(), shape.address()))
         return GetPropertyIC::CanAttachNone;
 
     RootedScript script(cx);
     jsbytecode *pc;
     cache.getScriptedLocation(&script, &pc);
     if (IsCacheableGetPropReadSlotForIon(obj, holder, shape) ||
         IsCacheableNoProperty(obj, holder, shape, pc, cache.output()))
     {
@@ -2778,30 +2778,30 @@ IsPropertyAddInlineable(NativeObject *ob
     if (needsTypeBarrier)
         return CanInlineSetPropTypeCheck(obj, id, val, checkTypeset);
 
     *checkTypeset = false;
     return true;
 }
 
 static SetPropertyIC::NativeSetPropCacheability
-CanAttachNativeSetProp(HandleObject obj, HandleId id, ConstantOrRegister val,
+CanAttachNativeSetProp(ThreadSafeContext *cx, HandleObject obj, HandleId id, ConstantOrRegister val,
                        bool needsTypeBarrier, MutableHandleNativeObject holder,
                        MutableHandleShape shape, bool *checkTypeset)
 {
     if (!obj->isNative())
         return SetPropertyIC::CanAttachNone;
 
     // See if the property exists on the object.
     if (IsPropertySetInlineable(&obj->as<NativeObject>(), id, shape, val, needsTypeBarrier, checkTypeset))
         return SetPropertyIC::CanAttachSetSlot;
 
     // If we couldn't find the property on the object itself, do a full, but
     // still pure lookup for setters.
-    if (!LookupPropertyPure(obj, id, holder.address(), shape.address()))
+    if (!LookupPropertyPure(cx, obj, id, holder.address(), shape.address()))
         return SetPropertyIC::CanAttachNone;
 
     // If the object doesn't have the property, we don't know if we can attach
     // a stub to add the property until we do the VM call to add. If the
     // property exists as a data property on the prototype, we should add
     // a new, shadowing property.
     if (!shape || (obj != holder && shape->hasDefaultSetter() && shape->hasSlot()))
         return SetPropertyIC::MaybeCanAttachAddSlot;
@@ -2859,17 +2859,17 @@ SetPropertyIC::update(JSContext *cx, siz
                     return false;
                 addedSetterStub = true;
             }
         }
 
         RootedShape shape(cx);
         RootedNativeObject holder(cx);
         bool checkTypeset;
-        canCache = CanAttachNativeSetProp(obj, id, cache.value(), cache.needsTypeBarrier(),
+        canCache = CanAttachNativeSetProp(cx, obj, id, cache.value(), cache.needsTypeBarrier(),
                                           &holder, &shape, &checkTypeset);
 
         if (!addedSetterStub && canCache == CanAttachSetSlot) {
             RootedNativeObject nobj(cx, &obj->as<NativeObject>());
             if (!cache.attachSetSlot(cx, script, ion, nobj, shape, checkTypeset))
                 return false;
             addedSetterStub = true;
         }
@@ -2952,17 +2952,17 @@ SetPropertyParIC::update(ForkJoinContext
             // this is not safe in parallel.
             if (nobj->hasLazyType())
                 return false;
 
             {
                 RootedShape shape(cx);
                 RootedNativeObject holder(cx);
                 bool checkTypeset;
-                canCache = CanAttachNativeSetProp(nobj, id, cache.value(), cache.needsTypeBarrier(),
+                canCache = CanAttachNativeSetProp(cx, nobj, id, cache.value(), cache.needsTypeBarrier(),
                                                   &holder, &shape, &checkTypeset);
 
                 if (canCache == SetPropertyIC::CanAttachSetSlot) {
                     if (!cache.attachSetSlot(ncx, ion, nobj, shape, checkTypeset))
                         return cx->setPendingAbortFatal(ParallelBailoutOutOfMemory);
                     attachedStub = true;
                 }
             }
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -438,19 +438,23 @@ ResolveInterpretedFunctionPrototype(JSCo
             return nullptr;
         }
     }
 
     return proto;
 }
 
 bool
-js::FunctionHasResolveHook(const JSAtomState &atomState, PropertyName *name)
+js::FunctionHasResolveHook(const JSAtomState &atomState, jsid id)
 {
-    return name == atomState.prototype || name == atomState.length || name == atomState.name;
+    if (!JSID_IS_ATOM(id))
+        return false;
+
+    JSAtom *atom = JSID_TO_ATOM(id);
+    return atom == atomState.prototype || atom == atomState.length || atom == atomState.name;
 }
 
 bool
 js::fun_resolve(JSContext *cx, HandleObject obj, HandleId id, bool *resolvedp)
 {
     if (!JSID_IS_ATOM(id))
         return true;
 
--- a/js/src/jsfun.h
+++ b/js/src/jsfun.h
@@ -534,17 +534,17 @@ IdToFunctionName(JSContext *cx, HandleId
 
 extern JSFunction *
 DefineFunction(JSContext *cx, HandleObject obj, HandleId id, JSNative native,
                unsigned nargs, unsigned flags,
                gc::AllocKind allocKind = JSFunction::FinalizeKind,
                NewObjectKind newKind = GenericObject);
 
 bool
-FunctionHasResolveHook(const JSAtomState &atomState, PropertyName *name);
+FunctionHasResolveHook(const JSAtomState &atomState, jsid id);
 
 extern bool
 fun_resolve(JSContext *cx, HandleObject obj, HandleId id, bool *resolvedp);
 
 extern bool
 fun_toString(JSContext *cx, unsigned argc, Value *vp);
 
 extern bool
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -3234,17 +3234,18 @@ js::HasOwnProperty(JSContext *cx, Handle
     RootedShape shape(cx);
     if (!HasOwnProperty<CanGC>(cx, obj->getOps()->lookupGeneric, obj, id, &pobj, &shape))
         return false;
     *resultp = (shape != nullptr);
     return true;
 }
 
 static MOZ_ALWAYS_INLINE bool
-LookupPropertyPureInline(JSObject *obj, jsid id, NativeObject **objp, Shape **propp)
+LookupPropertyPureInline(ThreadSafeContext *cx, JSObject *obj, jsid id, NativeObject **objp,
+                         Shape **propp)
 {
     if (!obj->isNative())
         return false;
 
     NativeObject *current = &obj->as<NativeObject>();
     while (true) {
         /* Search for a native dense element, typed array element, or property. */
 
@@ -3269,19 +3270,27 @@ LookupPropertyPureInline(JSObject *obj, 
         }
 
         if (Shape *shape = current->lookupPure(id)) {
             *objp = current;
             *propp = shape;
             return true;
         }
 
-        /* Fail if there's a resolve hook. */
-        if (current->getClass()->resolve != JS_ResolveStub)
+        // Fail if there's a resolve hook. We allow the JSFunction resolve hook
+        // if we know it will never add a property with this name.
+        do {
+            const Class *clasp = current->getClass();
+            MOZ_ASSERT(clasp->resolve);
+            if (clasp->resolve == JS_ResolveStub)
+                break;
+            if (clasp->resolve == fun_resolve && !FunctionHasResolveHook(cx->names(), id))
+                break;
             return false;
+        } while (0);
 
         JSObject *proto = current->getProto();
 
         if (!proto)
             break;
         if (!proto->isNative())
             return false;
 
@@ -3303,19 +3312,20 @@ NativeGetPureInline(NativeObject *pobj, 
         vp->setUndefined();
     }
 
     /* Fail if we have a custom getter. */
     return shape->hasDefaultGetter();
 }
 
 bool
-js::LookupPropertyPure(JSObject *obj, jsid id, NativeObject **objp, Shape **propp)
+js::LookupPropertyPure(ThreadSafeContext *cx, JSObject *obj, jsid id, NativeObject **objp,
+                       Shape **propp)
 {
-    return LookupPropertyPureInline(obj, id, objp, propp);
+    return LookupPropertyPureInline(cx, obj, id, objp, propp);
 }
 
 /*
  * A pure version of GetPropertyHelper that can be called from parallel code
  * without locking. This code path cannot GC. This variant returns false
  * whenever a side-effect might have occured in the effectful version. This
  * includes, but is not limited to:
  *
@@ -3324,17 +3334,17 @@ js::LookupPropertyPure(JSObject *obj, js
  *  - The property has a getter.
  */
 bool
 js::GetPropertyPure(ThreadSafeContext *cx, JSObject *obj, jsid id, Value *vp)
 {
     /* Deal with native objects. */
     NativeObject *obj2;
     Shape *shape;
-    if (!LookupPropertyPureInline(obj, id, &obj2, &shape))
+    if (!LookupPropertyPureInline(cx, obj, id, &obj2, &shape))
         return false;
 
     if (!shape) {
         /* Fail if we have a non-stub class op hooks. */
         if (obj->getClass()->getProperty && obj->getClass()->getProperty != JS_PropertyStub)
             return false;
 
         if (obj->getOps()->getElement)
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -1108,17 +1108,18 @@ LookupNameUnqualified(JSContext *cx, Han
 
 extern JSObject *
 js_FindVariableScope(JSContext *cx, JSFunction **funp);
 
 
 namespace js {
 
 bool
-LookupPropertyPure(JSObject *obj, jsid id, NativeObject **objp, Shape **propp);
+LookupPropertyPure(ThreadSafeContext *cx, JSObject *obj, jsid id, NativeObject **objp,
+                   Shape **propp);
 
 bool
 GetPropertyPure(ThreadSafeContext *cx, JSObject *obj, jsid id, Value *vp);
 
 inline bool
 GetPropertyPure(ThreadSafeContext *cx, JSObject *obj, PropertyName *name, Value *vp)
 {
     return GetPropertyPure(cx, obj, NameToId(name), vp);
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -2048,17 +2048,17 @@ baseops::SetPropertyHelper(typename Exec
         if (wpmap && !wpmap->triggerWatchpoint(cx, obj, id, vp))
             return false;
     }
 
     RootedObject pobj(cxArg);
     RootedShape shape(cxArg);
     if (mode == ParallelExecution) {
         NativeObject *npobj;
-        if (!LookupPropertyPure(obj, id, &npobj, shape.address()))
+        if (!LookupPropertyPure(cxArg, obj, id, &npobj, shape.address()))
             return false;
         pobj = npobj;
     } else {
         JSContext *cx = cxArg->asJSContext();
         if (!LookupNativeProperty(cx, obj, id, &pobj, &shape))
             return false;
     }