Repository navigation
Lower the untyped one-string process.stdout.write(text) call directly - #2964
Merged
Merged
Conversation
At an untyped call site, process.stdout.write(text) and process.stderr.write(text) with a single string argument went through dynamic method dispatch: box the argument, read the write slot, and invoke through the handle. The lowering now emits process.stdioWriteStr(fd, text), and the new runtime entry point writes the string when the slot is still the built-in method and otherwise makes the same call the site made before, with the string as its only argument and the stream as `this`. The write still costs one call and one write per chunk, and a replaced write keeps its boolean result. The write-slot feature rule lists the new entry point, so a program that does not use this call form links the objects it linked before.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Comment on lines
+1463
to
+1464
| ScrDyn *result = scr_dyn_call(stream->write, &chunk, 1, | ||
| stream->fd == 2 ? "process.stderr.write" : "process.stdout.write"); |
Contributor
There was a problem hiding this comment.
When a replacement write restores or replaces itself during the call, the unretained function can be freed along with its captured bindings, causing a use-after-free when the callback continues.
The untyped one-string write calls a replacement process.stdout.write straight from the stdio slot, and borrowed that slot value for the call. A replacement that assigns process.stdout.write during its own call — restoring the built-in method, for one — drops the slot's reference to the function that is running, and its captured bindings go with it while the body still reads them. Hold a reference for the duration of the call and drop it after the return, as the other dispatch paths do. tests/corpus/process-stdout-write-form.js now covers that shape: the replacement restores the built-in method before it accumulates its argument and reports the accumulated length. Without the extra reference the sanitized native run stops at the write.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
process.stdout.write(text)orprocess.stderr.write(text)now reaches one runtime call instead of a boxed receiver, a method read and a boxed chunk; a replacedwritestill receives the call with the same arguments,thisand result./dev/null: 3.13 s to 1.38 s (ratio 0.4413, 95% CI [0.4337, 0.4476], A/A [0.9705, 1.0018]), 313 to 138 ns per call, and the benchmark binary shrinks from 565,048 to 378,640 bytes.