From 88db0108b14d58cf5d82ed7346f48f010feaaf0d Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Thu, 7 Feb 2019 10:39:40 +0100 Subject: Align `instanceof` with the final ES6 spec. --- js/src/jsapi.h | 7 ++++++ js/src/jsfun.cpp | 10 ++++----- js/src/jswrapper.h | 2 ++ js/src/proxy/OpaqueCrossCompartmentWrapper.cpp | 8 +++++++ js/src/proxy/ScriptedProxyHandler.cpp | 4 +--- js/src/vm/Interpreter.cpp | 14 ++++++------ js/src/vm/Interpreter.h | 3 --- js/xpconnect/wrappers/WaiveXrayWrapper.cpp | 31 ++++++++++++++++++++++++++ js/xpconnect/wrappers/WaiveXrayWrapper.h | 2 ++ js/xpconnect/wrappers/XrayWrapper.cpp | 14 ++++++++++++ js/xpconnect/wrappers/XrayWrapper.h | 2 ++ 11 files changed, 79 insertions(+), 18 deletions(-) diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 799396a0a..005d2278e 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -2154,6 +2154,13 @@ namespace JS { extern JS_PUBLIC_API(bool) OrdinaryHasInstance(JSContext* cx, HandleObject objArg, HandleValue v, bool* bp); +// Implementation of +// https://www.ecma-international.org/ecma-262/6.0/#sec-instanceofoperator +// This is almost identical to JS_HasInstance, except the latter may call a +// custom hasInstance class op instead of InstanceofOperator. +extern JS_PUBLIC_API(bool) +InstanceofOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp); + } // namespace JS extern JS_PUBLIC_API(void*) diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index bcb0da80b..863871df9 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -690,7 +690,7 @@ js::fun_symbolHasInstance(JSContext* cx, unsigned argc, Value* vp) } /* - * ES6 (4-25-16) 7.3.19 OrdinaryHasInstance + * ES6 7.3.19 OrdinaryHasInstance */ bool JS::OrdinaryHasInstance(JSContext* cx, HandleObject objArg, HandleValue v, bool* bp) @@ -707,7 +707,7 @@ JS::OrdinaryHasInstance(JSContext* cx, HandleObject objArg, HandleValue v, bool* if (obj->is() && obj->isBoundFunction()) { /* Steps 2a-b. */ obj = obj->as().getBoundFunctionTarget(); - return InstanceOfOperator(cx, obj, v, bp); + return InstanceofOperator(cx, obj, v, bp); } /* Step 3. */ @@ -716,12 +716,12 @@ JS::OrdinaryHasInstance(JSContext* cx, HandleObject objArg, HandleValue v, bool* return true; } - /* Step 4. */ + /* Step 4-5. */ RootedValue pval(cx); if (!GetProperty(cx, obj, obj, cx->names().prototype, &pval)) return false; - /* Step 5. */ + /* Step 6. */ if (pval.isPrimitive()) { /* * Throw a runtime error if instanceof is called on a function that @@ -732,7 +732,7 @@ JS::OrdinaryHasInstance(JSContext* cx, HandleObject objArg, HandleValue v, bool* return false; } - /* Step 6. */ + /* Step 7. */ RootedObject pobj(cx, &pval.toObject()); bool isDelegate; if (!IsDelegate(cx, pobj, v, &isDelegate)) diff --git a/js/src/jswrapper.h b/js/src/jswrapper.h index 3c73979f8..84ebe2732 100644 --- a/js/src/jswrapper.h +++ b/js/src/jswrapper.h @@ -270,6 +270,8 @@ class JS_FRIEND_API(OpaqueCrossCompartmentWrapper) : public CrossCompartmentWrap virtual bool getBuiltinClass(JSContext* cx, HandleObject wrapper, ESClass* cls) const override; virtual bool isArray(JSContext* cx, HandleObject obj, JS::IsArrayAnswer* answer) const override; + virtual bool hasInstance(JSContext* cx, HandleObject wrapper, + MutableHandleValue v, bool* bp) const override; virtual const char* className(JSContext* cx, HandleObject wrapper) const override; virtual JSString* fun_toString(JSContext* cx, HandleObject proxy, unsigned indent) const override; diff --git a/js/src/proxy/OpaqueCrossCompartmentWrapper.cpp b/js/src/proxy/OpaqueCrossCompartmentWrapper.cpp index ff3f4145c..02bf237ff 100644 --- a/js/src/proxy/OpaqueCrossCompartmentWrapper.cpp +++ b/js/src/proxy/OpaqueCrossCompartmentWrapper.cpp @@ -175,6 +175,14 @@ OpaqueCrossCompartmentWrapper::isArray(JSContext* cx, HandleObject obj, return true; } +bool OpaqueCrossCompartmentWrapper::hasInstance(JSContext* cx, + HandleObject wrapper, + MutableHandleValue v, + bool* bp) const { + *bp = false; + return true; +} + const char* OpaqueCrossCompartmentWrapper::className(JSContext* cx, HandleObject proxy) const diff --git a/js/src/proxy/ScriptedProxyHandler.cpp b/js/src/proxy/ScriptedProxyHandler.cpp index 776547337..0e25f470c 100644 --- a/js/src/proxy/ScriptedProxyHandler.cpp +++ b/js/src/proxy/ScriptedProxyHandler.cpp @@ -8,8 +8,6 @@ #include "jsapi.h" -#include "vm/Interpreter.h" // For InstanceOfOperator - #include "jsobjinlines.h" #include "vm/NativeObject-inl.h" @@ -1230,7 +1228,7 @@ bool ScriptedProxyHandler::hasInstance(JSContext* cx, HandleObject proxy, MutableHandleValue v, bool* bp) const { - return InstanceOfOperator(cx, proxy, v, bp); + return InstanceofOperator(cx, proxy, v, bp); } bool diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index b747e4d7a..e6d6630c4 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -718,14 +718,14 @@ js::Execute(JSContext* cx, HandleScript script, JSObject& envChainArg, Value* rv } /* - * ES6 (4-25-16) 12.10.4 InstanceofOperator + * ES6 12.9.4 InstanceofOperator */ extern bool -js::InstanceOfOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp) +JS::InstanceofOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp) { /* Step 1. is handled by caller. */ - /* Step 2. */ + /* Step 2-3. */ RootedValue hasInstance(cx); RootedId id(cx, SYMBOL_TO_JSID(cx->wellKnownSymbols().hasInstance)); if (!GetProperty(cx, obj, obj, id, &hasInstance)) @@ -735,7 +735,7 @@ js::InstanceOfOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp) if (!IsCallable(hasInstance)) return ReportIsNotFunction(cx, hasInstance); - /* Step 3. */ + /* Step 4. */ RootedValue rval(cx); if (!Call(cx, hasInstance, obj, v, &rval)) return false; @@ -743,13 +743,13 @@ js::InstanceOfOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp) return true; } - /* Step 4. */ + /* Step 5. */ if (!obj->isCallable()) { RootedValue val(cx, ObjectValue(*obj)); return ReportIsNotFunction(cx, val); } - /* Step 5. */ + /* Step 6. */ return OrdinaryHasInstance(cx, obj, v, bp); } @@ -760,7 +760,7 @@ js::HasInstance(JSContext* cx, HandleObject obj, HandleValue v, bool* bp) RootedValue local(cx, v); if (JSHasInstanceOp hasInstance = clasp->getHasInstance()) return hasInstance(cx, obj, &local, bp); - return js::InstanceOfOperator(cx, obj, local, bp); + 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 330dbef5f..9fefd75cc 100644 --- a/js/src/vm/Interpreter.h +++ b/js/src/vm/Interpreter.h @@ -322,9 +322,6 @@ TypeOfObject(JSObject* obj); extern JSType TypeOfValue(const Value& v); -extern bool -InstanceOfOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp); - extern bool HasInstance(JSContext* cx, HandleObject obj, HandleValue v, bool* bp); diff --git a/js/xpconnect/wrappers/WaiveXrayWrapper.cpp b/js/xpconnect/wrappers/WaiveXrayWrapper.cpp index 27c010d34..dca3daa58 100644 --- a/js/xpconnect/wrappers/WaiveXrayWrapper.cpp +++ b/js/xpconnect/wrappers/WaiveXrayWrapper.cpp @@ -87,6 +87,37 @@ WaiveXrayWrapper::nativeCall(JSContext* cx, JS::IsAcceptableThis test, WrapperFactory::WaiveXrayAndWrap(cx, args.rval()); } +bool +WaiveXrayWrapper::hasInstance(JSContext* cx, HandleObject wrapper, + MutableHandleValue v, bool* bp) const { + if (v.isObject() && WrapperFactory::IsXrayWrapper(&v.toObject())) { + // If |v| is an XrayWrapper and in the same compartment as the value + // wrapped by |wrapper|, then the Xrays of |v| would be waived upon + // calling CrossCompartmentWrapper::hasInstance. This may trigger + // getters and proxy traps of unwrapped |v|. To prevent that from + // happening, we exit early. + + // |wrapper| is the right operand of "instanceof", and must either be + // a function or an object with a @@hasInstance method. We are not going + // to call @@hasInstance, so only check whether it is a function. + // This check is here for consistency with usual "instanceof" behavior, + // which throws if the right operand is not a function. Without this + // check, the "instanceof" operator would return false and potentially + // hide errors in the code that uses the "instanceof" operator. + if (!JS::IsCallable(wrapper)) { + RootedValue wrapperv(cx, JS::ObjectValue(*wrapper)); + js::ReportIsNotFunction(cx, wrapperv); + return false; + } + + *bp = false; + return true; + } + + // Both |wrapper| and |v| have no Xrays here. + return CrossCompartmentWrapper::hasInstance(cx, wrapper, v, bp); +} + bool WaiveXrayWrapper::getPrototype(JSContext* cx, HandleObject wrapper, MutableHandleObject protop) const { diff --git a/js/xpconnect/wrappers/WaiveXrayWrapper.h b/js/xpconnect/wrappers/WaiveXrayWrapper.h index b0b447796..0f9675c17 100644 --- a/js/xpconnect/wrappers/WaiveXrayWrapper.h +++ b/js/xpconnect/wrappers/WaiveXrayWrapper.h @@ -36,6 +36,8 @@ class WaiveXrayWrapper : public js::CrossCompartmentWrapper { JS::MutableHandle objp) const override; virtual bool nativeCall(JSContext* cx, JS::IsAcceptableThis test, JS::NativeImpl impl, const JS::CallArgs& args) const override; + virtual bool hasInstance(JSContext* cx, JS::HandleObject wrapper, + JS::MutableHandleValue v, bool* bp) const override; virtual bool getPropertyDescriptor(JSContext* cx, JS::Handle wrapper, JS::Handle id, JS::MutableHandle desc) const override; diff --git a/js/xpconnect/wrappers/XrayWrapper.cpp b/js/xpconnect/wrappers/XrayWrapper.cpp index 48a9fdc68..6e5a2f5e5 100644 --- a/js/xpconnect/wrappers/XrayWrapper.cpp +++ b/js/xpconnect/wrappers/XrayWrapper.cpp @@ -2308,6 +2308,20 @@ XrayWrapper::getBuiltinClass(JSContext* cx, JS::HandleObject wrapp return Traits::getBuiltinClass(cx, wrapper, Base::singleton, cls); } +template +bool +XrayWrapper::hasInstance(JSContext* cx, + JS::HandleObject wrapper, + JS::MutableHandleValue v, + bool* bp) const { + assertEnteredPolicy(cx, wrapper, JSID_VOID, BaseProxyHandler::GET); + + // CrossCompartmentWrapper::hasInstance unwraps |wrapper|'s Xrays and enters + // its compartment. Any present XrayWrappers should be preserved, so the + // standard "instanceof" implementation is called without unwrapping first. + return JS::InstanceofOperator(cx, wrapper, v, bp); +} + template const char* XrayWrapper::className(JSContext* cx, HandleObject wrapper) const diff --git a/js/xpconnect/wrappers/XrayWrapper.h b/js/xpconnect/wrappers/XrayWrapper.h index 5630982c2..038d82390 100644 --- a/js/xpconnect/wrappers/XrayWrapper.h +++ b/js/xpconnect/wrappers/XrayWrapper.h @@ -482,6 +482,8 @@ class XrayWrapper : public Base { JS::AutoIdVector& props) const override; virtual bool getBuiltinClass(JSContext* cx, JS::HandleObject wapper, js::ESClass* cls) const override; + virtual bool hasInstance(JSContext* cx, JS::HandleObject wrapper, + JS::MutableHandleValue v, bool* bp) const override; virtual const char* className(JSContext* cx, JS::HandleObject proxy) const override; static const XrayWrapper singleton; -- cgit v1.2.3