dalek-cryptography / curve25519-dalek

A pure-Rust implementation of group operations on Ristretto and Curve25519
Other
853 stars 422 forks source link

SIGILL / LLVM ERROR when compiling to a x86_64 target that lacks SSE2 instructions #601

Open japaric opened 8 months ago

japaric commented 8 months ago

Steps to reproduce

$ cargo new --lib repro
$ cd repro

$ echo '#![no_std]' > src/lib.rs
$ cargo add curve25519-dalek@4.1.1

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

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

using nightly, you get "LLVM ERROR: Do not know how to split the result of this operator!"


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  -p curve25519-dalek -j1 -Z build-std=core,alloc --target ./x86_64.json && echo OK
OK

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

perhaps the backend module could use #[cfg(not(target_feature = "sse2"))] to skip the runtime-detection and directly use the serial implementation? or maybe 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 LLVM ERROR


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.

tarcieri commented 8 months ago

See also:

RalfJung commented 8 months ago

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.

x86_64-unknown-none is a softfloat target so issues with code that uses hardware floating point operations is not too surprising. You seem to be looking for a no-OS hardfloat target, which I don't think we have.

tarcieri commented 8 months ago

FWIW both curve25519-dalek and the @RustCrypto crates impacted by this are written in a way where they have baseline pure Rust implementations that should work fine on softfloat targets, it's just that they also contain AVX2 implementations which are intended for other x86/x86_64 targets that are currently difficult to gate/disable on softfloat targets.

RalfJung commented 8 months ago

Okay, so the question is "how do I best write target-specific AVX2 implementations in a portable way such that the code builds even on softfloat targets", but there is not a request here to be able to use AVX2 instructions on softfloat targets?

That sounds reasonable and yeah we currently don't have a solution. I think my current preference is what I just sketched in https://github.com/rust-lang/rust/issues/117938:

Maybe we should declare (and have the feature-detect macros implement) that SSE features are never available on softfloat targets. Then we can compile functions with SSE #[target_features] into unreachable_unchecked and so their ABI does not matter so we can generate whatever LLVM IR we want.

tarcieri commented 8 months ago

Yep, from the perspective of all of these crates the AVX2 code is effectively dead on softfloat targets. The problem is that rustc is still trying to compile it anyway.

Compiling the relevant code into e.g. unreachable_unchecked! sounds good to me.