You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The code is not actually relevant. It still happens if you remove the content of packages/remark-parse/lib/index.js. The existence of the file is important.
π Actual behavior
Using Node.js 22, running tsc --build from the remark-parse workspace, yields:
I narrowed it down even further by removing parts from @types/mdast. I trimmed it down to this:
/** * How phrasing content is aligned * ({@link https://drafts.csswg.org/css-text/ | [CSSTEXT]}). * * * `'left'`: See the * {@link https://drafts.csswg.org/css-text/#valdef-text-align-left | left} * value of the `text-align` CSS property * * `'right'`: See the * {@link https://drafts.csswg.org/css-text/#valdef-text-align-right | right} * value of the `text-align` CSS property * * `'center'`: See the * {@link https://drafts.csswg.org/css-text/#valdef-text-align-center | center} * value of the `text-align` CSS property * * `null`: phrasing content is aligned as defined by the host environment * * Used in GFM tables. */exporttypeAlignType="center"|"left"|"right"|null;/** * Internal relation from one node to another. * * Whether the value of `identifier` is expected to be a unique identifier or * not depends on the type of node including the Association. * An example of this is that they should be unique on {@link Definition}, * whereas multiple {@link LinkReference}s can be non-unique to be associated * with one definition. */exportinterfaceAssociation{identifier: string;/** * Relation of association, in parsed form. * * `label` is a `string` value: it works just like `title` on {@link Link} * or a `lang` on {@link Code}: character escapes and character references * are parsed. * * It can match another node. */label?: string|null|undefined;}exportinterfaceReference{}/** * Registry of all mdast nodes that can occur as children of {@link Root}. * * > π **Note**: {@link Root} does not need to be an entire document. * > it can also be a fragment. * * This interface can be augmented to register custom node types: * * * For a union of all {@link Root} children, see {@link RootContent}. */exportinterfaceRootContentMap{}
This still causes a crash, but at this point, even removing random chunks from comments resolves the issue.
Any piece really. I.e. the line below the emoji and the non-empty line below that.
Now tsc is crashing randomly for me. I seem to have reached some treshold around this amount of content thatβs allowed in the mdast types. The emoji doesnβt cause the problem, but it counts as a big factor towards reaching this treshold.
Removing the line below the emoji does not change anything for me in mdast-util-to-string. Nor does the non empty line. Nor both together. For me the emoji is consistent
The TypeScript compiler is JavaScript code, which should never be able to cause a segfault (as that would likely be a security issue in e.g. browsers). This sound more like a Node and/or V8 bug to me.
Interesting from what Iβm reading: looks like an introduction of scanJSDocCommentTextToken, and new code relating to positional info?
The rest of the diff looks like the renumbering of the enums. And renaming functions from getStartPos -> getTokenFullStart; setTextPos -> resetTokenState.
some more interesting code at the end, around removeTrailingWhitespace and pushComment in I believe the tokenizer.
Idea: could Debug.assert cause a crash on Node 22?
My point stands. tsc is pure JS code; if any version of the compiler can cause a segmentation fault that crashes the Node.js process, that's a potential security issue and should be brought up to the Node.js team.
Using Node 22 and the linked repo, I can't reproduce this segfault.
If you have this narrowed down to two specific versions, https://www.npmjs.com/package/every-ts would let you go further without knowing how to build TS itself (not that checking out and running TS is that hard). That diff is just too large to guess at, but an actual bisect would reveal things.
Thanks for narrowing that down. Realistically, nothing in that PR should have been able to segfault node (or anything in JS at all, period), so if this is new in Node 22, it's definitely a Node bug if anything.
I wasn't sure how to test with your repro, though; if you can make a branch or new repo with that content, that'd be helpful for experimentation and reporting upstream.
After setting up everything again to make a nice reproduction, I see the behaviour Titus (@wooorm) described: the π emoji causes the crash, itβs not random anymore.
I have also setup a test matrix in GitHub actions which shows that this errors occurs in Linux. It canβt be reproduced on Windows, because npm shipped with Node.js 22 on Windows appears to be broken.
This is actually "fixed" on TS main by #58339; that PR prevents out of bounds accesses in more places, among other tweaks, so it sounds like Node 22 has a string handling bug.
I have nothing yet. At 3358157, the segfault happens in scanJSDocCommentTextToken, but if you add console log inside of the for loop, it stops segfaulting.
Even extracting the code out, I can't reproduce this with the same inputs to that function. At this point, I don't think there's any mimization that can be done and the only next steps are to bisect Node to see when it broke between v20 and v22 (and then when it's inevitably just a "pull in v8" change, bisect that too).
Activity
wooorm commentedon Apr 30, 2024
Also reproβd at https://github.com/syntax-tree/mdast-util-to-string/actions/runs/8894134799, https://github.com/syntax-tree/unist-util-visit-parents/actions/runs/8893972759, and https://github.com/syntax-tree/unist-util-visit/actions/runs/8894107813/job/24421655968, which are simpler.
The segment fault goes away by removing
declaration: truein tsconfig.Particularly
mdast-util-to-stringis a simple repo that I recommend looking at.What these repos have in common is that they use(edit:.d.tsfiles.mdast-util-to-stringdoesnβt)wooorm commentedon Apr 30, 2024
it doesnβt happen in https://github.com/syntax-tree/nlcst-to-string, which is similar, but revolved around a different ecosystem.
I am now guessing that there something in the types in
node_modulesof these packages. Likely around micromark/mdast (aka markdown).remcohaszing commentedon Apr 30, 2024
I narrowed it down by trimming
mdast-util-to-stringto a repo with the following files:This repro also causes the crash when running
tsc, not justtsc --build.remcohaszing commentedon Apr 30, 2024
I narrowed it down even further by removing parts from
@types/mdast. I trimmed it down to this:This still causes a crash, but at this point, even removing random chunks from comments resolves the issue.
wooorm commentedon Apr 30, 2024
random ideas:
or the[and]in the link text ([CSSTEXT])?wooorm commentedon Apr 30, 2024
Yep, only removing the emoji on L312 from
node_modules/@types/mdast/index.d.tssolves this for me inmdast-util-to-string.The emoji is
U+1f449, as in consisting of the surrogates0xd83dand0xdc49remcohaszing commentedon Apr 30, 2024
I tried removing the emoji. That did make a difference, but so did keeping the emoji and removing another random piece of comment instead.
wooorm commentedon Apr 30, 2024
Which other part?
remcohaszing commentedon Apr 30, 2024
Any piece really. I.e. the line below the emoji and the non-empty line below that.
Now
tscis crashing randomly for me. I seem to have reached some treshold around this amount of content thatβs allowed in the mdast types. The emoji doesnβt cause the problem, but it counts as a big factor towards reaching this treshold.wooorm commentedon Apr 30, 2024
Removing the line below the emoji does not change anything for me in
mdast-util-to-string. Nor does the non empty line. Nor both together. For me the emoji is consistentfatcerberus commentedon Apr 30, 2024
The TypeScript compiler is JavaScript code, which should never be able to cause a segfault (as that would likely be a security issue in e.g. browsers). This sound more like a Node and/or V8 bug to me.
wooorm commentedon Apr 30, 2024
The crash does not happen in TS 5.1.0-dev.20230307. But does occur with TS 5.1.0-dev.20230308.
Hereβs the diff, which is too big to put in markdown. And Iβm not allowed to use
.diffas an extension here.diff.txt
Interesting from what Iβm reading: looks like an introduction of
scanJSDocCommentTextToken, and new code relating to positional info?The rest of the diff looks like the renumbering of the enums. And renaming functions from
getStartPos->getTokenFullStart;setTextPos->resetTokenState.some more interesting code at the end, around
removeTrailingWhitespaceandpushCommentin I believe the tokenizer.Idea: could
Debug.assertcause a crash on Node 22?fatcerberus commentedon Apr 30, 2024
My point stands. tsc is pure JS code; if any version of the compiler can cause a segmentation fault that crashes the Node.js process, that's a potential security issue and should be brought up to the Node.js team.
jakebailey commentedon Apr 30, 2024
Using Node 22 and the linked repo, I can't reproduce this segfault.
If you have this narrowed down to two specific versions, https://www.npmjs.com/package/every-ts would let you go further without knowing how to build TS itself (not that checking out and running TS is that hard). That diff is just too large to guess at, but an actual bisect would reveal things.
remcohaszing commentedon Apr 30, 2024
Since Titus (@wooorm) and I have slightly different results, I suspect the hardware plays a role and you have a very beefy development machine.
I use Linux by the way. That may be relevant.
Based on a new project with only the files as described in #58369 (comment):
So if I understand correctly the culprit is #53081.
jakebailey commentedon Apr 30, 2024
Thanks for narrowing that down. Realistically, nothing in that PR should have been able to segfault node (or anything in JS at all, period), so if this is new in Node 22, it's definitely a Node bug if anything.
I wasn't sure how to test with your repro, though; if you can make a branch or new repo with that content, that'd be helpful for experimentation and reporting upstream.
remcohaszing commentedon Apr 30, 2024
Fair. I pushed the reproduction repo to https://github.com/remcohaszing/typescript-bug-58369.
RyanCavanaugh commentedon Apr 30, 2024
Not reproing for me on Windows x64 Node 22.0.0 tsc 5.5.0-dev.20240430, potato laptop
remcohaszing commentedon Apr 30, 2024
After setting up everything again to make a nice reproduction, I see the behaviour Titus (@wooorm) described: the
πemoji causes the crash, itβs not random anymore.I have also setup a test matrix in GitHub actions which shows that this errors occurs in Linux. It canβt be reproduced on Windows, because npm shipped with Node.js 22 on Windows appears to be broken.
jakebailey commentedon Apr 30, 2024
This is actually "fixed" on TS main by #58339; that PR prevents out of bounds accesses in more places, among other tweaks, so it sounds like Node 22 has a string handling bug.
remcohaszing commentedon Apr 30, 2024
I just confirmed using
typescript@nextfixed it for me.RyanCavanaugh commentedon Apr 30, 2024
Do we have a standalone (not all of tsc) repro we can hand off to Node/V8?
jakebailey commentedon Apr 30, 2024
I have nothing yet. At 3358157, the segfault happens in
scanJSDocCommentTextToken, but if you add console log inside of the for loop, it stops segfaulting.fatcerberus commentedon Apr 30, 2024
Sounds like some kind of JIT bug in V8; those are no fun. The
console.logprobably causes a deopt that sidesteps it.jakebailey commentedon Apr 30, 2024
Even extracting the code out, I can't reproduce this with the same inputs to that function. At this point, I don't think there's any mimization that can be done and the only next steps are to bisect Node to see when it broke between v20 and v22 (and then when it's inevitably just a "pull in v8" change, bisect that too).
jakebailey commentedon Apr 30, 2024
In any case, I don't think this is a problem that TS can handle; if the bug is "fixed" by adding an innocuous console log, that's a runtime problem.
typescript-bot commentedon May 3, 2024
This issue has been marked as "External" and has seen no recent activity. It has been automatically closed for house-keeping purposes.
Remove emoji from JSDoc to prevent segfault