From 09256df984b983718359a82d2a9738aa88999e1c Mon Sep 17 00:00:00 2001 From: Cameron Kaiser Date: Fri, 26 Oct 2018 21:21:20 -0700 Subject: [PATCH] #527: M1054906 M1278599 M1252228 (+ESR45 changes, see M1263778 for info) --- js/src/jit/BaselineIC.cpp | 14 +++ js/src/jit/IonBuilder.cpp | 26 +++++ js/src/jit/MIR.cpp | 2 +- js/src/jsapi.h | 1 + js/src/jsfun.cpp | 82 ++++++++++++++-- js/src/jsfun.h | 11 +++ .../ecma_6/Function/has-instance-jitted.js | 96 +++++++++++++++++++ js/src/tests/ecma_6/Function/has-instance.js | 88 +++++++++++++++++ js/src/tests/ecma_6/Symbol/well-known.js | 4 + js/src/vm/CommonPropertyNames.h | 3 +- js/src/vm/Interpreter.cpp | 43 ++++++++- js/src/vm/Interpreter.h | 3 + js/src/vm/Runtime.h | 4 + 13 files changed, 363 insertions(+), 14 deletions(-) create mode 100644 js/src/tests/ecma_6/Function/has-instance-jitted.js create mode 100644 js/src/tests/ecma_6/Function/has-instance.js diff --git a/js/src/jit/BaselineIC.cpp b/js/src/jit/BaselineIC.cpp index d00641521..24437ae47 100644 --- a/js/src/jit/BaselineIC.cpp +++ b/js/src/jit/BaselineIC.cpp @@ -8029,6 +8029,20 @@ TryAttachInstanceOfStub(JSContext* cx, BaselineFrame* frame, ICInstanceOf_Fallba if (fun->isBoundFunction()) return true; + // If the user has supplied their own @@hasInstance method we shouldn't + // clobber it. + if (!js::FunctionHasDefaultHasInstance(fun, cx->wellKnownSymbols())) + return true; + + // Refuse to optimize any function whose [[Prototype]] isn't + // Function.prototype. + if (fun->hasLazyPrototype() || fun->hasUncacheableProto()) + return true; + + Value funProto = cx->global()->getPrototype(JSProto_Function); + if (funProto.isObject() && fun->getProto() != &funProto.toObject()) + return true; + Shape* shape = fun->lookupPure(cx->names().prototype); if (!shape || !shape->hasSlot() || !shape->hasDefaultGetter()) return true; diff --git a/js/src/jit/IonBuilder.cpp b/js/src/jit/IonBuilder.cpp index 31f199361..a57760bcb 100644 --- a/js/src/jit/IonBuilder.cpp +++ b/js/src/jit/IonBuilder.cpp @@ -13440,10 +13440,36 @@ IonBuilder::jsop_instanceof() if (!rhsObject || !rhsObject->is() || rhsObject->isBoundFunction()) break; + // Refuse to optimize anything whose [[Prototype]] isn't Function.prototype + // since we can't guarantee that it uses the default @@hasInstance method. + if (rhsObject->hasUncacheableProto() || rhsObject->hasLazyPrototype()) + break; + + Value funProto = script()->global().getPrototype(JSProto_Function); + if (!funProto.isObject() || rhsObject->getProto() != &funProto.toObject()) + break; + + // If the user has supplied their own @@hasInstance method we shouldn't + // clobber it. + JSFunction* fun = &rhsObject->as(); + const WellKnownSymbols* symbols = &compartment->runtime()->wellKnownSymbols(); + if (!js::FunctionHasDefaultHasInstance(fun, *symbols)) + break; + + // Ensure that we will bail if the @@hasInstance property or [[Prototype]] + // change. TypeSet::ObjectKey* rhsKey = TypeSet::ObjectKey::get(rhsObject); + if (!rhsKey->hasStableClassAndProto(constraints())) + break; + if (rhsKey->unknownProperties()) break; + HeapTypeSetKey hasInstanceObject = + rhsKey->property(SYMBOL_TO_JSID(symbols->hasInstance)); + if (hasInstanceObject.isOwnProperty(constraints())) + break; + HeapTypeSetKey protoProperty = rhsKey->property(NameToId(names().prototype)); JSObject* protoObject = protoProperty.singleton(constraints()); diff --git a/js/src/jit/MIR.cpp b/js/src/jit/MIR.cpp index 0c0d2d91a..e757498c6 100644 --- a/js/src/jit/MIR.cpp +++ b/js/src/jit/MIR.cpp @@ -5154,7 +5154,7 @@ jit::PropertyReadNeedsTypeBarrier(JSContext* propertycx, TypeSet::TypeList types; if (!property.maybeTypes()->enumerateTypes(&types)) break; - if (types.length()) { + if (types.length() == 1) { // Note: the return value here is ignored. observed->addType(types[0], GetJitContext()->temp->lifoAlloc()); break; diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 8ad68a24c..008ab5c7d 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -4560,6 +4560,7 @@ GetSymbolDescription(HandleSymbol symbol); macro(iterator) \ macro(match) \ macro(species) \ + macro(hasInstance) \ macro(toPrimitive) \ macro(toStringTag) \ macro(unscopables) diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index d2cd4f116..0a333ac59 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -644,23 +644,73 @@ js::XDRInterpretedFunction(XDRState*, HandleObject, HandleScript, Mu template bool js::XDRInterpretedFunction(XDRState*, HandleObject, HandleScript, MutableHandleFunction); +/* ES6 (04-25-16) 19.2.3.6 Function.prototype [ @@hasInstance ] */ +bool +js::fun_symbolHasInstance(JSContext* cx, unsigned argc, Value* vp) +{ + CallArgs args = CallArgsFromVp(argc, vp); + + if (args.length() < 1) { + args.rval().setBoolean(false); + return true; + } + + /* Step 1. */ + HandleValue func = args.thisv(); + + // Primitives are non-callable and will always return false from + // OrdinaryHasInstance. + if (!func.isObject()) { + args.rval().setBoolean(false); + return true; + } + + RootedObject obj(cx, &func.toObject()); + RootedValue v(cx, args[0]); + + /* Step 2. */ + bool result; + if (!OrdinaryHasInstance(cx, obj, &v, &result)) + return false; + + args.rval().setBoolean(result); + return true; +} + /* - * [[HasInstance]] internal method for Function objects: fetch the .prototype - * property of its 'this' parameter, and walks the prototype chain of v (only - * if v is an object) returning true if .prototype is found. + * ES6 (4-25-16) 7.3.19 OrdinaryHasInstance */ -static bool -fun_hasInstance(JSContext* cx, HandleObject objArg, MutableHandleValue v, bool* bp) + +bool +js::OrdinaryHasInstance(JSContext* cx, HandleObject objArg, MutableHandleValue v, bool* bp) { RootedObject obj(cx, objArg); - while (obj->is() && obj->isBoundFunction()) - obj = obj->as().getBoundFunctionTarget(); + /* Step 1. */ + if (!obj->isCallable()) { + *bp = false; + return true; + } + /* Step 2. */ + if (obj->is() && obj->isBoundFunction()) { + /* Steps 2a-b. */ + obj = obj->as().getBoundFunctionTarget(); + return InstanceOfOperator(cx, obj, v, bp); + } + + /* Step 3. */ + if (!v.isObject()) { + *bp = false; + return true; + } + + /* Step 4. */ RootedValue pval(cx); if (!GetProperty(cx, obj, obj, cx->names().prototype, &pval)) return false; + /* Step 5. */ if (pval.isPrimitive()) { /* * Throw a runtime error if instanceof is called on a function that @@ -671,6 +721,7 @@ fun_hasInstance(JSContext* cx, HandleObject objArg, MutableHandleValue v, bool* return false; } + /* Step 6. */ RootedObject pobj(cx, &pval.toObject()); bool isDelegate; if (!IsDelegate(cx, pobj, v, &isDelegate)) @@ -835,7 +886,7 @@ const Class JSFunction::class_ = { fun_mayResolve, nullptr, /* finalize */ nullptr, /* call */ - fun_hasInstance, + nullptr, nullptr, /* construct */ fun_trace, { @@ -1122,6 +1173,20 @@ fun_toStringHelper(JSContext* cx, HandleObject obj, unsigned indent) return FunctionToString(cx, fun, indent != JS_DONT_PRETTY_PRINT); } +bool +js::FunctionHasDefaultHasInstance(JSFunction* fun, const WellKnownSymbols& symbols) +{ + jsid id = SYMBOL_TO_JSID(symbols.hasInstance); + Shape* shape = fun->lookupPure(id); + if (shape) { + if (!shape->hasSlot() || !shape->hasDefaultGetter()) + return false; + const Value hasInstance = fun->as().getSlot(shape->slot()); + return IsNativeFunction(hasInstance, js::fun_symbolHasInstance); + } + return true; +} + bool js::fun_toString(JSContext* cx, unsigned argc, Value* vp) { @@ -1733,6 +1798,7 @@ const JSFunctionSpec js::function_methods[] = { JS_FN(js_call_str, fun_call, 1,0), JS_FN("bind", fun_bind, 1,0), JS_FN("isGenerator", fun_isGenerator,0,0), + JS_SYM_FN(hasInstance, fun_symbolHasInstance, 1, JSPROP_READONLY | JSPROP_PERMANENT), JS_FS_END }; diff --git a/js/src/jsfun.h b/js/src/jsfun.h index 606b77161..184843774 100644 --- a/js/src/jsfun.h +++ b/js/src/jsfun.h @@ -692,6 +692,17 @@ fun_toString(JSContext* cx, unsigned argc, Value* vp); extern bool fun_bind(JSContext* cx, unsigned argc, Value* vp); +struct WellKnownSymbols; + +extern bool +FunctionHasDefaultHasInstance(JSFunction* fun, const WellKnownSymbols& symbols); + +extern bool +fun_symbolHasInstance(JSContext* cx, unsigned argc, Value* vp); + +extern bool +OrdinaryHasInstance(JSContext* cx, HandleObject objArg, MutableHandleValue v, bool* bp); + /* * Function extended with reserved slots for use by various kinds of functions. * Most functions do not have these extensions, but enough do that efficient diff --git a/js/src/tests/ecma_6/Function/has-instance-jitted.js b/js/src/tests/ecma_6/Function/has-instance-jitted.js new file mode 100644 index 000000000..a2d33abc7 --- /dev/null +++ b/js/src/tests/ecma_6/Function/has-instance-jitted.js @@ -0,0 +1,96 @@ +const OriginalHasInstance = Function.prototype[Symbol.hasInstance]; + +// Ensure that folding doesn't impact user defined @@hasInstance methods. +{ + function Test() { + this.x = 1; + } + + Object.defineProperty(Test, Symbol.hasInstance, + {writable: true, value: () => false}); + + function x(t) { + return t instanceof Test; + } + + function y() { + let t = new Test; + let b = true; + for (let i = 0; i < 10; i++) { + b = b && x(t); + } + return b; + } + + + function z() { + let f = 0; + let t = 0; + for (let i = 0; i < 100; i++) + assertEq(y(), false); + } + + z(); +} + +// Ensure that the jitting does not clobber user defined @@hasInstance methods. +{ + function a() { + function b() {}; + b.__proto__ = a.prototype; + return b; + }; + let c = new a(); + + let t = 0; + let f = 0; + let e = 0; + for (let i = 0; i < 40000; i++) { + if (i == 20000) + Object.defineProperty(a.prototype, Symbol.hasInstance, + {writable: true, value: () => true}); + if (i == 30000) + Object.setPrototypeOf(c, Function.prototype); + + if (1 instanceof c) { + t++; + } else { + f++; + } + } + + assertEq(t, 10000); + assertEq(f, 30000); +} + +{ + function a() {}; + function b() {}; + Object.defineProperty(a, Symbol.hasInstance, {writable: true, value: () => true}); + assertEq(b instanceof a, true); + for (let _ of Array(10000)) + assertEq(b instanceof a, true); +} + +{ + function a(){}; + function b(){}; + function c(){}; + function d(){}; + function e(){}; + Object.defineProperty(a, Symbol.hasInstance, {value: () => true }); + Object.defineProperty(b, Symbol.hasInstance, {value: () => true }); + Object.defineProperty(c, Symbol.hasInstance, {value: () => true }); + Object.defineProperty(d, Symbol.hasInstance, {value: () => true }); + let funcs = [a, b, c, d]; + for (let f of funcs) + assertEq(e instanceof f, true); + + for (let _ of Array(10001)) { + for (let f of funcs) + assertEq(e instanceof f, true); + } +} + +if (typeof reportCompare === "function") + reportCompare(true, true); diff --git a/js/src/tests/ecma_6/Function/has-instance.js b/js/src/tests/ecma_6/Function/has-instance.js new file mode 100644 index 000000000..53ebf0432 --- /dev/null +++ b/js/src/tests/ecma_6/Function/has-instance.js @@ -0,0 +1,88 @@ +// It is possible to override Function.prototype[@@hasInstance]. +let passed = false; +let obj = { foo: true }; +let C = function(){}; + +Object.defineProperty(C, Symbol.hasInstance, { + value: function(inst) { passed = inst.foo; return false; } +}); + +assertEq(obj instanceof C, false); +assertEq(passed, true); + +{ + let obj = { + [Symbol.hasInstance](v) { return true; }, + }; + let whatevs = {}; + assertEq(whatevs instanceof obj, true); +} + +{ + + function zzzz() {}; + let xxxx = new zzzz(); + assertEq(xxxx instanceof zzzz, true); + assertEq(zzzz[Symbol.hasInstance](xxxx), true); + +} + +// Non-callable objects should return false. +const nonCallables = [ + 1, + undefined, + null, + "nope", +] + +for (let nonCallable of nonCallables) { + assertEq(nonCallable instanceof Function, false); + assertEq(nonCallable instanceof Object, false); +} + +// Non-callables should throw when used on the right hand side +// of `instanceof`. +assertThrowsInstanceOf(() => { + function foo() {}; + let obj = {}; + foo instanceof obj; +}, TypeError); + +// Non-callables do not throw for overridden methods +let o = {[Symbol.hasInstance](v) { return true; }} +assertEq(1 instanceof o, true); + +// Non-callables return false instead of an exception when +// Function.prototype[Symbol.hasInstance] is called directly. +for (let nonCallable of nonCallables) { + assertEq(Function.prototype[Symbol.hasInstance].call(nonCallable, Object), false); +} + +// It should be possible to call the Symbol.hasInstance method directly. +assertEq(Function.prototype[Symbol.hasInstance].call(Function, () => 1), true); +assertEq(Function.prototype[Symbol.hasInstance].call(Function, Object), true); +assertEq(Function.prototype[Symbol.hasInstance].call(Function, null), false); +assertEq(Function.prototype[Symbol.hasInstance].call(Function, Array), true); +assertEq(Function.prototype[Symbol.hasInstance].call(Object, Array), true); +assertEq(Function.prototype[Symbol.hasInstance].call(Array, Function), false); +assertEq(Function.prototype[Symbol.hasInstance].call(({}), Function), false); +assertEq(Function.prototype[Symbol.hasInstance].call(), false) +assertEq(Function.prototype[Symbol.hasInstance].call(({})), false) + +// Ensure that bound functions are unwrapped properly +let bindme = {x: function() {}}; +let instance = new bindme.x(); +let xOuter = bindme.x; +let bound = xOuter.bind(bindme); +let doubleBound = bound.bind(bindme); +let tripleBound = bound.bind(doubleBound); +assertEq(Function.prototype[Symbol.hasInstance].call(bound, instance), true); +assertEq(Function.prototype[Symbol.hasInstance].call(doubleBound, instance), true); +assertEq(Function.prototype[Symbol.hasInstance].call(tripleBound, instance), true); + +// Function.prototype[Symbol.hasInstance] is not configurable +let desc = Object.getOwnPropertyDescriptor(Function.prototype, Symbol.hasInstance) +assertEq(desc.configurable, false) + +if (typeof reportCompare === "function") + reportCompare(true, true); diff --git a/js/src/tests/ecma_6/Symbol/well-known.js b/js/src/tests/ecma_6/Symbol/well-known.js index 61ec8565f..b7a36bbb2 100644 --- a/js/src/tests/ecma_6/Symbol/well-known.js +++ b/js/src/tests/ecma_6/Symbol/well-known.js @@ -5,6 +5,10 @@ var names = [ "iterator", "match", "species", + "hasInstance", + "toPrimitive", + "toStringTag", + "unscopables" ]; for (var name of names) { diff --git a/js/src/vm/CommonPropertyNames.h b/js/src/vm/CommonPropertyNames.h index 2bba4fe64..70a3bfbb4 100644 --- a/js/src/vm/CommonPropertyNames.h +++ b/js/src/vm/CommonPropertyNames.h @@ -285,15 +285,16 @@ macro(iterator, iterator, "iterator") \ macro(match, match, "match") \ macro(species, species, "species") \ + macro(hasInstance, hasInstance, "hasInstance") \ macro(toPrimitive, toPrimitive, "toPrimitive") \ macro(toStringTag, toStringTag, "toStringTag") \ macro(unscopables, unscopables, "unscopables") \ /* Same goes for the descriptions of the well-known symbols. */ \ - macro(Symbol_hasInstance, Symbol_hasInstance, "Symbol.hasInstance") \ macro(Symbol_isConcatSpreadable, Symbol_isConcatSpreadable, "Symbol.isConcatSpreadable") \ macro(Symbol_iterator, Symbol_iterator, "Symbol.iterator") \ macro(Symbol_match, Symbol_match, "Symbol.match") \ macro(Symbol_species, Symbol_species, "Symbol.species") \ + macro(Symbol_hasInstance, Symbol_hasInstance, "Symbol.hasInstance") \ macro(Symbol_toPrimitive, Symbol_toPrimitive, "Symbol.toPrimitive") \ macro(Symbol_toStringTag, Symbol_toStringTag, "Symbol.toStringTag") \ macro(Symbol_unscopables, Symbol_unscopables, "Symbol.unscopables") \ diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 6326a325e..4bf6e96cf 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -685,6 +685,44 @@ js::Execute(JSContext* cx, HandleScript script, JSObject& scopeChainArg, Value* NullFramePtr() /* evalInFrame */, rval); } +/* + * ES6 (4-25-16) 12.10.4 InstanceofOperator + */ +extern bool +js::InstanceOfOperator(JSContext* cx, HandleObject obj, MutableHandleValue v, bool* bp) +{ + /* Step 1. is handled by caller. */ + + /* Step 2. */ + RootedValue hasInstance(cx); + RootedId id(cx, SYMBOL_TO_JSID(cx->wellKnownSymbols().hasInstance)); + if (!GetProperty(cx, obj, obj, id, &hasInstance)) + return false; + + if (!hasInstance.isNullOrUndefined()) { + if (!IsCallable(hasInstance)) + ReportIsNotFunction(cx, hasInstance); + + /* Step 3. */ + RootedValue rval(cx); + RootedValue vv(cx, v); + if (!Call(cx, obj, hasInstance, HandleValueArray(vv), &rval)) + return false; + *bp = ToBoolean(rval); + return true; + } + + /* Step 4. */ + if (!obj->isCallable()) { + RootedValue val(cx, ObjectValue(*obj)); + ReportIsNotFunction(cx, val); + return false; + } + + /* Step 5. */ + return js::OrdinaryHasInstance(cx, obj, v, bp); +} + bool js::HasInstance(JSContext* cx, HandleObject obj, HandleValue v, bool* bp) { @@ -693,10 +731,7 @@ js::HasInstance(JSContext* cx, HandleObject obj, HandleValue v, bool* bp) if (clasp->hasInstance) return clasp->hasInstance(cx, obj, &local, bp); - RootedValue val(cx, ObjectValue(*obj)); - ReportValueError(cx, JSMSG_BAD_INSTANCEOF_RHS, - JSDVG_SEARCH_STACK, val, nullptr); - return false; + return js::InstanceOfOperator(cx, obj, &local, bp); } static inline bool diff --git a/js/src/vm/Interpreter.h b/js/src/vm/Interpreter.h index 420854c48..b228edc3a 100644 --- a/js/src/vm/Interpreter.h +++ b/js/src/vm/Interpreter.h @@ -248,6 +248,9 @@ TypeOfObject(JSObject* obj); extern JSType TypeOfValue(const Value& v); +extern bool +InstanceOfOperator(JSContext* cx, HandleObject obj, MutableHandleValue v, bool* bp); + extern bool HasInstance(JSContext* cx, HandleObject obj, HandleValue v, bool* bp); diff --git a/js/src/vm/Runtime.h b/js/src/vm/Runtime.h index e7c8d187c..81e7dce6a 100644 --- a/js/src/vm/Runtime.h +++ b/js/src/vm/Runtime.h @@ -456,6 +456,10 @@ struct WellKnownSymbols const ImmutableSymbolPtr& get(JS::SymbolCode code) const { return get(size_t(code)); } + + WellKnownSymbols() {} + WellKnownSymbols(const WellKnownSymbols&) = delete; + WellKnownSymbols& operator=(const WellKnownSymbols&) = delete; }; #define NAME_OFFSET(name) offsetof(JSAtomState, name)