Skip to content

Maglev on x64 causes segmentation fault while running TypeScript #52797

Closed
Closed
Maglev on x64 causes segmentation fault while running TypeScript#52797
Labels
confirmed-bugIssues and PRs for confirmed bugs.linuxIssues and PRs related to the Linux platform.regressionIssues related to regressions.v22.xIssues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.v8 engineIssues and PRs related to the V8 dependency.

Description

@remcohaszing
remcohaszing
opened on May 2, 2024

Version

v22.0.0

Platform

Linux vali 6.8.0-76060800daily20240311-generic #202403110203171407766522.04~4c8e9a0 SMP PREEMPT_DYNAMIC Thu A x86_64 x86_64 x86_64 GNU/Linux

Subsystem

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?

Segmentation fault (core dumped)

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.

Activity

  1. avivkeller commented on May 2, 2024

    @avivkeller
    avivkeller
    on May 2, 2024
    Member

    Hi! Could you possibly provide some example code to reproduce this? Preferably code that has been compiled into plain JS.

  2. remcohaszing commented on May 2, 2024

    @remcohaszing
    remcohaszing
    on May 2, 2024
    ContributorAuthor

    I would absolutely love to. Unfortunately the bug is reproduced by running tsc. Neither I nor the TypeScript team have been able to make a small reproduction.

    It was caused by microsoft/TypeScript#53081 and fixed by microsoft/TypeScript#58339 (unreleased yet).

  3. avivkeller commented on May 2, 2024

    @avivkeller
    avivkeller
    on May 2, 2024
    Member

    AFAICT this doesn't seem like an issue with Node.js itself, but rather a compiler (such as tsc), but I'm no expert, and I'd love to get a second opinion.

  4. avivkeller
    added
    help wantedIssues that need assistance from volunteers or PRs that need help to proceed.
    on May 2, 2024

  5. remcohaszing commented on May 2, 2024

    @remcohaszing
    remcohaszing
    on May 2, 2024
    ContributorAuthor

    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.

  6. avivkeller commented on May 2, 2024

    @avivkeller
    avivkeller
    on May 2, 2024
    Member

    Ahh, okay, thank you.

  7. avivkeller
    added
    regressionIssues related to regressions.
    v22.xIssues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.
    on May 2, 2024

  8. mcollina commented on May 2, 2024

    @mcollina
    mcollina
    on May 2, 2024
    SponsorMember

    @targos I have a feel we rushed the V8 upgrades.

    cc @nodejs/v8 @RafaelGSS

  9. avivkeller
    added
    v8 engineIssues and PRs related to the V8 dependency.
    and removed
    help wantedIssues that need assistance from volunteers or PRs that need help to proceed.
    on May 2, 2024

  10. targos commented on May 2, 2024

    @targos
    targos
    on May 2, 2024
    Member

    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.

  11. targos commented on May 2, 2024

    @targos
    targos
    on May 2, 2024
    Member

    Seems specific to Linux or x64 as I cannot reproduce on ARM64 macOS.

  12. targos commented on May 2, 2024

    @targos
    targos
    on May 2, 2024
    Member

    We also don't know which version of V8 introduced the bug (assuming it's in V8).

  13. targos commented on May 2, 2024

    @targos
    targos
    on May 2, 2024
    Member

    So, it's specific to x64. I can reproduce with node-v22.0.0-darwin-x64 on Rosetta.

  14. 41 remaining items

  15. targos commented on Aug 8, 2024

    @targos
    targos
    on Aug 8, 2024
    Member

    A full V8 update in v22 is very unlikely (too many breaking changes in the C++ API)

  16. joyeecheung commented on Aug 14, 2024

    @joyeecheung
    joyeecheung
    on Aug 14, 2024
    Member

    Looking at the related bugs I think we should disable maglev by default for v22, considering it will get stuck with V8 12.4 for ~2 years, and it will probably be increasingly difficult to backport any maglev patches to it even if they get fixed in the upstream. We could continue enabling it on the main branch and maybe consider enabling it in v23 to test it out, but it doesn't look great to ship the crashes to LTS.

  17. mcollina commented on Aug 14, 2024

    @mcollina
    mcollina
    on Aug 14, 2024
    SponsorMember

    It seems the safest approach, considering that there isn't really much we can do.

  18. RafaelGSS
    added 2 commits that reference this issue on Sep 6, 2024

    src: create handle scope in FastInternalModuleStat

    5c9599a

    Revert "v8: enable maglev on supported architectures"

    5243e32

  19. targos commented on Sep 19, 2024

    @targos
    targos
    on Sep 19, 2024
    Member

    Maglev was disabled on v22.x with #54384

  20. targos
    closed this as completedon Sep 19, 2024
  21. targos
    added
    v22.xIssues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.
    and removed
    v22.xIssues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.
    on Sep 19, 2024

  22. jakebailey commented on Sep 19, 2024

    @jakebailey
    jakebailey
    on Sep 19, 2024
    Member

    It's still enabled on main, right? So it's still not fixed until V8 is fixed?

  23. targos commented on Sep 19, 2024

    @targos
    targos
    on Sep 19, 2024
    Member

    Do we know that this issue also affects main?

  24. joyeecheung commented on Sep 19, 2024

    @joyeecheung
    joyeecheung
    on Sep 19, 2024
    Member

    BTW on the V8 slack @deepak1556 mentioned that they were able to reproduce it in Electron with v8 12.4 and 12.6 but not 12.8 so it's likely fixed.

  25. deepak1556 commented on Sep 19, 2024

    @deepak1556
    deepak1556
    on Sep 19, 2024
    Contributor

    Yup the repro mentioned in this post can be run with ELECTRON_RUN_AS_NODE=1 <electron x64 binary> node_modules/.bin/tsc

    Simple bisect of our prebuilt binaries showed, first good version is 128.0.6571.0 and last known bad version is 127.0.6521.0. So I can confirm that it is definitely addressed in V8 12.8 but haven't bisected to which commit addressed it.

    Here is the chromium changelist url https://chromium.googlesource.com/chromium/src/+log/127.0.6521.0..128.0.6571.0?n=10000&pretty=fuller and V8 changelist https://chromium.googlesource.com/v8/v8/+log/12.7.189..12.8.185?n=10000&pretty=fuller

  26. runspired commented on May 14, 2025

    @runspired
    runspired
    on May 14, 2025

    I hit this tonight with the following node version:

    Resolved package.json as 22.3.0
Attempting to download 22.3.0...
Acquiring 22.3.0 - x64 from https://github.com/actions/node-versions/releases/download/22.3.0-95[69](https://github.com/emberjs/data/actions/runs/15011990197/job/42182302379?pr=9989#step:3:75)309553/node-22.3.0-linux-x64.tar.gz

    from this thread, it's not totally clear which minor/patch of 22 got the fix added so if someone has that detail (or that it never did) would be gtk. I resolved by bumping to node 24.

  27. bnoordhuis commented on May 15, 2025

    @bnoordhuis
    bnoordhuis
    on May 15, 2025
    Member

    @runspired

    $ git tag --contains 5243e32 | tail -1
v22.9.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    confirmed-bugIssues and PRs for confirmed bugs.linuxIssues and PRs related to the Linux platform.regressionIssues related to regressions.v22.xIssues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.v8 engineIssues and PRs related to the V8 dependency.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions