-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
SWC has a segfault condition #8840
Comments
While reproducing vercel/next.js#63924 in valgrind:
|
This is the latest valgrind output I get. Output
It strangely only occurs when building from our CI (using the deno-linux-x86_64 artifact https://github.com/denoland/deno/actions/runs/8640318958?pr=23187#artifacts) and not with a local build. I'm unfortuantely not good at this kind of debugging in Linux and haven't gotten far trying to reproduce it on other platforms. |
@dsherret Do you have |
Myabe related: rust-lang/rust#91979 |
Also maybe related: #8695 |
I unfortunately don't have the version it starts occurring, but it was working in Rust 1.77.2 and this diff of the Cargo.lock shows when it was last working (https://github.com/denoland/deno/pull/23187/files#diff-13ee4b2252c9e516a0547f2891aa2105c3ca71c6d7a1e682c69be97998dfc87e). It's only a few patch releases away for most of the crates luckily. Output with latest published crates as of this post (basically the same as above)
This first uninitailized value line is maybe the suspect one? (maybe the cloning of an Atom?) I wish it gave more detail.
|
[[package]]
name = "hstr"
-version = "0.2.6"
+version = "0.2.9"
[[package]]
name = "swc_atoms"
-version = "0.6.5"
+version = "0.6.6"
[[package]]
name = "swc_bundler"
-version = "0.225.9"
+version = "0.225.18"
[[package]]
name = "swc_common"
-version = "0.33.18"
+version = "0.33.22"
[[package]]
name = "swc_config"
-version = "0.1.11"
+version = "0.1.12"
[[package]]
name = "swc_ecma_ast"
-version = "0.112.4"
+version = "0.112.7"
[[package]]
name = "swc_ecma_codegen"
-version = "0.148.7"
+version = "0.148.15"
[[package]]
name = "swc_ecma_loader"
-version = "0.45.20"
+version = "0.45.25"
[[package]]
name = "swc_ecma_parser"
-version = "0.143.5"
+version = "0.143.13"
[[package]]
name = "swc_ecma_transforms_base"
-version = "0.137.10"
+version = "0.137.17"
[[package]]
name = "swc_ecma_transforms_classes"
-version = "0.126.10"
+version = "0.126.16"
[[package]]
name = "swc_ecma_transforms_optimization"
-version = "0.198.10"
+version = "0.198.19"
[[package]]
name = "swc_ecma_transforms_proposal"
-version = "0.171.10"
+version = "0.171.20"
[[package]]
name = "swc_ecma_transforms_react"
-version = "0.183.10"
+version = "0.183.18"
[[package]]
name = "swc_ecma_transforms_typescript"
-version = "0.188.10"
+version = "0.188.18"
[[package]]
name = "swc_ecma_utils"
-version = "0.127.7"
+version = "0.127.17"
[[package]]
name = "swc_ecma_visit"
-version = "0.98.4"
+version = "0.98.7"
[[package]]
name = "swc_fast_graph"
-version = "0.21.18"
+version = "0.21.20"
[[package]]
name = "swc_graph_analyzer"
-version = "0.22.20"
+version = "0.22.22"
[[package]]
name = "swc_visit"
-version = "0.5.9"
+version = "0.5.13"
[[package]]
name = "swc_visit_macros"
-version = "0.5.10"
+version = "0.5.11"
] |
Not sure if this is related, but we're also seeing a segfault after upgrading swc in Parcel. But only on Linux and Windows, not macOS (might be due to different stack sizes though). It happens when processing a file with a very deep AST due to string concatenation: https://unpkg.com/browse/highlight.js@11.9.0/lib/languages/isbl.js Upgrade PR: parcel-bundler/parcel#9574 |
I think it would worth to try reverting some dependency version upgrades. (of swc crates) I'll create a PR later today |
I reduced the range of commits, and I guess the cause is visitor changes in
These PRs had a noticeable impact on the compile time of turbopack, because these PRs change the way |
We're currently working on debugging this via ASAN and Valgrind in Deno as well, but it's been difficult to repro it outside of a full Deno binary. |
I found a bug in |
@mmastrac I've had some concerns about potential bugs in hstr merging, but it doesn't appear that Potentially the |
Had to revert back swc due to swc-project/swc#8840 Fixes: - denoland/deno_lint#1262 - denoland/deno_doc#538 - denoland/deno_doc#537 - denoland/deno_graph#430 - denoland/deno_graph#425 - denoland/deno_graph#432
Had to revert back swc due to swc-project/swc#8840 Fixes: - denoland/deno_lint#1262 - denoland/deno_doc#538 - denoland/deno_doc#537 - denoland/deno_graph#430 - denoland/deno_graph#425 - denoland/deno_graph#432
|
Can you all try #8849? You can depend on using git specifier. [patch.crates-io]
swc_core = { git = "https://github.com/kdy1/swc.git", branch = "segfault" } |
I was about to answer "yes, that does fix it". So just this version bump was enough for Parcel (together the the currently published crates): diff --git a/Cargo.lock b/Cargo.lock
index 6795a8883..15859184b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -681,15 +681,16 @@ dependencies = [
[[package]]
name = "hstr"
-version = "0.2.6"
+version = "0.2.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "de90d3db62411eb62eddabe402d706ac4970f7ac8d088c05f11069cad9be9857"
+checksum = "96274be293b8877e61974a607105d09c84caebe9620b47774aa8a6b942042dd4"
dependencies = [
+ "hashbrown 0.14.3",
"new_debug_unreachable",
"once_cell",
"phf",
"rustc-hash",
- "smallvec",
+ "triomphe",
]
[[package]]
@@ -1940,9 +1941,9 @@ checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623"
[[package]]
name = "swc_atoms"
-version = "0.6.5"
+version = "0.6.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7d538eaaa6f085161d088a04cf0a3a5a52c5a7f2b3bd9b83f73f058b0ed357c0"
+checksum = "04d9d1941a7d24fc503efa29c53f88dd61e6a15cc371947a75cca3b48d564b5b"
dependencies = [
"hstr",
"once_cell",
@@ -2740,6 +2741,16 @@ dependencies = [
"once_cell",
]
+[[package]]
+name = "triomphe"
+version = "0.1.11"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "859eb650cfee7434994602c3a68b25d77ad9e68c8a6cd491616ef86661382eb3"
+dependencies = [
+ "serde",
+ "stable_deref_trait",
+]
+
[[package]]
name = "typed-arena"
version = "2.0.2" |
Actually, that only fixed it on Linux. Windows still fails with exit code 3221225725 (so stack overflow) even with #8849. |
We just bumped LLVM from 16 to 17 on our CI and it appears to have fixed the crash we were seeing in Deno. It's possible this was a miscompilation or LTO bug. I'm curious what LLVM toolchains everyone else is using. I was never able to pin down a specific bit of UB that caused this, and our Miri builds have been clean. |
@mmastrac I've been reproducing the issue reported in vercel/next.js#63924 with LLVM 18 on Debian 12 arm64 (with rust installed via rustup):
I've also reproduced the issue on x86_64 Linux, as well as ARM64 Windows. The original user report was from a person using (presumably x86_64) Windows. @kdy1 #8849 does not appear to help. FWIW, here's the full unabbreviated output from valgrind: https://gist.github.com/bgw/b6f4a4439593efbcabbdba7f0a9f5362 I've tried every combination LLVM sanitizer I can think to try (memory, address, and thread sanitizer), and none of them catch any issues. I was able to reproduce the issue inside |
Such is our life 😢 |
@bgw Are you able to isolate the problem from the rest of the code in your case? We have not been able to identify a specific failing case outside of the entire system. Given our lack of Miri errors in the experiments we have run, and general difficulty in reproducing it, I'm starting to lean towards a Rust or LLVM mis-compilation (potentially during LTO) of valid code. |
I think for Parcel, the current versions work "correctly" now. So effectively #1627, but this time it fails in release mode as well and in a real world example (bundling the |
@mmastrac next.js uses It may worth to reproduce the next.js segfault using |
I'll fix #1627 to avoid excessive stack usage |
I think I managed to narrow the crash in vercel/next.js#63924 down to a dependency on node-mysql2. That does have a massive binary expression, which does line up with what I was seeing in GDB/LLDB (I had just convinced myself that valgrind was more trustworthy), and with what @kdy1 is hypothesizing in #1627: |
Hmmm... we were able to reproduce it with just this code: https://deno.land/x/which@0.3.0/mod.ts?source= |
It turns out that parser is fine with a very large input. |
Yeah, we haven't had any issues parsing deeply nested code since the introduction of stacker. |
Can you provide the list of passes (impl Fold) used by Deno? I'll fix them first |
@kdy1 sure! // https://github.com/denoland/deno_ast/blob/c73eea7eb9aa1a5c2ccc8524fa36fb695fb129f1/src/transpiling/mod.rs#L308
let mut passes = chain!(
resolver(unresolved_mark, top_level_mark, true),
proposal::decorator_2022_03::decorator_2022_03(),
proposal::explicit_resource_management::explicit_resource_management(),
helpers::inject_helpers(top_level_mark),
typescript::typescript(typescript::Config {
verbatim_module_syntax: false,
import_not_used_as_values: typescript::ImportsNotUsedAsValues::Remove,
no_empty_export: true,
import_export_assign_config: typescript::TsImportExportAssignConfig::Preserve,
ts_enum_is_mutable: true,
}, top_level_mark),
fixer(Some(comments)),
hygiene(),
); |
I'm almost done with the fix, but linux CI is failing with stack overflow. Maybe I need to add more similar changes, but I don't have a linux device. I'll try WSL |
Had to revert back swc due to swc-project/swc#8840 Fixes: - denoland/deno_lint#1262 - denoland/deno_doc#538 - denoland/deno_doc#537 - denoland/deno_graph#430 - denoland/deno_graph#425 - denoland/deno_graph#432
Just giving a status update on this: we upgraded to the latest swc in the latest version of Deno and haven't run into any issues. Thanks for all the help! |
This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you. |
This tracks a segfault issue in swc.
Related:
Bisecting
swc_core@v0.89.6
=>swc_core@v0.90.22
d006482...a0d0563
swc_ecma_ast@0.112.4
worked. 7c5113b bumped the version ofswc_ecma_ast
tov0.112.4
and 4fa09eb published the crates,so the previous commits are good.4fa09eb...a0d0563Related patches
Arc
dudykr/ddbase#29Map
implementation forBox
#8819miri
#8836Vec::move_map
#8838The text was updated successfully, but these errors were encountered: