Skip to content
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

"LLVM ERROR: Access past stack top!" when compiling without sse2 #65844

Open
nivkner opened this issue Oct 26, 2019 · 32 comments
Open

"LLVM ERROR: Access past stack top!" when compiling without sse2 #65844

nivkner opened this issue Oct 26, 2019 · 32 comments
Assignees
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-bug Category: This is a bug. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. ICEBreaker-LLVM Bugs identified for the LLVM ICE-breaker group O-x86_64 Target: x86-64 processors (like x86_64-*) P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nivkner
Copy link
Contributor

nivkner commented Oct 26, 2019

the following code produces the compilation error LLVM ERROR: Access past stack top! when compiling with E:RUSTFLAGS="-Ctarget-feature=-sse2" cargo +nightly run

fn main() {
    let (a, b) = get_pair();
}

fn get_pair() -> (f64, f64) {
    (0.0, 0.0)
}

only happens when the return values of get_pair are named.

also when compiling an example to a library crate like this: E:RUSTFLAGS="-Ctarget-feature=-sse2" cargo +nightly run --example bug

i get the following error instead:

process didn't exit successfully: `rustc --edition=2018 --crate-name bug examples/bug.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type bin --emit=dep-info,link -C debuginfo=2 -C metadata=6421b7fc9708c373 -C extra-filename=-6421b7fc9708c373 --out-dir /home/user/Documents/bug_lib/target/debug/examples -C incremental=/home/user/Documents/bug_lib/target/debug/incremental -L dependency=/home/user/Documents/bug_lib/target/debug/deps --extern bug_lib=/home/user/Documents/bug_lib/target/debug/deps/libbug_lib-f0c2fba934c178d2.rlib -Ctarget-feature=-sse2` (signal: 11, SIGSEGV: invalid memory reference)

rustc version:

rustc 1.40.0-nightly (246be7e1a 2019-10-25)
binary: rustc
commit-hash: 246be7e1a557b8ac8287c6842379a0db67770be6
commit-date: 2019-10-25
host: x86_64-unknown-linux-gnu
release: 1.40.0-nightly
LLVM version: 9.0
@jonas-schievink jonas-schievink added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-nominated labels Oct 26, 2019
@jonas-schievink
Copy link
Contributor

Does this also happen on older Rust versions?

@mati865
Copy link
Contributor

mati865 commented Oct 26, 2019

AFAIK it affects all Rust versions since 1.18.0.

@jonas-schievink jonas-schievink added the O-x86_64 Target: x86-64 processors (like x86_64-*) label Oct 26, 2019
@pnkfelix
Copy link
Member

@rustbot ping ICEbreakers-LLVM

@rustbot
Copy link
Collaborator

rustbot commented Oct 31, 2019

Error: This team (ICEbreakers-LLVM) cannot be pinged via this command;it may need to be added to triagebot.toml on the master branch.

Please let @rust-lang/release know if you're having trouble with this bot.

@pnkfelix
Copy link
Member

@rustbot ping icebreakers-llvm

@rustbot
Copy link
Collaborator

rustbot commented Oct 31, 2019

Hey LLVM ICE-breakers! This bug has been identified as a good
"LLVM ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @hdhoang @heyrutvik @jryans @mmilenko @nagisa @nikic @rkruppe @spastorino @vertexclique

@rustbot rustbot added the ICEBreaker-LLVM Bugs identified for the LLVM ICE-breaker group label Oct 31, 2019
@pnkfelix
Copy link
Member

triage: P-high, removing I-nominated

@pnkfelix pnkfelix added P-high High priority and removed I-nominated labels Oct 31, 2019
@hanna-kruppe
Copy link
Contributor

FWIW I can reproduce with

rustc 1.38.0 (625451e37 2019-09-23)
binary: rustc
commit-hash: 625451e376bb2e5283fc4741caa0a3e8a2ca4d54
commit-date: 2019-09-23
host: x86_64-pc-windows-msvc
release: 1.38.0
LLVM version: 9.0

