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
Maglev on x64 causes segmentation fault while running TypeScript #52797
Comments
Hi! Could you possibly provide some example code to reproduce this? Preferably code that has been compiled into plain JS. |
I would absolutely love to. Unfortunately the bug is reproduced by running It was caused by microsoft/TypeScript#53081 and fixed by microsoft/TypeScript#58339 (unreleased yet). |
AFAICT this doesn't seem like an issue with Node.js itself, but rather a compiler (such as |
Sorry, I now see I forgot to provide a critical piece of information. This is a regression in Node.js 22.0.0. It wasn’t a problem before. |
Ahh, okay, thank you. |
@targos I have a feel we rushed the V8 upgrades. cc @nodejs/v8 @RafaelGSS |
It's probably related to V8, but I'm not sure waiting would have changed anything? We released v22.0.0 with the version of V8 that's in current Chrome. |
Seems specific to Linux or x64 as I cannot reproduce on ARM64 macOS. |
We also don't know which version of V8 introduced the bug (assuming it's in V8). |
So, it's specific to x64. I can reproduce with |
I'm going to compile a debug build on one of the Hetzner machines to get a meaningful stack trace. |
I’m on macOS and repro it consistently btw |
@woorm ARM or Intel? |
The code that started/stopped crashing in TS had do to with indexing into strings. One TS maintainer potentially saw the crash appear/disappear when adding a |
I’m on an 2.6 GHz 6-Core Intel Core i7, Sonoma 14.4.1 (23E224). |
Ignore the repro-exists tag, I didn't mean to add it, and it won't effect anything. |
Just to give a side by side using https://github.com/remcohaszing/typescript-bug-58369: $ grep -A8 'function scanJSDocCommentTextToken' ./node_modules/typescript/lib/tsc.js
function scanJSDocCommentTextToken(inBackticks) {
fullStartPos = tokenStart = pos;
tokenFlags = 0 /* None */;
if (pos >= end) {
return token = 1 /* EndOfFileToken */;
}
for (let ch = text.charCodeAt(pos); pos < end && (!isLineBreak(ch) && ch !== 96 /* backtick */); ch = codePointAt(text, ++pos)) {
if (!inBackticks) {
if (ch === 123 /* openBrace */) {
$ node ./node_modules/typescript/lib/tsc.js
[1] 1090384 segmentation fault node ./node_modules/typescript/lib/tsc.js Now, add $ grep -A8 'function scanJSDocCommentTextToken' ./node_modules/typescript/lib/tsc.js
function scanJSDocCommentTextToken(inBackticks) {
fullStartPos = tokenStart = pos;
tokenFlags = 0 /* None */;
if (pos >= end) {
return token = 1 /* EndOfFileToken */;
}
for (let ch = text.charCodeAt(pos); pos < end && (!isLineBreak(ch) && ch !== 96 /* backtick */); ch = codePointAt(text, ++pos)) {
debugger; // ADDED
if (!inBackticks) {
$ node ./node_modules/typescript/lib/tsc.js I have not been able to extract out a test which just calls the parser via the public API, nor by extracting this code and giving it the same inputs. |
Weird, thanks for the information! |
Perfect 🙃 |
Just to note it, you can also add |
/cc @victorgomes |
I'm rebuilding with ASan and GCC stack protection flags to see if it helps to pinpoint the issue |
GCC failed to build so I switched to clang.
I don't know what else to do at this point. Happy to run more commands if you have any idea. |
|
I also tried the repro with:
They all segfault so I don't think it's a V8 regression, but really a Maglev inlining bug. |
I submitted a V8 bug report: https://issues.chromium.org/issues/338535750 |
It looks very similar to https://issues.chromium.org/issues/42204637 |
Well, there is a v8 option called // tsc.js
function isLineBreak(ch) {
return ch === 10 /* lineFeed */ || ch === 13 /* carriageReturn */ ;
}
|
@targos Could you try to run the test with maglev and with pointer compression enabled? |
FWIW, I faced this issue on macOS 13 (Intel Mac) as well in my project. My Node.js version is v22.1.0 (installed via Homebrew). Here is a reproduction (sorry I don't have enough time to minimize it):
|
@remcohaszing your issue also involved emojis, right? |
The original issue is TypeScript processing |
This is a workaround for nodejs/node#52797.
Version
v22.0.0
Platform
Linux vali 6.8.0-76060800daily20240311-generic #202403110203
171407766522.04~4c8e9a0 SMP PREEMPT_DYNAMIC Thu A x86_64 x86_64 x86_64 GNU/LinuxSubsystem
No response
What steps will reproduce the bug?
On Linux using Node.js 22:
git clone git@github.com:remcohaszing/typescript-bug-58369.git cd typescript-bug-58369 npm ci tsc
See also this failed GitHub action: https://github.com/remcohaszing/typescript-bug-58369/actions/runs/8899456400/job/24438867767
How often does it reproduce? Is there a required condition?
For this reproduction it’s reproduced consistently on Linux on both my machine and GitHub actions.
While troubleshooting by trimming down the content of
node_modules/@types/mdast/index.d.ts
, I got into a state where it seemed to happen randomly. The major factor is the👉
emoji in a comment.The error did not occur on macOS in the GitHub action, but it did happen consistently for @wooorm on their macbook.
The problem was not reproducible on Windows.
What is the expected behavior? Why is that the expected behavior?
No segmentation fault
What do you see instead?
Additional information
This was originally reported to TypeScript: microsoft/TypeScript#58369. This issue contains more information.
This has coincidentally already been fixed for the upcoming TypeScript 5.5. Still, a segfault should not occur.
We were unable to make a smaller reproduction.
The text was updated successfully, but these errors were encountered: