Possibly deoptimizing use of `arguments` in libs
I've started to read the v8-bailout-reasons, in particular the "Bad value context for arguments
value" part. @vhf
refers to this part of more detailed research, where we can find some safety rules about arguments
. So I've tried to scan Node.js libs for possible violations of these rules. Here are the suspicious fragments I've found.
- Leaking
arguments
:
(Possibly is not worth the efforts; see this comment)
domain.js
: function Domain.prototype.intercept
leaks here. (Possibly is not worth the efforts; see this comment)
domain.js
: function Domain.prototype.bind
leaks here. (Possibly is not worth the efforts; see this comment)
internal/process.js
: function setupConfig
leaks here. (It does not cause deopt; see this PR)internal/util.js
: function exports._deprecate
leaks here.
- Using
arguments[index]
with a possibility ofindex
to be out of thearguments
bounds.
This is more difficult case to check, but I've inferred possible arguments.length
variability from docs and code comments. Maybe I'm wrong for some or all cases. I will refer to a first possible violation in a function, the function can contain more.
(Fixed)
child_process.js
: function normalizeExecArgs
uses possible out of bounds index(es) from here. (Fixed)
child_process.js
: function exports.execFile
uses possible out of bounds index(es) from here. (Fixed)child_process.js
: function normalizeSpawnArguments
uses possible out of bounds index(es) from here.
(Fixed)dgram.js
: function Socket.prototype.bind
uses possible out of bounds index(es) from here.
(Fixed)events.js
: function EventEmitter.prototype.emit
uses possible out of bounds index(es) from here.
There is a sign of awareness in the code, but maybe ES6 makes it possible to resolve this difficulty in the EventEmitter.prototype.emit
itself.
(they are not called without parameters; see this PR)fs.js
: all the functions with maybeCallback
or makeCallback
calls can use possible out of bounds index(es).
(Fixed)util.js
: function inspect
uses possible out of bounds index(es) from here.
(Possibly is not worth the efforts; see this comment)
_debugger.js
: function Interface.prototype.scripts
uses possible out of bounds index(es) from here. (Possibly is not worth the efforts; see this comment)_debugger.js
: function Interface.prototype.watchers
uses possible out of bounds index(es) from here.
I am not sure about any of these cases: if they are real violations, if these violations have any real impact on the performance and so on. And unfortunately, I have not sufficient knowledge of all the codebase system to propose any changes here. So take this as a start point memo for contributors who consider it to be worth any attention and close it if this is some needless overagitating.
P.S. Part 2.