-
Notifications
You must be signed in to change notification settings - Fork 13k
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
SIGSEGV compiling num-integer for asmjs-unknown-emscripten #66308
Comments
GDB backtrace of the SIGSEGV thread on nightly:
|
Thanks @cuviper. Minimal reproducible test case: pub fn foo() {
(0..0).rev().next();
} |
triage: P-high for now. (but this is a Tier-2 target, so it may get downgraded to P-medium after a few weeks. We will see.) |
cc #65251 @tlively @alexcrichton -- the versions correspond to the LLVM change for emscripten. |
This looks like an error in the LLVM verifier, so there is something wrong with the generated LLVM IR. I think we can rule out a bug in the WebAssembly LLVM backend. |
Sorry I'm not sure where this would be coming from either.. It's likely due to the upgrade of LLVM for the emscripten target for sure though |
This commit disables the Emscripten CI job until a regression in rustc for `wasm32-unknown-emscripten` target is fixed. Tracking issue: [rust-lang/rust 66308](rust-lang/rust#66308).
This commit disables the Emscripten CI job until a regression in rustc for `wasm32-unknown-emscripten` target is fixed. Tracking issue: [rust-lang/rust 66308](rust-lang/rust#66308).
Hello, |
This is on my backlog of things to look into, but I don't know when I will be able to get to it. I don't know of anyone else planning to look into it. |
It looks like this affects |
tested the minimum test case (above) on wasm32-unknown-emscripten using stable (1.40.0 2019-12-16) and nightly (1.42.0 - 2020-01-01) with the same result: |
I started looking into this and I can successfully reproduce the crash. On some runs I get additional errors |
It looks like this is a use-after-free when LLVM is built in release mode. It is caught earlier by an assertion with a debug build of LLVM, and from the backtrace there it looks like there is indeed a previously-unknown issue with the Emscripten exception lowering pass. We are investigating further now. |
Fix in LLVM incoming: https://reviews.llvm.org/D72308. Once it merges upstream, I will handle getting it merged to Rust's LLVM fork and from there to rustc. |
Pulls in a bug fix from upstream to resolve #66308. Also adds a small Rust regression test. r? @alexcrichton
Should be fixed by #68030. |
Were you able to verify that? I can still reproduce it with |
No, I just checked that the patch had landed. Reopening. |
I thought it had been fixed because the regression test I had locally for it passes. But it turns out the test runner uses |
Adds a small Rust regression test for rust-lang#66308. r? @alexcrichton
Confirmed, thanks! |
On current num-integer master (commit f0e329af1195), rustc nightly and beta both crash when targeting
asmjs-unknown-emscripten
. Stable is fine.(The inline attribute warning is irrelevant to success/failure, but I'll clean that up separately.)
cc @mashedcode rust-num/num-integer#28
The text was updated successfully, but these errors were encountered: