Closed Bug 1530413 Opened 9 months ago Closed Last month

Inconsistent error reporting for non-function object RHS to instanceof

Categories

(Core :: JavaScript Engine, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox67 --- wontfix
firefox71 --- fixed

People

(Reporter: bzbarsky, Assigned: annyG)

Details

Attachments

(1 file)

({} instanceof {})
TypeError: ({}) is not a function
({} instanceof document)
TypeError: invalid 'instanceof' operand document
({} instanceof new Proxy(document, {}))
TypeError: (new Proxy(...)) is not a function

What's going on here is that there are two ways we can report an error here. We can end up with JSMSG_BAD_INSTANCEOF_RHS. That can happen due to reaching BaseProxyHandler::hasInstance (the "document" case) or either the interpreter or JIT having a non-object as RHS.

Or we can land in JS::InstanceofOperator, determine that !obj->isCallable() and js::ReportIsNotFunction.

Arguably the BaseProxyHandler behavior is just broken: it's ignoring whether the proxy has an @@hasInstance! Evaluating this in the web console:

  document[Symbol.hasInstance] = function() { return true; }; ({} instanceof document)

throws whereas in other browsers it evaluates to true. I can fix this in the DOM proxy code, but it really feels like BaseProxyHandler should handle this better.

Priority: -- → P2

I guess here's the fundamental question: why does BaseProxyHandler::hasInstance not just call JS::InstanceofOperator? That seems like a much better default fallback for proxies that choose to not override hasInstance than throwing...

Assignee: nobody → agakhokidze
Status: NEW → ASSIGNED

Additionally, this fixes test dom/tests/mochitest/bugs/test_bug1530292.html
which fails if fission is enabled.

Pushed by agakhokidze@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c4f1cd4e616
Inconsistent error reporting for non-function object RHS to instanceof, r=bzbarsky,tcampbell
Status: ASSIGNED → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.