903389 - valueOf getter not called for ToPrimitive conversion on wrapper types.

js/src/builtin/RegExp.cpp

@@ -1800,7 +1800,7 @@ js::intrinsic_GetStringDataProperty(JSContext* cx, unsigned argc, Value* vp)
         return false;
 
     RootedValue v(cx);
-    if (HasDataProperty(cx, nobj, AtomToId(atom), v.address()) && v.isString())
+    if (GetPropertyPure(cx, nobj, AtomToId(atom), v.address()) && v.isString())
         args.rval().set(v);
     else
         args.rval().setUndefined();

js/src/jit-test/tests/asm.js/import-function-toPrimitive.js

@@ -0,0 +1,26 @@
+var counter = 0;
+
+function f(stdlib, foreign)
+{
+  "use asm";
+  var a = +foreign.a;
+  var b = +foreign.b;
+  function g() {}
+  return g;
+}
+
+var foreign =
+  {
+    a: function() {},
+    b: /value that doesn't coerce purely/,
+  };
+
+foreign.a[Symbol.toPrimitive] =
+  function()
+  {
+    counter++;
+    return 0;
+  };
+
+f(null, foreign);
+assertEq(counter, 1);

js/src/jit-test/tests/basic/hasnativemethodpure-optimization.js

@@ -0,0 +1,21 @@
+load(libdir + "asserts.js");
+
+let string = Object.defineProperty(new String("123"), "valueOf", {
+    get: function() { throw "get-valueOf"; }
+});
+assertThrowsValue(() => "" + string, "get-valueOf");
+
+string = Object.defineProperty(new String("123"), "toString", {
+    get: function() { throw "get-toString"; }
+});
+assertThrowsValue(() => string.toLowerCase(), "get-toString");
+
+string = Object.defineProperty(new String("123"), Symbol.toPrimitive, {
+    get: function() { throw "get-toPrimitive"; }
+});
+assertThrowsValue(() => string.toLowerCase(), "get-toPrimitive");
+
+let number = Object.defineProperty(new Number(123), "valueOf", {
+    get: function() { throw "get-valueOf"; }
+});
+assertThrowsValue(() => +number, "get-valueOf");

js/src/jsobj.cpp

@@ -2326,9 +2326,18 @@ js::LookupOwnPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, Shape**
 }
 
 static inline bool
-NativeGetPureInline(NativeObject* pobj, Shape* shape, Value* vp)
+NativeGetPureInline(NativeObject* pobj, jsid id, Shape* shape, Value* vp)
 {
-    /* Fail if we have a custom getter. */
+    if (IsImplicitDenseOrTypedArrayElement(shape)) {
+        // For simplicity we ignore the TypedArray with string index case.
+        if (!JSID_IS_INT(id))
+            return false;
+
+        *vp = pobj->getDenseOrTypedArrayElement(JSID_TO_INT(id));
+        return true;
+    }
+
+    // Fail if we have a custom getter.
     if (!shape->hasDefaultGetter())
         return false;
 
@@ -2355,13 +2364,13 @@ js::GetPropertyPure(ExclusiveContext* cx, JSObject* obj, jsid id, Value* vp)
         return true;
     }
 