but can't reproduce with

rustc 1.40.0-nightly (7979016af 2019-10-20)
binary: rustc
commit-hash: 7979016aff545f7b41cc517031026020b340989d
commit-date: 2019-10-20
host: x86_64-pc-windows-msvc
release: 1.40.0-nightly
LLVM version: 9.0

@mati865
Copy link
Contributor

mati865 commented Oct 31, 2019

Reproduces on Compiler Explorer with latest nightly but only with optimisations disabled.

@pnkfelix
Copy link
Member

Oh dear:

% rustc +nightly-2019-10-24 -C target-feature=-sse2 --allow warnings issue65844.rs 
% rustc +nightly-2019-10-24 -C target-feature=-sse2 --allow warnings issue65844.rs 
% rustc +nightly-2019-10-24 -C target-feature=-sse2 --allow warnings issue65844.rs 
Segmentation fault: 11
% rustc +nightly-2019-10-24 -C target-feature=-sse2 --allow warnings issue65844.rs 
% rustc +nightly-2019-10-24 -C target-feature=-sse2 --allow warnings issue65844.rs 
% rustc +nightly-2019-10-24 -C target-feature=-sse2 --allow warnings issue65844.rs 
% rustc +nightly-2019-10-24 -C target-feature=-sse2 --allow warnings issue65844.rs 
% rustc +nightly-2019-10-24 -C target-feature=-sse2 --allow warnings issue65844.rs 
% rustc +nightly-2019-10-24 -C target-feature=-sse2 --allow warnings issue65844.rs 
% rustc +nightly-2019-10-24 -C target-feature=-sse2 --allow warnings issue65844.rs 
Segmentation fault: 11
% 

@pnkfelix
Copy link
Member

I love segfaults from rustc. Assigning self.

@pnkfelix pnkfelix self-assigned this Nov 14, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Nov 20, 2019

(seems like it might only happen at opt-level=0 ... oh, that matches what @mati865 already said )

@pnkfelix
Copy link
Member

pnkfelix commented Nov 20, 2019

Building with a compiler that has optimize=false in the [llvm] section of the config.toml file shows this error:

