Skip to content

Commit

Permalink
Remove instanceof fast path
Browse files Browse the repository at this point in the history
Summary:
Original Author: [email protected]
Original Git: 2eaa584
Original Reviewed By: avp
Original Revision: D43642297

`instanceof`'s fast path is incorrect, it elides the lookup of
`Symbol.hasInstance` if the RHS is a function, based on the reasoning
that `Function.prototype[Symbol.hasInstance]` cannot be modified.
However this is incorrect since you could still define
`Symbol.hasInstance` on the function itself, or change the prototype of
the function.

NOTE: Porting required changes to nodelib for node-hermes because we
were relying on the incorrect behavior. Switched to use new.target
like the original file does.

Reviewed By: neildhar

Differential Revision: D60787313

fbshipit-source-id: 6a7be1cc2cc9a268ff424bb11c84e19327aae766
  • Loading branch information
avp authored and facebook-github-bot committed Aug 6, 2024
1 parent 2c36b0a commit f60d2b8
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 8 deletions.
7 changes: 0 additions & 7 deletions lib/VM/Operations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1910,13 +1910,6 @@ CallResult<bool> instanceOfOperator_RJS(
"right operand of 'instanceof' is not an object");
}

// Fast path: Function.prototype[Symbol.hasInstance] is non-configurable
// and non-writable (ES6.0 19.2.3.6), so we directly run its behavior here.
// Simply call through to ordinaryHasInstance.
if (vmisa<JSFunction>(*constructor)) {
return ordinaryHasInstance(runtime, constructor, object);
}

// 2. Let instOfHandler be GetMethod(C,@@hasInstance).
CallResult<PseudoHandle<>> instOfHandlerRes = JSObject::getNamed_RJS(
Handle<JSObject>::vmcast(constructor),
Expand Down
4 changes: 4 additions & 0 deletions test/hermes/instanceof.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,7 @@ try {
}
//CHECK-NEXT: caught RangeError Maximum prototype chain length exceeded

function A(){}
Object.defineProperty(A, Symbol.hasInstance, {value: function(){return true;}})
print(undefined instanceof A);
//CHECK-NEXT: true
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ function Console(options
// We have to test new.target here to see if this function is called
// with new, because we need to define a custom instanceof to accommodate
// the global console.
if (!(this instanceof Console ? this.constructor : void 0)) {
if (new.target === undefined) {
return ReflectConstruct(Console, arguments);
}

Expand Down

0 comments on commit f60d2b8

Please sign in to comment.