-    return pobj->isNative() && NativeGetPureInline(&pobj->as<NativeObject>(), shape, vp);
+    return pobj->isNative() && NativeGetPureInline(&pobj->as<NativeObject>(), id, shape, vp);
 }
 
 static inline bool
 NativeGetGetterPureInline(Shape* shape, JSFunction** fp)
 {
-    if (shape->hasGetterObject()) {
+    if (!IsImplicitDenseOrTypedArrayElement(shape) && shape->hasGetterObject()) {
         if (shape->getterObject()->is<JSFunction>()) {
             *fp = &shape->getterObject()->as<JSFunction>();
             return true;
@@ -2852,24 +2861,6 @@ js::GetObjectClassName(JSContext* cx, HandleObject obj)
 
 /* * */
 
-bool
-js::HasDataProperty(JSContext* cx, NativeObject* obj, jsid id, Value* vp)
-{
-    if (JSID_IS_INT(id) && obj->containsDenseElement(JSID_TO_INT(id))) {
-        *vp = obj->getDenseElement(JSID_TO_INT(id));
-        return true;
-    }
-
-    if (Shape* shape = obj->lookup(cx, id)) {
-        if (shape->hasDefaultGetter() && shape->hasSlot()) {
-            *vp = obj->getSlot(shape->slot());
-            return true;
-        }
-    }
-
-    return false;
-}
-
 extern bool
 PropertySpecNameToId(JSContext* cx, const char* name, MutableHandleId id,
                      js::PinningBehavior pin = js::DoNotPinAtom);
@@ -2985,7 +2976,7 @@ JS::OrdinaryToPrimitive(JSContext* cx, HandleObject obj, JSType hint, MutableHan
         /* Optimize (new String(...)).toString(). */
         if (clasp == &StringObject::class_) {
             StringObject* nobj = &obj->as<StringObject>();
-            if (ClassMethodIsNative(cx, nobj, &StringObject::class_, id, str_toString)) {
+            if (HasNativeMethodPure(nobj, cx->names().toString, str_toString, cx)) {
                 vp.setString(nobj->unbox());
                 return true;
             }
@@ -3007,7 +2998,7 @@ JS::OrdinaryToPrimitive(JSContext* cx, HandleObject obj, JSType hint, MutableHan
         /* Optimize new String(...).valueOf(). */
         if (clasp == &StringObject::class_) {
             StringObject* nobj = &obj->as<StringObject>();
-            if (ClassMethodIsNative(cx, nobj, &StringObject::class_, id, str_toString)) {
+            if (HasNativeMethodPure(nobj, cx->names().valueOf, str_toString, cx)) {
                 vp.setString(nobj->unbox());
                 return true;
             }
@@ -3016,7 +3007,7 @@ JS::OrdinaryToPrimitive(JSContext* cx, HandleObject obj, JSType hint, MutableHan
         /* Optimize new Number(...).valueOf(). */
         if (clasp == &NumberObject::class_) {
             NumberObject* nobj = &obj->as<NumberObject>();
-            if (ClassMethodIsNative(cx, nobj, &NumberObject::class_, id, num_valueOf)) {
+            if (HasNativeMethodPure(nobj, cx->names().valueOf, num_valueOf, cx)) {
                 vp.setNumber(nobj->unbox());
                 return true;
             }

js/src/jsobjinlines.h

@@ -557,48 +557,30 @@ IsNativeFunction(const js::Value& v, JSNative native)
     return IsFunctionObject(v, &fun) && fun->maybeNative() == native;
 }
 
-/*
- * When we have an object of a builtin class, we don't quite know what its
- * valueOf/toString methods are, since these methods may have been overwritten
- * or shadowed. However, we can still do better than the general case by
- * hard-coding the necessary properties for us to find the native we expect.
- *
- * TODO: a per-thread shape-based cache would be faster and simpler.
- */
+// Return whether looking up a method on 'obj' definitely resolves to the
+// original specified native function. The method may conservatively return
+// 'false' in the case of proxies or other non-native objects.
 static MOZ_ALWAYS_INLINE bool
-ClassMethodIsNative(JSContext* cx, NativeObject* obj, const Class* clasp, jsid methodid, JSNative native)
+HasNativeMethodPure(JSObject* obj, PropertyName* name, JSNative native, JSContext* cx)
 {
-    MOZ_ASSERT(obj->getClass() == clasp);
-
     Value v;
-    if (!HasDataProperty(cx, obj, methodid, &v)) {
-        JSObject* proto = obj->staticPrototype();
-        if (!proto || proto->getClass() != clasp || !HasDataProperty(cx, &proto->as<NativeObject>(), methodid, &v))
-            return false;
-    }
+    if (!GetPropertyPure(cx, obj, NameToId(name), &v))
+        return false;
 
     return IsNativeFunction(v, native);
 }
 
-// Return whether looking up 'valueOf' on 'obj' definitely resolves to the
-// original Object.prototype.valueOf. The method may conservatively return
-// 'false' in the case of proxies or other non-native objects.
+// Return whether 'obj' definitely has no @@toPrimitive method.
 static MOZ_ALWAYS_INLINE bool
-HasObjectValueOf(JSObject* obj, JSContext* cx)
+HasNoToPrimitiveMethodPure(JSObject* obj, JSContext* cx)
 {
-    if (obj->is<ProxyObject>() || !obj->isNative())
+    jsid id = SYMBOL_TO_JSID(cx->wellKnownSymbols().toPrimitive);
+    JSObject* pobj;
+    Shape* shape;
+    if (!LookupPropertyPure(cx, obj, id, &pobj, &shape))
         return false;
 
-    jsid valueOf = NameToId(cx->names().valueOf);
-
-    Value v;
-    while (!HasDataProperty(cx, &obj->as<NativeObject>(), valueOf, &v)) {
-        obj = obj->staticPrototype();
-        if (!obj || obj->is<ProxyObject>() || !obj->isNative())
-            return false;
-    }
-
-    return IsNativeFunction(v, obj_valueOf);
+    return !shape;
 }
 
 /* ES6 draft rev 28 (2014 Oct 14) 7.1.14 */

js/src/jsstr.cpp

@@ -461,9 +461,13 @@ ToStringForStringFunction(JSContext* cx, HandleValue thisv)
         RootedObject obj(cx, &thisv.toObject());
         if (obj->is<StringObject>()) {
             StringObject* nobj = &obj->as<StringObject>();
-            Rooted<jsid> id(cx, NameToId(cx->names().toString));
-            if (ClassMethodIsNative(cx, nobj, &StringObject::class_, id, str_toString))
+            // We have to make sure that the ToPrimitive call from ToString
+            // would be unobservable.
+            if (HasNoToPrimitiveMethodPure(nobj, cx) &&
+                HasNativeMethodPure(nobj, cx->names().toString, str_toString, cx))
+            {
                 return nobj->unbox();
+            }
         }
     } else if (thisv.isNullOrUndefined()) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_CANT_CONVERT_TO,

js/src/vm/NativeObject.h

@@ -1470,19 +1470,6 @@ NativeGetExistingProperty(JSContext* cx, HandleObject receiver, HandleNativeObje
 
 /* * */
 
-/*
- * If obj has an already-resolved data property for id, return true and
- * store the property value in *vp.
- */
-extern bool
-HasDataProperty(JSContext* cx, NativeObject* obj, jsid id, Value* vp);
-
-inline bool
-HasDataProperty(JSContext* cx, NativeObject* obj, PropertyName* name, Value* vp)
-{
-    return HasDataProperty(cx, obj, NameToId(name), vp);
-}
-
 extern bool
 GetPropertyForNameLookup(JSContext* cx, HandleObject obj, HandleId id, MutableHandleValue vp);
 

js/src/wasm/AsmJS.cpp

@@ -7478,10 +7478,10 @@ HasPureCoercion(JSContext* cx, HandleValue v)
     // coercions are not observable and coercion via ToNumber/ToInt32
     // definitely produces NaN/0. We should remove this special case later once
     // most apps have been built with newer Emscripten.
-    jsid toString = NameToId(cx->names().toString);
     if (v.toObject().is<JSFunction>() &&
-        HasObjectValueOf(&v.toObject(), cx) &&
-        ClassMethodIsNative(cx, &v.toObject().as<JSFunction>(), &JSFunction::class_, toString, fun_toString))
+        HasNoToPrimitiveMethodPure(&v.toObject(), cx) &&
+        HasNativeMethodPure(&v.toObject(), cx->names().valueOf, obj_valueOf, cx) &&
+        HasNativeMethodPure(&v.toObject(), cx->names().toString, fun_toString, cx))
     {
         return true;
     }