instanceof should not get the prototype for non-default HasInstance
https://bugs.webkit.org/show_bug.cgi?id=68656
Reviewed by Oliver Hunt.
Source/JavaScriptCore:
Instanceof is currently implemented as a sequance of three opcodes:
check_has_instance
get_by_id(prototype)
op_instanceof
There are three interesting types of base value that instanceof can be applied to:
(A) Objects supporting default instanceof behaviour (functions, other than those created with bind)
(B) Objects overriding the default instancecof behaviour with a custom one (API objects, bound functions)
(C) Values that do not respond to the [[HasInstance]] trap.
Currently check_has_instance handles case (C), leaving the op_instanceof opcode to handle (A) & (B). There are
two problems with this apporach. Firstly, this is suboptimal for case (A), since we have to check for
hasInstance support twice (once in check_has_instance, then for default behaviour in op_instanceof). Secondly,
this means that in cases (B) we also perform the get_by_id, which is both suboptimal and an observable spec
violation.
The fix here is to move handing of non-default instanceof (cases (B)) to the check_has_instance op, leaving
op_instanceof to handle only cases (A).
* API/JSCallbackObject.h:
(JSCallbackObject):
* API/JSCallbackObjectFunctions.h:
(JSC::::customHasInstance):
* API/JSValueRef.cpp:
(JSValueIsInstanceOfConstructor):
- renamed hasInstance to customHasInstance
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::dump):
- added additional parameters to check_has_instance opcode
* bytecode/Opcode.h:
(JSC):
(JSC::padOpcodeName):
- added additional parameters to check_has_instance opcode
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::emitCheckHasInstance):
- added additional parameters to check_has_instance opcode
* bytecompiler/BytecodeGenerator.h:
(BytecodeGenerator):
- added additional parameters to check_has_instance opcode
* bytecompiler/NodesCodegen.cpp:
(JSC::InstanceOfNode::emitBytecode):
- added additional parameters to check_has_instance opcode
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
- added additional parameters to check_has_instance opcode
* interpreter/Interpreter.cpp:
(JSC::isInvalidParamForIn):
(JSC::Interpreter::privateExecute):
- Add handling for non-default instanceof to op_check_has_instance
* jit/JITInlineMethods.h:
(JSC::JIT::emitArrayProfilingSiteForBytecodeIndex):
- Fixed no-LLInt no_DFG build
* jit/JITOpcodes.cpp:
(JSC::JIT::emit_op_check_has_instance):
(JSC::JIT::emitSlow_op_check_has_instance):
- check for ImplementsDefaultHasInstance, handle additional arguments to op_check_has_instance.
(JSC::JIT::emit_op_instanceof):
(JSC::JIT::emitSlow_op_instanceof):
- no need to check for ImplementsDefaultHasInstance.
* jit/JITOpcodes32_64.cpp:
(JSC::JIT::emit_op_check_has_instance):
(JSC::JIT::emitSlow_op_check_has_instance):
- check for ImplementsDefaultHasInstance, handle additional arguments to op_check_has_instance.
(JSC::JIT::emit_op_instanceof):
(JSC::JIT::emitSlow_op_instanceof):
- no need to check for ImplementsDefaultHasInstance.
* jit/JITStubs.cpp:
(JSC::DEFINE_STUB_FUNCTION):
* jit/JITStubs.h:
- Add handling for non-default instanceof to op_check_has_instance
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::LLINT_SLOW_PATH_DECL):
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
- move check for ImplementsDefaultHasInstance, handle additional arguments to op_check_has_instance.
* runtime/ClassInfo.h:
(MethodTable):
(JSC):
- renamed hasInstance to customHasInstance
* runtime/CommonSlowPaths.h:
(CommonSlowPaths):
- removed opInstanceOfSlow (this was whittled down to one function call!)
* runtime/JSBoundFunction.cpp:
(JSC::JSBoundFunction::customHasInstance):
* runtime/JSBoundFunction.h:
(JSBoundFunction):
- renamed hasInstance to customHasInstance, reimplemented.
* runtime/JSCell.cpp:
(JSC::JSCell::customHasInstance):
* runtime/JSCell.h:
(JSCell):
* runtime/JSObject.cpp:
(JSC::JSObject::hasInstance):
(JSC):
(JSC::JSObject::defaultHasInstance):
* runtime/JSObject.h:
(JSObject):
LayoutTests:
* fast/js/function-bind-expected.txt:
- check in passing result.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@129281 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/runtime/ClassInfo.h b/Source/JavaScriptCore/runtime/ClassInfo.h
index 0e1747b..e8823d5 100644
--- a/Source/JavaScriptCore/runtime/ClassInfo.h
+++ b/Source/JavaScriptCore/runtime/ClassInfo.h
@@ -81,8 +81,8 @@
typedef String (*ClassNameFunctionPtr)(const JSObject*);
ClassNameFunctionPtr className;
- typedef bool (*HasInstanceFunctionPtr)(JSObject*, ExecState*, JSValue, JSValue);
- HasInstanceFunctionPtr hasInstance;
+ typedef bool (*CustomHasInstanceFunctionPtr)(JSObject*, ExecState*, JSValue);
+ CustomHasInstanceFunctionPtr customHasInstance;
typedef void (*PutWithAttributesFunctionPtr)(JSObject*, ExecState*, PropertyName propertyName, JSValue, unsigned attributes);
PutWithAttributesFunctionPtr putDirectVirtual;
@@ -130,7 +130,7 @@
&ClassName::getOwnNonIndexPropertyNames, \
&ClassName::getPropertyNames, \
&ClassName::className, \
- &ClassName::hasInstance, \
+ &ClassName::customHasInstance, \
&ClassName::putDirectVirtual, \
&ClassName::defineOwnProperty, \
&ClassName::getOwnPropertyDescriptor, \
diff --git a/Source/JavaScriptCore/runtime/CommonSlowPaths.h b/Source/JavaScriptCore/runtime/CommonSlowPaths.h
index e4c76ad..7edd909 100644
--- a/Source/JavaScriptCore/runtime/CommonSlowPaths.h
+++ b/Source/JavaScriptCore/runtime/CommonSlowPaths.h
@@ -75,28 +75,6 @@
return newExec;
}
-ALWAYS_INLINE bool opInstanceOfSlow(ExecState* exec, JSValue value, JSValue baseVal, JSValue proto)
-{
- ASSERT(!value.isCell() || !baseVal.isCell() || !proto.isCell()
- || !value.isObject() || !baseVal.isObject() || !proto.isObject()
- || !asObject(baseVal)->structure()->typeInfo().implementsDefaultHasInstance());
-
-
- // ECMA-262 15.3.5.3:
- // Throw an exception either if baseVal is not an object, or if it does not implement 'HasInstance' (i.e. is a function).
- TypeInfo typeInfo(UnspecifiedType);
- if (!baseVal.isObject() || !(typeInfo = asObject(baseVal)->structure()->typeInfo()).implementsHasInstance()) {
- exec->globalData().exception = createInvalidParamError(exec, "instanceof", baseVal);
- return false;
- }
- ASSERT(typeInfo.type() != UnspecifiedType);
-
- if (!typeInfo.overridesHasInstance() && !value.isObject())
- return false;
-
- return asObject(baseVal)->methodTable()->hasInstance(asObject(baseVal), exec, value, proto);
-}
-
inline bool opIn(ExecState* exec, JSValue propName, JSValue baseVal)
{
if (!baseVal.isObject()) {
diff --git a/Source/JavaScriptCore/runtime/JSBoundFunction.cpp b/Source/JavaScriptCore/runtime/JSBoundFunction.cpp
index 08d6831..3815c14 100644
--- a/Source/JavaScriptCore/runtime/JSBoundFunction.cpp
+++ b/Source/JavaScriptCore/runtime/JSBoundFunction.cpp
@@ -89,14 +89,9 @@
return function;
}
-bool JSBoundFunction::hasInstance(JSObject* object, ExecState* exec, JSValue value, JSValue)
+bool JSBoundFunction::customHasInstance(JSObject* object, ExecState* exec, JSValue value)
{
- JSBoundFunction* thisObject = jsCast<JSBoundFunction*>(object);
- // FIXME: our instanceof implementation will have already (incorrectly) performed
- // a [[Get]] of .prototype from the bound function object, which is incorrect!
- // https://bugs.webkit.org/show_bug.cgi?id=68656
- JSValue proto = thisObject->m_targetFunction->get(exec, exec->propertyNames().prototype);
- return thisObject->m_targetFunction->methodTable()->hasInstance(thisObject->m_targetFunction.get(), exec, value, proto);
+ return jsCast<JSBoundFunction*>(object)->m_targetFunction->hasInstance(exec, value);
}
JSBoundFunction::JSBoundFunction(ExecState* exec, JSGlobalObject* globalObject, Structure* structure, JSObject* targetFunction, JSValue boundThis, JSValue boundArgs)
diff --git a/Source/JavaScriptCore/runtime/JSBoundFunction.h b/Source/JavaScriptCore/runtime/JSBoundFunction.h
index 5067d19..05a6ad8 100644
--- a/Source/JavaScriptCore/runtime/JSBoundFunction.h
+++ b/Source/JavaScriptCore/runtime/JSBoundFunction.h
@@ -39,7 +39,7 @@
static JSBoundFunction* create(ExecState*, JSGlobalObject*, JSObject* targetFunction, JSValue boundThis, JSValue boundArgs, int, const String&);
- static bool hasInstance(JSObject*, ExecState*, JSValue, JSValue proto);
+ static bool customHasInstance(JSObject*, ExecState*, JSValue);
JSObject* targetFunction() { return m_targetFunction.get(); }
JSValue boundThis() { return m_boundThis.get(); }
diff --git a/Source/JavaScriptCore/runtime/JSCell.cpp b/Source/JavaScriptCore/runtime/JSCell.cpp
index e050a53..739247f 100644
--- a/Source/JavaScriptCore/runtime/JSCell.cpp
+++ b/Source/JavaScriptCore/runtime/JSCell.cpp
@@ -199,7 +199,7 @@
ASSERT_NOT_REACHED();
}
-bool JSCell::hasInstance(JSObject*, ExecState*, JSValue, JSValue)
+bool JSCell::customHasInstance(JSObject*, ExecState*, JSValue)
{
ASSERT_NOT_REACHED();
return false;
diff --git a/Source/JavaScriptCore/runtime/JSCell.h b/Source/JavaScriptCore/runtime/JSCell.h
index d6abcba..cf6f4ec 100644
--- a/Source/JavaScriptCore/runtime/JSCell.h
+++ b/Source/JavaScriptCore/runtime/JSCell.h
@@ -160,7 +160,7 @@
static NO_RETURN_DUE_TO_ASSERT void getOwnNonIndexPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
static NO_RETURN_DUE_TO_ASSERT void getPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
static String className(const JSObject*);
- static bool hasInstance(JSObject*, ExecState*, JSValue, JSValue prototypeProperty);
+ JS_EXPORT_PRIVATE static bool customHasInstance(JSObject*, ExecState*, JSValue);
static NO_RETURN_DUE_TO_ASSERT void putDirectVirtual(JSObject*, ExecState*, PropertyName, JSValue, unsigned attributes);
static bool defineOwnProperty(JSObject*, ExecState*, PropertyName, PropertyDescriptor&, bool shouldThrow);
static bool getOwnPropertyDescriptor(JSObject*, ExecState*, PropertyName, PropertyDescriptor&);
diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp
index 2bea795..acff8a6 100644
--- a/Source/JavaScriptCore/runtime/JSObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSObject.cpp
@@ -783,7 +783,18 @@
return 0;
}
-bool JSObject::hasInstance(JSObject*, ExecState* exec, JSValue value, JSValue proto)
+bool JSObject::hasInstance(ExecState* exec, JSValue value)
+{
+ TypeInfo info = structure()->typeInfo();
+ if (info.implementsDefaultHasInstance())
+ return defaultHasInstance(exec, value, get(exec, exec->propertyNames().prototype));
+ if (info.implementsHasInstance())
+ return methodTable()->customHasInstance(this, exec, value);
+ throwError(exec, createInvalidParamError(exec, "instanceof" , this));
+ return false;
+}
+
+bool JSObject::defaultHasInstance(ExecState* exec, JSValue value, JSValue proto)
{
if (!value.isObject())
return false;
diff --git a/Source/JavaScriptCore/runtime/JSObject.h b/Source/JavaScriptCore/runtime/JSObject.h
index de5935f..bb59eb3 100644
--- a/Source/JavaScriptCore/runtime/JSObject.h
+++ b/Source/JavaScriptCore/runtime/JSObject.h
@@ -306,7 +306,8 @@
JS_EXPORT_PRIVATE static JSValue defaultValue(const JSObject*, ExecState*, PreferredPrimitiveType);
- JS_EXPORT_PRIVATE static bool hasInstance(JSObject*, ExecState*, JSValue, JSValue prototypeProperty);
+ bool hasInstance(ExecState*, JSValue);
+ static bool defaultHasInstance(ExecState*, JSValue, JSValue prototypeProperty);
JS_EXPORT_PRIVATE static void getOwnPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
JS_EXPORT_PRIVATE static void getOwnNonIndexPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