rustc: /home/pnkfelix/Dev/Mozilla/rust.git/src/llvm-project/llvm/lib/Target/X86/X86FloatingPoint.cpp:317: unsigned int getFPReg(const llvm::MachineOperand&): Assertion `Reg >= X86::FP0 && Reg <= X86::FP6 && "Expected FP register!"' failed.

However, if I break the compile up into two stages, so that rustc generates the .bc file and llc is responsible for compiling the .bc file, I get this:

% ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc -Ccodegen-units=1 -C target-feature=-sse2 issue-65844.rs  --emit=llvm-bc
warning: unused variable: `a`
 --> issue-65844.rs:2:10
  |
2 |     let (a, b) = get_pair();
  |          ^ help: consider prefixing with an underscore: `_a`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `b`
 --> issue-65844.rs:2:13
  |
2 |     let (a, b) = get_pair();
  |             ^ help: consider prefixing with an underscore: `_b`

% ./build/x86_64-unknown-linux-gnu/llvm/bin/llc issue-65844.bc
error: <unknown>:0:0: in function _ZN11issue_658448get_pair17h78e1eb5d988d8ff6E { double, double } (): SSE2 register return with SSE2 disabled

error: <unknown>:0:0: in function _ZN11issue_658448get_pair17h78e1eb5d988d8ff6E { double, double } (): SSE2 register return with SSE2 disabled

The rustc-generated LLVM IR in question looks like this:

; issue_65844::get_pair
; Function Attrs: nonlazybind uwtable
define internal { double, double } @_ZN11issue_658448get_pair17h78e1eb5d988d8ff6E() unnamed_addr #0 {
start:
  %0 = alloca { double, double }, align 8
  %1 = bitcast { double, double }* %0 to double*
  store double 0.000000e+00, double* %1, align 8
  %2 = getelementptr inbounds { double, double }, { double, double }* %0, i32 0, i32 1
  store double 0.000000e+00, double* %2, align 8
  %3 = getelementptr inbounds { double, double }, { double, double }* %0, i32 0, i32 0
  %4 = load double, double* %3, align 8
  %5 = getelementptr inbounds { double, double }, { double, double }* %0, i32 0, i32 1
  %6 = load double, double* %5, align 8
  %7 = insertvalue { double, double } undef, double %4, 0
  %8 = insertvalue { double, double } %7, double %6, 1
  ret { double, double } %8
}

So... is this rustc's fault for generating LLVM IR that passes around a { double, double }? or is this LLVM's fault for failing to represent the { double, double } in a manner that it knows how to handle on a target without sse2 registers?

@mati865
Copy link
Contributor

mati865 commented Nov 20, 2019

So... is this rustc's fault for generating LLVM IR that passes around a { double, double }? or is this LLVM's fault for failing to represent the { double, double } in a manner that it knows how to handle on a target without sse2 registers?

Probably @nagisa or @nikic could answer this question.

@pnkfelix
Copy link
Member

My personal suspicion is that we shift this over to LLVM's camp, and that we should try to turn the LLVM IR .ll file into something we can file as a bug against LLVM.

@hanna-kruppe
Copy link
Contributor

LLVM should not crash on valid LLVM IR (accepted by -verify) so this is definitely an LLVM bug. Assuming it can be reproduced with a standalone .ll testcase, at least.

@pnkfelix
Copy link
Member

pnkfelix commented Nov 20, 2019

I wonder if this reduction would be acceptable to the LLVM developers: https://godbolt.org/z/9RvgsQ :

attributes #0 = { "target-cpu"="x86-64" "target-features"="-sse2" }

define { double, double } @get_pair() unnamed_addr #0 {
    %1 = alloca { double, double }, align 8
    %2 = bitcast { double, double }* %1 to double*
    store double 0.000000e+00, double* %2, align 8
    %3 = getelementptr inbounds { double, double }, { double, double }* %1, i32 0, i32 1
    store double 0.000000e+00, double* %3, align 8
    %4 = getelementptr inbounds { double, double }, { double, double }* %1, i32 0, i32 0
    %5 = load double, double* %4, align 8
    %6 = getelementptr inbounds { double, double }, { double, double }* %1, i32 0, i32 1
    %7 = load double, double* %6, align 8
    %8 = insertvalue { double, double } undef, double %5, 0
    %9 = insertvalue { double, double } %8, double %7, 1
    ret { double, double } %9
}

@hanna-kruppe
Copy link
Contributor

That seems to (relatively) gracefully report an error, not fail an assertion. And not exactly a novel error, see e.g. #26449. I can't say for sure but "x86_64 without SSE2" is an extremely odd configuration to begin with, so it seems sensible to require addition of -sse,+soft-float to make it work. The only clear bug to me is that in some cases you get a segfault/assertion failure instead of a proper error.

@pnkfelix
Copy link
Member

pnkfelix commented Nov 20, 2019

That seems to (relatively) gracefully report an error, not fail an assertion.

So that means its not the right reduction then, it seems. I wasn't sure if there was something going on where llc might catch errors that end up turning into LLVM Assertion Failures in the context of rustc . (After all, that seems to be what is strongly implied by the decoupled invocation of rustc-Csave-temps+llc in my earlier comment.)

@pnkfelix
Copy link
Member

(It also still seems very bad that this scenario leads to rustc itself crashing. So I guess I'll keep looking.)

@nikic
Copy link
Contributor

nikic commented Dec 7, 2019

Possibly this is just a failure to handle Unsupported diagnostics inside our LLVM diagnostic handler?

unsafe extern "C" fn diagnostic_handler(info: &DiagnosticInfo, user: *mut c_void) {

@pnkfelix
Copy link
Member

pnkfelix commented Dec 31, 2019

Aha!

My insight today: why bother reducing to some .ll file, when you can just reduce to a C source file and invoke clang to show the bug!

Step 1. Verified that bug still reproduces with this version of the code (when I use a [llvm] optimize=false build):

pub struct S1 { _x: f64, _y: f64 }

pub fn indirect(get_pair: fn () -> S1) {
    let _s1 = get_pair();
}

Step 2. The above is readily translatable to C:

struct S { double x; double y; };

int indirect(struct S get_pair()) {
    struct S s = get_pair();
    return 0;
}

Step 3. Get your hands on clang build with assertions enabled. (This was harder than it should have been for me, in part because I insisted on doing it by figuring out some way to get the llvm-project checkout in my Rust tree to be the thing that built the clang)

Step 4: Run clang on the above C source file:

% ./build/x86_64-unknown-linux-gnu/llvm/bin/clang -c issue65844.c
%

Step 5. Get sad that the above compiled without error. Set the bug aside for the day

Step 6. Review work in evening. Realize that above clang invocation isn't passing the equivalent of -Ctarget-feature=-sse2

Step 7. Rerun clang the right way:

% ./build/x86_64-unknown-linux-gnu/llvm/bin/clang -c issue65844.c -mno-sse2
clang-9: /home/pnkfelix/Dev/Mozilla/rust.git/src/llvm-project/llvm/lib/Target/X86/X86FloatingPoint.cpp:317: unsigned int getFPReg(const llvm::MachineOperand&): Assertion `Reg >= X86::FP0 && Reg <= X86\
::FP6 && "Expected FP register!"' failed.
Stack dump:
[...]
clang-9: note: diagnostic msg: PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.
clang-9: note: diagnostic msg:
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-9: note: diagnostic msg: /tmp/issue65844-3b4b26.c
clang-9: note: diagnostic msg: /tmp/issue65844-3b4b26.sh
clang-9: note: diagnostic msg:

********************
[ERROR#1] %

Woot!

Time to finally file that LLVM bug!

@pnkfelix
Copy link
Member

Filed https://bugs.llvm.org/show_bug.cgi?id=44413

At this point I'm going to downgrade this to P-medium. (It is not really reasonable for us to try to fix this on our end; it will need to wait for LLVM to fix its bug, and then us to either cherry-pick the fix, or wait for an LLVM upgrade.)

@pnkfelix pnkfelix added P-medium Medium priority and removed P-high High priority labels Dec 31, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Dec 31, 2019

LLVM posted proposed "fix" (*) in llvm/llvm-project@e898ba2 ; see also topper's comment on my bug report

(*) at least fix in terms of removing the assertions; the codegen is still wrong, but they say we should at least get a diagnostic now in addition to no assertions...

@pnkfelix
Copy link
Member

pnkfelix commented Jan 2, 2020

they say we should at least get a diagnostic now in addition to no assertions...

I tried the patch locally; I do not see any diagnostics from rustc in this case.

(That may be due to the issue @nikic noted above. Will investigate.)

@pnkfelix
Copy link
Member

Its possible that LLVM's preferred solution may be this one: https://reviews.llvm.org/D70465

@fu5ha
Copy link
Contributor

fu5ha commented Sep 21, 2020

This also happens when compiling with target-feature=sse2 rather than explicitly disabling (target-feature=-sse2) with opt-level=0

Also, possibly related but slightly different (I can open another issue if you want), when building with target-feature=sse2 and --release (opt-level=3), I get a different segfault within rustc..

error: could not compile `wide`.                                                                                                                                                                                                                Caused by:                                                                                                                
process didn't exit successfully: `rustc --crate-name wide --edition=2018 C:\Users\Gray\.cargo\registry\src\github.com-1ecc6299db9ec823\wide-0.5.2\src\lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts --crate-type lib --emit=dep-info,metadata,link -C opt-level=3 -Cembed-bitcode=no --cfg "feature=\"default\"" -C metadata=0c5a0cff74905c8c -C extra-filename=-0c5a0cff74905c8c --out-dir C:\Users\Gray\Code\ultraviolet\target\release\deps -C linker=rust-lld.exe -L dependency=C:\Users\Gray\Code\ultraviolet\target\release\deps --extern bytemuck=C:\Users\Gray\Code\ultraviolet\target\release\deps\libbytemuck-9756b0f38fd1c716.rmeta --extern safe_arch=C:\Users\Gray\Code\ultraviolet\target\release\deps\libsafe_arch-d2a7b20d16175fe1.rmeta --cap-lints allow -C target-feature=fma,sse2` (exit code: 0xc0000005, STATUS_ACCESS_VIOLATION)                                                                                                               

warning: build failed, waiting for other jobs to finish...                                                              
error: build failed

@shepmaster
Copy link
Member

This continues to occur with Rust 1.60 and 1.62.0-nightly (e1b71fe 2022-05-03). I've seen it in the wild with both the ryu and serde crates.

@pnkfelix
Copy link
Member

pnkfelix commented May 9, 2022

This continues to occur with Rust 1.60 and 1.62.0-nightly (e1b71fe 2022-05-03). I've seen it in the wild with both the ryu and serde crates.

My working assumption is that the continued occurrences are due to https://bugs.llvm.org/show_bug.cgi?id=44413 still being unclosed on the LLVM side. However, there is this comment over there:

I think I fixed the assert/crash in e898ba2d151d621dcfc35828aad6fcded5a554e8. The code generated is completely wrong and does not follow any ABI, but we did signal an error through the diagnostic system as well.

which would imply that we shouldn't be seeing an assert/crash anymore, just ... wrong... codegen...?

@mojingran
Copy link
Contributor

mojingran commented Jun 21, 2023

With the latest nightly, running the rustc command will only cause the "SSE2 register return with SSE2 disabled" error. AFAIK it is unfeasible to return a pair of doubles without SSE2, which makes this behavior normal. Compiling the C code provided by @pnkfelix with clang produces a similar error. I could also reproduce the crash/assertion failure with previous versions.
Are there any steps before we can close this issue? Feel free to point me out if I made a mistake.

Edit: I realized that compiling with the rustc built from source would give me rustc: /home/d50032309/rust/src/llvm-project/llvm/lib/Target/X86/X86FloatingPoint.cpp:318: unsigned int getFPReg(const llvm::MachineOperand&): Assertion Reg >= X86::FP0 && Reg <= X86::FP6 && "Expected FP register!" failed. Aborted (core dumped), but the same version of rustc built from rustup would give me a different error which is "SSE2 register return with SSE2 disabled". I am inclined to believe that the latter error is the fix to this problem, though it is weird that they are giving different results...

@zackw
Copy link
Contributor

zackw commented Jun 27, 2023

Without SSE I would expect a pair of doubles to be returned on the stack, "by invisible reference," i.e. caller supplies an invisible zeroth argument (or possibly N+1'th argument, it's been a long time) which is a pointer to a space in caller's stack frame where the return value is to be written. Same as how it would be done in x86-32 mode, or how it would be done for returning a large struct that doesn't fit in registers at all.

@mojingran
Copy link
Contributor

mojingran commented Jul 4, 2023

Noted. I would also add that:

  1. Compiling the C code on GCC with SSE disabled gives similar result, and with SSE2 disabled, generates assembly that could be inaccurate.
  2. According to the x86_64 ABI, the use of "double" requires SSE2 registers.
  3. SSE2 is ubiquitous, as x86_64 has SSE2 by definition.
    It could be possible to fix, but in all honesty there is rarely any use case even if we really fixed it. Why would we want to forcefully make a feature work when this has been realized with SSE2 that explicitly allows the use of doubles?
    Nonetheless, I am curious if this issue is causing a crash or just a graceful error; all I am implying from above is that if it is throwing a graceful error, then we should leave it as is. I am getting conflicting results from source-built rustc and rustup-built rustc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-bug Category: This is a bug. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. ICEBreaker-LLVM Bugs identified for the LLVM ICE-breaker group O-x86_64 Target: x86-64 processors (like x86_64-*) P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests