Bug 1053544 - OdinMonkey: issue link-time validation error when given value with under-defined coercion (r=Waldo)

--HG--
extra : rebase_source : 636b2fe0085e9deeded18889f0db1d7ace4c0ef9

js/src/asmjs/AsmJSLink.cpp

@@ -123,6 +123,24 @@ ValidateGlobalVariable(JSContext *cx, const AsmJSModule &module, AsmJSModule::Gl
         if (!GetDataProperty(cx, importVal, field, &v))
             return false;
 
+        if (!v.isPrimitive()) {
+            // Ideally, we'd reject all non-primitives, but Emscripten has a bug
+            // that generates code that passes functions for some imports. To
+            // avoid breaking all the code that contains this bug, we make an
+            // exception for functions that don't have user-defined valueOf or
+            // toString, for their 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(), &JSFunction::class_, toString, fun_toString))
+            {
+                return LinkFail(cx, "Imported values must be primitives");
+            }
+        }
+
         switch (global.varInitCoercion()) {
           case AsmJS_ToInt32:
             if (!ToInt32(cx, v, (int32_t *)datum))
@@ -181,6 +199,7 @@ ValidateMathBuiltinFunction(JSContext *cx, AsmJSModule::Global &global, HandleVa
     RootedValue v(cx);
     if (!GetDataProperty(cx, globalVal, cx->names().Math, &v))
         return false;
+
     RootedPropertyName field(cx, global.mathName());
     if (!GetDataProperty(cx, v, field, &v))
         return false;
@@ -226,6 +245,7 @@ ValidateConstant(JSContext *cx, AsmJSModule::Global &global, HandleValue globalV
 
     if (!GetDataProperty(cx, v, field, &v))
         return false;
+
     if (!v.isNumber())
         return LinkFail(cx, "math / global constant value needs to be a number");
 

js/src/builtin/Object.cpp

@@ -380,8 +380,8 @@ obj_toLocaleString(JSContext *cx, unsigned argc, Value *vp)
     return obj->callMethod(cx, id, 0, nullptr, args.rval());
 }
 
-static bool
-obj_valueOf(JSContext *cx, unsigned argc, Value *vp)
+bool
+js::obj_valueOf(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     RootedObject obj(cx, ToObject(cx, args.thisv()));

js/src/builtin/Object.h

@@ -25,8 +25,12 @@ extern const JSFunctionSpec object_static_selfhosted_methods[];
 bool
 obj_construct(JSContext *cx, unsigned argc, JS::Value *vp);
 
+bool
+obj_valueOf(JSContext *cx, unsigned argc, JS::Value *vp);
+
 // Exposed so SelfHosting.cpp can use it in the OwnPropertyKeys intrinsic
-bool GetOwnPropertyKeys(JSContext *cx, const JS::CallArgs &args, unsigned flags);
+bool
+GetOwnPropertyKeys(JSContext *cx, const JS::CallArgs &args, unsigned flags);
 
 /*
  * Like IdToValue, but convert int jsids to strings. This is used when

js/src/jit-test/tests/asm.js/testGlobals.js

@@ -1,4 +1,5 @@
 load(libdir + "asm.js");
+load(libdir + "asserts.js");
 
 assertAsmTypeFail(USE_ASM + "var i; function f(){} return f");
 assertAsmTypeFail(USE_ASM + "const i; function f(){} return f");
@@ -117,6 +118,44 @@ assertEq(asmLink(asmCompile('global', 'imp', USE_ASM + "const i=+imp.i; function
 assertEq(asmLink(asmCompile('global', 'imp', USE_ASM + "var i=+imp.i; function f() { return +i } return f")(this, {i:1.4})), 1.4);
 assertEq(asmLink(asmCompile('global', 'imp', USE_ASM + "const i=+imp.i; function f() { return +i } return f")(this, {i:1.4})), 1.4);
 assertEq(asmLink(asmCompile(USE_ASM + "var g=0; function f() { var i=42; while (1) { break; } g = i; return g|0 } return f"))(), 42);
+assertAsmLinkFail(asmCompile('glob','foreign', USE_ASM + 'var i = +foreign.x; function f() {} return f'), null, {x:{valueOf:function() { return 42 }}});
+assertAsmLinkFail(asmCompile('glob','foreign', USE_ASM + 'var i = foreign.x|0; function f() {} return f'), null, {x:{valueOf:function() { return 42 }}});
+assertEq(asmLink(asmCompile('glob','foreign', USE_ASM + 'var i = foreign.x|0; function f() { return i|0} return f'), null, {x:"blah"})(), 0);
+assertEq(asmLink(asmCompile('glob','foreign', USE_ASM + 'var i = +foreign.x; function f() { return +i} return f'), null, {x:"blah"})(), NaN);
+assertEq(asmLink(asmCompile('glob','foreign', USE_ASM + 'var tof = glob.Math.fround; var i = tof(foreign.x); function f() { return +i} return f'), this, {x:"blah"})(), NaN);
+assertThrowsInstanceOf(() => asmCompile('glob','foreign',USE_ASM + 'var i = foreign.x|0; function f() { return i|0} return f')(null, {x:Symbol("blah")}), TypeError);
+assertThrowsInstanceOf(() => asmCompile('glob','foreign',USE_ASM + 'var i = +foreign.x; function f() { return +i} return f')(null, {x:Symbol("blah")}), TypeError);
+assertThrowsInstanceOf(() => asmCompile('glob','foreign',USE_ASM + 'var tof = glob.Math.fround; var i = tof(foreign.x); function f() { return +i} return f')(this, {x:Symbol("blah")}), TypeError);
+
+// Temporary workaround; this test can be removed when Emscripten is fixed and
+// the fix has propagated out to most apps:
+assertEq(asmLink(asmCompile('glob','foreign', USE_ASM + 'var i = foreign.i|0; function f() { return i|0} return f'), null, {i:function(){}})(), 0);
+assertEq(asmLink(asmCompile('glob','foreign', USE_ASM + 'var i = +foreign.i; function f() { return +i} return f'), null, {i:function(){}})(), NaN);
+assertEq(asmLink(asmCompile('glob','foreign', USE_ASM + 'var tof = glob.Math.fround; var i = tof(foreign.i); function f() { return +i} return f'), this, {i:function(){}})(), NaN);
+var module = asmCompile('glob','foreign', USE_ASM + 'var i = foreign.i|0; function f() { return i|0} return f');
+var fun = function() {}
+fun.valueOf = function() {}
+assertAsmLinkFail(module, null, {i:fun});
+var fun = function() {}
+fun.toString = function() {}
+assertAsmLinkFail(module, null, {i:fun});
+var fun = function() {}
+var origFunToString = Function.prototype.toString;
+Function.prototype.toString = function() {}
+assertAsmLinkFail(module, null, {i:fun});
+Function.prototype.toString = origFunToString;
+assertEq(asmLink(module, null, {i:fun})(), 0);
+Function.prototype.valueOf = function() {}
+assertAsmLinkFail(module, null, {i:fun});
+delete Function.prototype.valueOf;
+assertEq(asmLink(module, null, {i:fun})(), 0);
+var origObjValueOf = Object.prototype.valueOf;
+Object.prototype.valueOf = function() {}
+assertAsmLinkFail(module, null, {i:fun});
+Object.prototype.valueOf = origObjValueOf;
+assertEq(asmLink(module, null, {i:fun})(), 0);
+Object.prototype.toString = function() {}
+assertEq(asmLink(module, null, {i:fun})(), 0);
 
 var f1 = asmCompile('global', 'foreign', 'heap', USE_ASM + 'var i32 = new global.Int32Array(heap); function g() { return i32[4]|0 } return g');
 var global = this;

js/src/jsfun.cpp

@@ -1183,8 +1183,8 @@ fun_toStringHelper(JSContext *cx, HandleObject obj, unsigned indent)
     return FunctionToString(cx, fun, false, indent != JS_DONT_PRETTY_PRINT);
 }
 
-static bool
-fun_toString(JSContext *cx, unsigned argc, Value *vp)
+bool
+js::fun_toString(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     JS_ASSERT(IsFunctionObject(args.calleev()));

js/src/jsfun.h

@@ -533,6 +533,9 @@ FunctionHasResolveHook(const JSAtomState &atomState, PropertyName *name);
 extern bool
 fun_resolve(JSContext *cx, HandleObject obj, HandleId id, MutableHandleObject objp);
 
+extern bool
+fun_toString(JSContext *cx, unsigned argc, Value *vp);
+
 /*
  * Function extended with reserved slots for use by various kinds of functions.
  * Most functions do not have these extensions, but enough do that efficient

js/src/jsobjinlines.h

@@ -805,7 +805,28 @@ ClassMethodIsNative(JSContext *cx, JSObject *obj, const Class *clasp, jsid metho
             return false;
     }
 
-    return js::IsNativeFunction(v, native);
+    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.
+static MOZ_ALWAYS_INLINE bool
+HasObjectValueOf(JSObject *obj, JSContext *cx)
+{
+    if (obj->is<ProxyObject>() || !obj->isNative())
+        return false;
+
+    jsid valueOf = NameToId(cx->names().valueOf);
+
+    Value v;
+    while (!HasDataProperty(cx, obj, valueOf, &v)) {
+        obj = obj->getProto();
+        if (!obj || obj->is<ProxyObject>() || !obj->isNative())
+            return false;
+    }
+
+    return IsNativeFunction(v, obj_valueOf);
 }
 
 /* ES5 9.1 ToPrimitive(input). */