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 idunknown
push userunknown
push dateunknown
reviewersbhackett
bugs1100594
milestone36.0a1
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;
     }