Repository navigation
Conversation
|
Preview build of 1dce549: |
There was a problem hiding this comment.
Beyond the inline findings, I also checked the lifetime of the raw inferredName pointer handed to the parser: it points at the UnlinkedFunctionExecutable's m_ecmaName (materialized first if deferred), and the executable is held by the caller for the whole reparse, so the pointer stays valid; isSymbol() also correctly keeps *default* out of the body name.
Extended reasoning...
The change is confined to USE(BUN_JSC_ADDITIONS) blocks in the parser and UnlinkedFunctionExecutable reparse path plus one new stress test, and touches no security-sensitive surface. The two confirmed inline findings (an existing stress test asserting the old empty frame name, and the new test's loop count versus the 200ms budget) are what keep this from an approve.
Findings marked 🟡 are optional suggestions and need no follow-up push.
af807fa to
35d17ed
Compare
…bodies autobuild-preview-pr-820-35d17ed5 is the preview build of oven-sh/WebKit#820. This pin is for the draft only: the merged sha replaces it.
…he inferred name, while parsing
A generator, an async generator and an async function that awaits are a
wrapper function that makes a separate body function. The body function
is the callee of the frames that run the function's code, and of the
frames in the await chain, so stack traces, the sampling profiler and the
inspector name those frames after the body's ecmaName.
The parser gives the body only the declared name of the wrapper: the
wrapper is reparsed with executable->name() as the callee name. A name
that was inferred from the binding (`const f = async () => { await g(); }`)
is in the wrapper's ecmaName only, so the frames of such a function had
no name while f.name is "f".
generateUnlinkedFunctionCodeBlock() now hands the inferred name of a
wrapper without a declared name to the parser, and parseInner() names the
body function after it. A declared name is used as before. An inferred
name is not a callee name, so it is not declared in the function's scope.
A private name (`export default async function () { }` is `*default*`) is
not used, as for a declared name.
This replaces the earlier attempt that copied the name onto the body
executable after code generation (#426).
no-tail-call-in-generator-and-async-function-bodies.js expected a resumed async arrow body to have no name. It has the inferred name now. generator-and-async-body-function-name.js ran every check testLoopCount times. A frame's name does not depend on the tier, so it runs once.
35d17ed to
1dce549
Compare
…bodies autobuild-preview-pr-820-1dce5490 is the preview build of oven-sh/WebKit#820. This pin is for the draft only: the merged sha replaces it.
Replaces #426. That PR copied the name after code generation. The review there asked for it to be done while parsing.
Problem
at async <anonymous>forconst f = async () => { await g(); }, Node printsat async f, andf.nameis"f".Error.captureStackTrace(err, f)in Bun looks for the frame offby name. It finds none, soerr.stackisError: boomalone.generateUnlinkedFunctionCodeBlock(bytecode/UnlinkedFunctionExecutable.cpp). It reparses the function withexecutable->name(), the declared name. An inferred name is only inecmaName()and never reaches the parser.Fix
generateUnlinkedFunctionCodeBlockpasses the inferred name of a function without a declared name to the parser (Parser::setInferredFunctionName).parseInnernames the body function after the declared name, or else after the inferred one. The inferred name is not declared as a callee in the function's scope.export default async function () {}is*default*), as for a declared name.JSTests/stress/generator-and-async-body-function-name.jsin default, no-jit, dfg-eager and ftl-eager runs. It fails without the change. The Bun tests are in the Bun PR that pins this build.Background
at asyncframes. Its name is what stack traces, the sampling profiler and the inspector show. It is never visible to JavaScript.ecmaNameis the name a function has forFunction.prototype.name. The parser infers it fromconst f = ...,{ f: ... }, a class field or an assignment.Downsides
nameredefined withObject.defineProperty.Error.captureStackTrace(err, fn)in Bun can now cut at the frame of another async function with the inferred name offn. It already does that for declared names.