1607608 - Support calling builtin natives when doing eager evaluations

Status: RESOLVED FIXED

(DevTools :: Console, enhancement)

Description

[Brian Hackett [Laid off!]]

Right now native functions (C++ natives and self-hosted JS functions) can only be called in eager evaluations when they are invoked as getters. Many functions that are called normally are non-effectful (e.g. String.prototype.slice) and calls to these functions should be allowed as well. The attached patch makes this change for a variety of JS builtins, though not yet any DOM or other functions we might want to call like document.getElementById. I based this on V8's whitelist in https://github.com/v8/v8/blob/master/src/debug/debug-evaluate.cc, which is also linked from bug 1561424 comment 0. I tried to tighten things up using some helper functions when there are lots of non-effectful methods, like everything on Math and all the Date.prototype.getSomething() methods. There are a few lingering issues, though:

• Methods that execute RegExps are allowed, while V8 requires runtime checks for these (I don't know what the nature of the checks is). Executing a RegExp can have side effects on its lastIndex property if the regexp is global or sticky.

• There is a subtle unsoundness in that native functions directly called by self hosted functions will not fire the onNativeCall hook (see bug 1607596). So eagerly evaluating something like "[1,2,3].map(Array.prototype.push)" will allow the push calls to execute.

• Natives identified by symbols aren't handled, so things like "[...someIterable]" can't be eagerly evaluated.

I'm not sure what to do about these cases. I could handle update the patch to handle them, deal with them in followup bugs, or just not worry about them for the time being.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: Bug 1607608 - Support calling builtin natives when doing eager evaluations, r=nchevobbe.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

I'm not sure what to do about these cases. I could handle update the patch to handle them, deal with them in followup bugs, or just not worry about them for the time being.

We should probably file follow-up bugs for each of those, and then we'll evaluate them individually.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9547b77030c9
Support calling builtin natives when doing eager evaluations, r=nchevobbe.

Comment 4

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/9547b77030c9

Comment 5

[Oriol Brufau [:Oriol]]

Why allow things like Object.getOwnPropertyDescriptor but not Reflect.getOwnPropertyDescriptor?

Comment 6

[Brian Hackett [Laid off!]]

(In reply to Oriol Brufau [:Oriol] from comment #5)

Why allow things like Object.getOwnPropertyDescriptor but not Reflect.getOwnPropertyDescriptor?

It would be fine to allow Reflect.getOwnPropertyDescriptor, this bug is just providing an initial set of natives from the most common JS builtins for handling.