-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Segmentation fault on Node.js 22 #58369
Comments
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 Particularly
|
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 |
I narrowed it down by trimming // package.json
{
"dependencies": {
"@types/mdast": "^4.0.0",
"typescript": "^5.0.0"
}
} // tsconfig.json
{
"compilerOptions": {
// This option doesnβt seem to matter
// "noEmit": true
}
} // index.ts
import 'mdast' This repro also causes the crash when running |
I narrowed it down even further by removing parts from /**
* 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.
*/
export type AlignType = "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.
*/
export interface Association {
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;
}
export interface Reference {
}
/**
* 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}.
*/
export interface RootContentMap {
} This still causes a crash, but at this point, even removing random chunks from comments resolves the issue. |
random ideas:
|
Yep, only removing the emoji on L312 from The emoji is |
I tried removing the emoji. That did make a difference, but so did keeping the emoji and removing another random piece of comment instead. |
Which other part? |
Any piece really. I.e. the line below the emoji and the non-empty line below that. Now |
Removing the line below the emoji does not change anything for me in |
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. |
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 Interesting from what Iβm reading: looks like an introduction of Idea: could |
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. |
Since @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. |
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. |
Fair. I pushed the reproduction repo to https://github.com/remcohaszing/typescript-bug-58369. |
Not reproing for me on Windows x64 Node 22.0.0 tsc 5.5.0-dev.20240430, potato laptop |
After setting up everything again to make a nice reproduction, I see the behaviour @wooorm described: the 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 just confirmed using |
Do we have a standalone (not all of tsc) repro we can hand off to Node/V8? |
I have nothing yet. At 3358157, the segfault happens in |
Sounds like some kind of JIT bug in V8; those are no fun. The |
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). |
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. |
This issue has been marked as "External" and has seen no recent activity. It has been automatically closed for house-keeping purposes. |
Related-to: DefinitelyTyped/DefinitelyTyped#69584. Related-to: microsoft/TypeScript#58369. Related-to: nodejs/node#52797.
π Search Terms
"segmentation fault"
π Version & Regression Information
β― Playground Link
https://github.com/remarkjs/remark/tree/main/packages/remark-parse
π» Code
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 theremark-parse
workspace, yields:π Expected behavior
It generates type definitions
Additional information about the issue
It works in Node.js 20, but not Node.js 22.
If the project is built with Node.js 20, then an incremental build with Node.js 22 wonβt crash TypeScript.
Neither
tsc
nortsc --showConfig
nottsc --build --clean
causes a crash, onlytsc --build
.See remarkjs/remark#1291 (comment) for some more info where this was first discovered.
The text was updated successfully, but these errors were encountered: