RustCrypto / universal-hashes

Collection of universal hashing functions
27 stars 13 forks source link

SIGILL / LLVM ERROR from rustc when compiling to a x86_64 target that lacks SSE2 instructions #189

Open japaric opened 11 months ago

japaric commented 11 months ago

Steps to reproduce

$ cargo new --lib repro
$ cd repro

$ echo '#![no_std]' > src/lib.rs
$ cargo add poly1305@0.8.0

$ rustup default 1.73.0
$ rustup target add x86_64-unknown-none
$ cargo b --target x86_64-unknown-none
error: could not compile `poly1305` (lib)

Caused by:
  process didn't exit successfully: `$RUSTUP_TOOLCHAIN/bin/rustc (..)` (signal: 4, SIGILL: illegal instruction)

at some point I was able to get a "LLVM ERROR" message but I can't no longer reproduce that


after some digging it seems that the lack of SSE2 instructions in the x86_64-unknown-none is the problem:

$ rustup default nightly-2023-11-15
$ rustc -Z unstable-options --print target-spec-json --target x86_64-unknown-none | grep features
  "features": "-mmx,-sse,-sse2,-sse3,-ssse3,-sse4.1,-sse4.2,-3dnow,-3dnowa,-avx,-avx2,+soft-float",

creating a custom target that adds back the sse2 feature makes the crate compile

$ rustc -Z unstable-options --print target-spec-json --target x86_64-unknown-none > x86_64.json

patch the JSON like this

--- before.json 2023-11-15 14:35:19.565448966 +0100
+++ x86_64.json 2023-11-15 14:35:10.115355396 +0100
@@ -5,8 +5,7 @@
   "crt-objects-fallback": "false",
   "data-layout": "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128",
   "disable-redzone": true,
-  "features": "-mmx,-sse,-sse2,-sse3,-ssse3,-sse4.1,-sse4.2,-3dnow,-3dnowa,-avx,-avx2,+soft-float",
-  "is-builtin": true,
+  "features": "-mmx,-sse3,-ssse3,-sse4.1,-sse4.2,-3dnow,-3dnowa,-avx,-avx2",
   "linker": "rust-lld",
   "linker-flavor": "gnu-lld",
   "llvm-target": "x86_64-unknown-none-elf",
$ rustup component add rust-src
$ cargo b -j1 -Z build-std=core --target ./x86_64.json && echo OK
OK

keeping the -sse2 produces "LLVM ERROR: Access past stack top!"

perhaps the auto_detect module could use #[cfg(not(target_feature = "sse2"))] to skip the auto-detection and directly use the soft implementation? or you could add something like this:

#[cfg(not(target_feature = "sse2"))]
compile_error!("compilation targets without SSE2 instructions are not supported");

which is nicer than the SIGILL


for additional context ("why are you even using this target?"): I was looking for a built-in "OS agnostic" / no-std target for a no-std rustls demo (rustls/rustls PR1534) and tried x86_64-unknown-none and run into this and other problems. I ended up using a custom target for the demo but I figured I should still report what I found.

by the way, the sha2 has a similar problem with this target. Let me know if you think I should report a similar issue in the hashes repo.

newpavlov commented 11 months ago

cpufeatures should support freestanding x86 targets, see https://github.com/RustCrypto/utils/pull/821

What do you get for target_os?

Small rant: we are dealing with a clear deficiency in both the Rust language and LLVM. Currently Rust only supports +feature and (effectively) ?feature, while we also need -feature, i.e. a way to indicate that autodetection code should eliminate branches dependent on the feature. While LLVM in theory should be able to compile SIMD-dependent functions for which we explicitly enabled target features dependent on hard floats.

tarcieri commented 11 months ago

@japaric there's a cfg(poly1305_force_soft) you can set to disable CPU feature autodetection.

Unfortunately these x86_64-unknown-none targets are something of a longstanding problem which I thought we had fixed at this point. Apparently not.

I'm a little curious why the existing cpufeatures detection for avx2 wouldn't prevent those codepaths from being taken in the first place: https://github.com/RustCrypto/universal-hashes/blob/master/poly1305/src/backend/autodetect.rs#L9

I would think that would prevent any such codepaths from being taken on x86_64-unknown-none already, so I'm kind of curious what SSE2 instructions are even being executed.

japaric commented 11 months ago

target_os for x86_64-unknown-none is "none"

$ rustc --print cfg --target x86_64-unknown-none | grep target_os
target_os="none"

I don't think target_os="none" is a good proxy for the instructions supported by a compilation target since one can create a custom compilation target that has the same codegen settings as nightly-x86_64-unknown-linux-gnu but set target_os to "none" like I have done in the issue description. target_feature could perhaps be more appropriate but it's lacking in what it currently exposes and I'm no x86_64 expert to know which instruction set / feature should be exposed over there to better support this particular scenario

tarcieri commented 11 months ago

cpufeatures does consult target_feature, but only to bypass runtime checks. Its main goal is to autodetect CPU features at runtime when they haven't been explicitly enabled.

I'm curious how you're running into a situation where the runtime CPUID checks are being bypassed and you're hitting SSE2 code.

newpavlov commented 11 months ago

I don't think target_os="none" is a good proxy for the instructions supported by a compilation target

There are currently no other way. Rust does not expose soft-float "target feature" (let's forget about it's peculiar "negativity"). And even if it did, I think it would be a bad way for handling this situation. We need something like this.

japaric commented 11 months ago

there's a cfg(poly1305_force_soft) you can set to disable CPU feature autodetection.

thanks. I was aware of poly1305_force_soft, curve25519_dalek_backend and sha2's force-soft Cargo feature but using those together with a particular mix of crypto libraries produced a compilation error in curve25519-dalek. that could be a different bug or PEBCAK that I didn't manage to identify -- at that point I gave up on the x86_64-unknown-none target and used a custom one.

tarcieri commented 11 months ago

Is it possible for you to capture this in a debugger and get a backtrace of some sort? I'm curious where the SIGILL is actually occurring and whether or not it's inside a backend which should not be compiled at all on x86_64-unknown-none targets

newpavlov commented 11 months ago

Can you try to compile you code with --release? Maybe LLVM simply fails to eliminate the SIMD branch with opt-level=0 and thus attempts to compile it? The cpufeatures module simply results in false, while the branch is still present in source code.

japaric commented 11 months ago

I'm curious where the SIGILL is actually occurring and whether or not it's inside a backend which should not be compiled at all on x86_64-unknown-none targets

I think the SIGILL is Cargo misreporting the rustc error on stable. if you follow the repro steps with nightly-2023-11-15 you get this LLVM error and no SIGILL

Compiling poly1305 v0.8.0
LLVM ERROR: Do not know how to split the result of this operator!

error: could not compile `poly1305` (lib)
japaric commented 11 months ago

Can you try to compile you code with --release?

interestingly, adding --release produces the SIGILL on both stable and nightly. I don't get the "LLVM ERROR" message using --release

newpavlov commented 11 months ago

Oh... It's SIGILL triggered by the Rust compiler, not "LLVM ERROR: Do not know how to split the result of this operator!". The latter should be handled by the cpufeatures change linked earlier.

I think you should report this error to the Rust lang repository.

japaric commented 11 months ago

the Cargo backtrace does not say much

Thread 2 "cargo" received signal SIGUSR1, User defined signal 1.
[Switching to Thread 0x7ffff7c8e6c0 (LWP 99944)]
0x00007ffff7d9556c in read () from /usr/lib/libc.so.6
(gdb) backtrace
#0  0x00007ffff7d9556c in read () from /usr/lib/libc.so.6
#1  0x0000555556a859d6 in std::sys::unix::fd::FileDesc::read () at library/std/src/sys/unix/fd.rs:90
#2  std::sys::unix::fs::File::read () at library/std/src/sys/unix/fs.rs:1162
#3  std::fs::{impl#5}::read () at library/std/src/fs.rs:749
#4  0x0000555556a535cf in <jobserver[63568e20ee72e952]::imp::Client>::acquire_allow_interrupts ()
#5  0x0000555556a5541d in <jobserver[63568e20ee72e952]::HelperState>::for_each_request::<jobserver[63568e20ee72e952]::imp::spawn_helper::{closure#1}::{closure#0}> ()
#6  0x0000555556a5585b in std[3759e478f3a6c4f2]::sys_common::backtrace::__rust_begin_short_backtrace::<jobserver[63568e20ee72e952]::imp::spawn_helper::{closure#1}, ()> ()
#7  0x0000555556a56a49 in <<std[3759e478f3a6c4f2]::thread::Builder>::spawn_unchecked_<jobserver[63568e20ee72e952]::imp::spawn_helper::{closure#1}, ()>::{closure#1} as core[d28c4e8d9c4eebaa]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0} ()
#8  0x0000555556a9bcd5 in alloc::boxed::{impl#47}::call_once<(), dyn core::ops::function::FnOnce<(), Output=()>, alloc::alloc::Global> () at library/alloc/src/boxed.rs:2007
#9  alloc::boxed::{impl#47}::call_once<(), alloc::boxed::Box<dyn core::ops::function::FnOnce<(), Output=()>, alloc::alloc::Global>, alloc::alloc::Global> () at library/alloc/src/boxed.rs:2007
#10 std::sys::unix::thread::{impl#2}::new::thread_start () at library/std/src/sys/unix/thread.rs:108
#11 0x00007ffff7d1e9eb in ?? () from /usr/lib/libc.so.6
#12 0x00007ffff7da27cc in ?? () from /usr/lib/libc.so.6

running the rustc invocation that Cargo prints through the debugger seems to indicate that something in LLVM triggered the SIGILL

Thread 7 "opt cgu.1" received signal SIGILL, Illegal instruction.
[Switching to Thread 0x7fffe11ff6c0 (LWP 100591)]
0x00007ffff13c939f in llvm::X86TargetLowering::ReplaceNodeResults(llvm::SDNode*, llvm::SmallVectorImpl<llvm::SDValue>&, llvm::SelectionDAG&) const [clone .cold.0] ()
   from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM-17-rust-1.73.0-stable.so
(gdb) backtrace
#0  0x00007ffff13c939f in llvm::X86TargetLowering::ReplaceNodeResults(llvm::SDNode*, llvm::SmallVectorImpl<llvm::SDValue>&, llvm::SelectionDAG&) const [clone .cold.0] ()
   from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM-17-rust-1.73.0-stable.so
