Bug 216816 - An issue about evaluating instanceof
Summary: An issue about evaluating instanceof
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: PC Linux
: P2 Normal
Assignee: Alexey Shvayka
URL:
Keywords: InRadar
Depends on: 228763
Blocks:
  Show dependency treegraph
 
Reported: 2020-09-22 01:28 PDT by NWU_NISL
Modified: 2021-08-06 19:57 PDT (History)
17 users (show)

See Also:


Attachments
WIP (143.96 KB, patch)
2021-03-02 14:16 PST, Alexey Shvayka
no flags Details | Formatted Diff | Diff
WIP (163.64 KB, patch)
2021-07-12 11:20 PDT, Alexey Shvayka
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
WIP (164.22 KB, patch)
2021-07-12 15:09 PDT, Alexey Shvayka
no flags Details | Formatted Diff | Diff
WIP (167.41 KB, patch)
2021-07-15 13:01 PDT, Alexey Shvayka
no flags Details | Formatted Diff | Diff
WIP (169.59 KB, patch)
2021-07-16 07:20 PDT, Alexey Shvayka
no flags Details | Formatted Diff | Diff
WIP (169.64 KB, patch)
2021-07-16 12:47 PDT, Alexey Shvayka
no flags Details | Formatted Diff | Diff
WIP (171.89 KB, patch)
2021-07-17 19:26 PDT, Alexey Shvayka
no flags Details | Formatted Diff | Diff
WIP (177.50 KB, patch)
2021-07-22 18:30 PDT, Alexey Shvayka
no flags Details | Formatted Diff | Diff
Patch (190.09 KB, patch)
2021-08-06 19:57 PDT, Alexey Shvayka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description NWU_NISL 2020-09-22 01:28:19 PDT
According to ES10.0, If an object does not define or inherit "@@hasInstance" it uses the default "instanceof" semantics. When changing the value of "__proto__" of an object into "String", the algorithm to be used will be "Function.prototype[@@hasInstance]" instead of the default semantics. So the output of the testcase below is "false" as other engines do. This may be an issue of javascriptCore.

#### version
dbae081


#### command
webkit/WebKitBuild/Debug/bin/jsc testcase.js testcase.js


#### testcase
var  obj = {__proto__:String};
var result = "hello" instanceof obj;
print(result);


#### output
TypeError: obj is not a function. (evaluating '"hello" instanceof obj')


#### expected output
false


contributor:Yuan Wang
Comment 1 Radar WebKit Bug Importer 2020-09-22 15:42:42 PDT
<rdar://problem/69396430>
Comment 2 NWU_NISL 2020-09-24 20:31:27 PDT
sorry, I accidentally entered the wrong command.
#### Right command
webkit/WebKitBuild/Debug/bin/jsc testcase.js
Comment 3 Alexey Shvayka 2020-09-29 12:59:11 PDT
(In reply to NWU_NISL from comment #0)
> #### testcase
> var  obj = {__proto__:String};
> var result = "hello" instanceof obj;
> print(result);

https://test262.report/browse/language/expressions/instanceof/prototype-getter-with-primitive.js failure seems to be related.

To fix this, JSObject::{hasInstance,defaultHasInstance} should be redesigned to accept a constructor instead of a prototype.
Also, we'd need to make sure that primitives are correctly handled by JIT tiers.
Comment 4 Yusuke Suzuki 2021-03-02 14:02:50 PST
Yes, and if it is possible, we should avoid bloating bytecodes for instanceof. Currently, our implementation is not so good...
Comment 5 Alexey Shvayka 2021-03-02 14:13:10 PST
(In reply to Yusuke Suzuki from comment #4)
> Yes, and if it is possible, we should avoid bloating bytecodes for
> instanceof. Currently, our implementation is not so good...

Absolutely, it is the bytecode count that makes this change quite complicated.
The best I could do is +1.
Comment 6 Alexey Shvayka 2021-03-02 14:16:18 PST
Created attachment 421996 [details]
WIP
Comment 7 Alexey Shvayka 2021-07-12 11:20:07 PDT
Created attachment 433333 [details]
WIP
Comment 8 Alexey Shvayka 2021-07-12 15:09:32 PDT
Created attachment 433364 [details]
WIP

Introduce JSCell::isJSFunction(), fix ASSERT in emitNodeInTailPosition() and use correct uint8_t comparison in LLInt's op_is_cell_with_type.
Comment 9 Alexey Shvayka 2021-07-15 13:01:30 PDT
Created attachment 433615 [details]
WIP

Don't use 'bba' in LLInt in attempt to fix MASM build, fix JSCallbackObject<Parent>::customHasInstance() to use callFrame->thisValue(), fix JSValueIsInstanceOfConstructor to call hasInstance(), don't perform defaultHasInstance() for JSCallbackConstructor as it's non-callable, and swap arguments inside checkForbiddenPrototype().
Comment 10 Alexey Shvayka 2021-07-16 07:20:08 PDT
Created attachment 433675 [details]
WIP

Revert r161564 and introduce isNonCallableAPIObjectThatPerformsDefaultHasInstance() instead to ensure correct 'instanceof' behavior for API objects, fix arguments for operationInstanceOfOptimize() to fix 32-bit build, and replace 'bbb' with LLInt instruction that sets value in attempt to fix MASM build.
Comment 11 Alexey Shvayka 2021-07-16 12:47:37 PDT
Created attachment 433694 [details]
WIP

Bring back accidently removed throwException() to JSObject::hasInstance(), remove UntypedUse assert in compileInstanceOf() from 32-bit builds, and hoist 'getu' LLInt instructions in attempt to fix MASM build.
Comment 12 Alexey Shvayka 2021-07-17 19:26:00 PDT
Created attachment 433737 [details]
WIP

Introduce _llint_slow_path_is_cell_with_type for MSVC targets in attempt to fix Windows builds, augment JSTypeRange with fromBits() / rawBits() to clean up casts, pass correct OpInfo for IsCellWithType in op_iterator_open, and speculate cell for 'prototype' in 32-bit compileInstanceOf().
Comment 13 Alexey Shvayka 2021-07-22 18:30:38 PDT
Created attachment 434051 [details]
WIP

Bring back Function.prototype[Symbol.hasInstance] being a built-in function, reland r161564, add microbenchmarks for Function.prototype[Symbol.hasInstance] and Object.prototype.isPrototypeOf, fix 'instanceof' not to throw on API objects that lack 'hasInstance', fix JSValueIsInstanceOfConstructor not to invoke Symbol.hasInstance on non-callable targets, merge JSObject::hasInstance() overrides for clarity, and improve testapi coverage.
Comment 14 Alexey Shvayka 2021-08-06 19:57:08 PDT
Created attachment 435108 [details]
Patch

Add tests, move js* methods from Operations.h to CommonSlowPaths namespace, refine speculated type for InternalFunction to drop SpecObjectOther, adding DFG use kind / fixup rules and introducing SpecObjectMaybeCallable to preserve correct DFG folding.