Bug 895223 - Always pass the holder to JSGetterOps. r=jorendorff,jandem

js/src/jit/IonCaches.cpp

@@ -1039,10 +1039,9 @@ EmitGetterCall(JSContext* cx, MacroAssembler& masm,
         masm.adjustStack(IonOOLNativeExitFrameLayout::Size(0));
     } else if (IsCacheableGetPropCallPropertyOp(obj, holder, shape)) {
         Register argJSContextReg = regSet.takeAnyGeneral();
-        Register argUintNReg     = regSet.takeAnyGeneral();
-        Register argVpReg        = regSet.takeAnyGeneral();
-        Register argObjReg       = argUintNReg;
+        Register argObjReg       = regSet.takeAnyGeneral();
         Register argIdReg        = regSet.takeAnyGeneral();
+        Register argVpReg        = regSet.takeAnyGeneral();
 
         GetterOp target = shape->getterOp();
         MOZ_ASSERT(target);
@@ -1056,11 +1055,22 @@ EmitGetterCall(JSContext* cx, MacroAssembler& masm,
         masm.Push(UndefinedValue());
         masm.moveStackPtrTo(argVpReg);
 
-        // push canonical jsid from shape instead of propertyname.
+        // Push canonical jsid from shape instead of propertyname.
         masm.Push(shape->propid(), scratchReg);
         masm.moveStackPtrTo(argIdReg);
 
-        masm.Push(object);
+        // Push the holder.
+        if (obj == holder) {
+            // When the holder is also the current receiver, we just have a shape guard,
+            // so we might end up with a random object which is also guaranteed to have
+            // this JSGetterOp.
+            masm.Push(object);
+        } else {
+            // If the holder is on the prototype chain, the prototype-guarding
+            // only allows objects with the same holder.
+            masm.movePtr(ImmMaybeNurseryPtr(holder), scratchReg);
+            masm.Push(scratchReg);
+        }
         masm.moveStackPtrTo(argObjReg);
 
         masm.loadJSContext(argJSContextReg);

js/src/jsarray.cpp

@@ -436,25 +436,10 @@ js::SetLengthProperty(JSContext* cx, HandleObject obj, double length)
     return SetProperty(cx, obj, cx->names().length, v);
 }
 
-/*
- * Since SpiderMonkey supports cross-class prototype-based delegation, we have
- * to be careful about the length getter and setter being called on an object
- * not of Array class. For the getter, we search obj's prototype chain for the
- * array that caused this getter to be invoked. In the setter case to overcome
- * the JSPROP_SHARED attribute, we must define a shadowing length property.
- */
 static bool
-array_length_getter(JSContext* cx, HandleObject obj_, HandleId id, MutableHandleValue vp)
+array_length_getter(JSContext* cx, HandleObject obj, HandleId id, MutableHandleValue vp)
 {
-    RootedObject obj(cx, obj_);
-    do {
-        if (obj->is<ArrayObject>()) {
-            vp.setNumber(obj->as<ArrayObject>().length());
-            return true;
-        }
-        if (!GetPrototype(cx, obj, &obj))
-            return false;
-    } while (obj);
+    vp.setNumber(obj->as<ArrayObject>().length());
     return true;
 }
 

js/src/jscntxtinlines.h

@@ -298,13 +298,13 @@ CallJSNativeConstructor(JSContext* cx, Native native, const CallArgs& args)
 }
 
 MOZ_ALWAYS_INLINE bool
-CallJSGetterOp(JSContext* cx, GetterOp op, HandleObject receiver, HandleId id,
+CallJSGetterOp(JSContext* cx, GetterOp op, HandleObject obj, HandleId id,
                MutableHandleValue vp)
 {
     JS_CHECK_RECURSION(cx, return false);
 
-    assertSameCompartment(cx, receiver, id, vp);
-    bool ok = op(cx, receiver, id, vp);
+    assertSameCompartment(cx, obj, id, vp);
+    bool ok = op(cx, obj, id, vp);
     if (ok)
         assertSameCompartment(cx, vp);
     return ok;

js/src/proxy/BaseProxyHandler.cpp

@@ -68,7 +68,9 @@ BaseProxyHandler::get(JSContext* cx, HandleObject proxy, HandleObject receiver,
     else
         vp.setUndefined();
 
-    return CallJSGetterOp(cx, desc.getter(), receiver, id, vp);
+    // A proxy object should never have own JSGetterOps.
+    MOZ_ASSERT(desc.object() != proxy);
+    return CallJSGetterOp(cx, desc.getter(), desc.object(), id, vp);
 }
 
 bool

js/src/vm/ArgumentsObject.cpp

@@ -288,9 +288,6 @@ args_delProperty(JSContext* cx, HandleObject obj, HandleId id, ObjectOpResult& r
 static bool
 ArgGetter(JSContext* cx, HandleObject obj, HandleId id, MutableHandleValue vp)
 {
-    if (!obj->is<NormalArgumentsObject>())
-        return true;
-
     NormalArgumentsObject& argsobj = obj->as<NormalArgumentsObject>();
     if (JSID_IS_INT(id)) {
         /*
@@ -412,9 +409,6 @@ args_enumerate(JSContext* cx, HandleObject obj)
 static bool
 StrictArgGetter(JSContext* cx, HandleObject obj, HandleId id, MutableHandleValue vp)
 {
-    if (!obj->is<StrictArgumentsObject>())
-        return true;
-
     StrictArgumentsObject& argsobj = obj->as<StrictArgumentsObject>();
 
     if (JSID_IS_INT(id)) {

js/src/vm/NativeObject.cpp

@@ -1565,7 +1565,8 @@ js::NativeHasProperty(JSContext* cx, HandleNativeObject obj, HandleId id, bool*
 /*** [[Get]] *************************************************************************************/
 
 static inline bool
-CallGetter(JSContext* cx, HandleObject receiver, HandleShape shape, MutableHandleValue vp)
+CallGetter(JSContext* cx, HandleObject obj, HandleObject receiver, HandleShape shape,
+           MutableHandleValue vp)
 {
     MOZ_ASSERT(!shape->hasDefaultGetter());
 
@@ -1574,8 +1575,9 @@ CallGetter(JSContext* cx, HandleObject receiver, HandleShape shape, MutableHandl
         return InvokeGetter(cx, receiver, fval, vp);
     }
 
+    // In contrast to normal getters JSGetterOps always want the holder.
     RootedId id(cx, shape->propid());
-    return CallJSGetterOp(cx, shape->getterOp(), receiver, id, vp);
+    return CallJSGetterOp(cx, shape->getterOp(), obj, id, vp);
 }
 
 template <AllowGC allowGC>
@@ -1619,6 +1621,7 @@ GetExistingProperty(JSContext* cx,
         return false;
 
     if (!CallGetter(cx,
+                    MaybeRooted<JSObject*, allowGC>::toHandle(obj),
                     MaybeRooted<JSObject*, allowGC>::toHandle(receiver),
                     MaybeRooted<Shape*, allowGC>::toHandle(shape),
                     MaybeRooted<Value, allowGC>::toMutableHandle(vp)))