#1  0x00007ffff1130ef5 in llvm::DAGTypeLegalizer::SplitVectorResult(llvm::SDNode*, unsigned int) [clone .cold.0] () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM-17-rust-1.73.0-stable.so
#2  0x00007ffff000d41a in llvm::DAGTypeLegalizer::run() () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM-17-rust-1.73.0-stable.so
#3  0x00007ffff01ff81a in llvm::SelectionDAGISel::SelectBasicBlock(llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void>, false, true>, llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void>, false, true>, bool&) () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM-17-rust-1.73.0-stable.so
#4  0x00007ffff04ff282 in llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM-17-rust-1.73.0-stable.so
#5  0x00007ffff034fc0a in llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM-17-rust-1.73.0-stable.so
#6  0x00007ffff034f4ee in (anonymous namespace)::X86DAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) [clone .llvm.6232165262612102610] () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM-17-rust-1.73.0-stable.so
#7  0x00007ffff016d66a in llvm::FPPassManager::runOnModule(llvm::Module&) () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM-17-rust-1.73.0-stable.so
#8  0x00007ffff05dcaac in llvm::legacy::PassManagerImpl::run(llvm::Module&) () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM-17-rust-1.73.0-stable.so
#9  0x00007ffff64d36a6 in LLVMRustWriteOutputFile () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-453cf35e1dd187fa.so
#10 0x00007ffff64d2558 in rustc_codegen_llvm[13e834ec38ef84a5]::back::write::write_output_file () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-453cf35e1dd187fa.so
#11 0x00007ffff64cfcd4 in rustc_codegen_llvm[13e834ec38ef84a5]::back::write::codegen () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-453cf35e1dd187fa.so
#12 0x00007ffff64cd074 in rustc_codegen_ssa[1239057ba2d16fcb]::back::write::finish_intra_module_work::<rustc_codegen_llvm[13e834ec38ef84a5]::LlvmCodegenBackend> () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-453cf35e1dd187fa.so
#13 0x00007ffff64cc75d in rustc_codegen_ssa[1239057ba2d16fcb]::back::write::execute_optimize_work_item::<rustc_codegen_llvm[13e834ec38ef84a5]::LlvmCodegenBackend> () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-453cf35e1dd187fa.so
#14 0x00007ffff64ca627 in std[3759e478f3a6c4f2]::sys_common::backtrace::__rust_begin_short_backtrace::<<rustc_codegen_llvm[13e834ec38ef84a5]::LlvmCodegenBackend as rustc_codegen_ssa[1239057ba2d16fcb]::traits::backend::ExtraBackendMethods>::spawn_named_thread<rustc_codegen_ssa[1239057ba2d16fcb]::back::write::spawn_work<rustc_codegen_llvm[13e834ec38ef84a5]::LlvmCodegenBackend>::{closure#0}, ()>::{closure#0}, ()> () from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-453cf35e1dd187fa.so
#15 0x00007ffff6459256 in <<std[3759e478f3a6c4f2]::thread::Builder>::spawn_unchecked_<<rustc_codegen_llvm[13e834ec38ef84a5]::LlvmCodegenBackend as rustc_codegen_ssa[1239057ba2d16fcb]::traits::backend::ExtraBackendMethods>::spawn_named_thread<rustc_codegen_ssa[1239057ba2d16fcb]::back::write::spawn_work<rustc_codegen_llvm[13e834ec38ef84a5]::LlvmCodegenBackend>::{closure#0}, ()>::{closure#0}, ()>::{closure#1} as core[d28c4e8d9c4eebaa]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0} ()
   from /home/japaric/.rustup/toolchains/1.73.0-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-453cf35e1dd187fa.so
#16 0x00007ffff3d71295 in alloc::boxed::{impl#47}::call_once<(), dyn core::ops::function::FnOnce<(), Output=()>, alloc::alloc::Global> () at library/alloc/src/boxed.rs:2007
#17 alloc::boxed::{impl#47}::call_once<(), alloc::boxed::Box<dyn core::ops::function::FnOnce<(), Output=()>, alloc::alloc::Global>, alloc::alloc::Global> () at library/alloc/src/boxed.rs:2007
#18 std::sys::unix::thread::{impl#2}::new::thread_start () at library/std/src/sys/unix/thread.rs:108
#19 0x00007ffff3ae59eb in ?? () from /usr/lib/libc.so.6
#20 0x00007ffff3b697cc in ?? () from /usr/lib/libc.so.6
tarcieri commented 11 months ago

Aha, we've definitely encountered that "LLVM ERROR: Do not know how to split the result of this operator!" before:

https://github.com/RustCrypto/AEADs/issues/405 https://github.com/RustCrypto/AEADs/issues/498 https://github.com/RustCrypto/hashes/issues/446

I thought we had successfully worked around it in https://github.com/RustCrypto/utils/pull/821 though. Apparently not.

newpavlov commented 11 months ago

@tarcieri I think our workarounds works fine, but it triggers a Rust/LLVM compiler bug, which results in SIGILL.

tarcieri commented 11 months ago

Yeah, something very strange is happening

japaric commented 11 months ago

reported this in the rust-lang/rust repo: https://github.com/rust-lang/rust/issues/117938