assert!(cfg!(target_feature = "sse") && cfg!(target_feature = "sse2")) panic at build on i386, i586, FreeBSD, or many target_os=none x86 targets.

girgen commented 6 months ago

Hi!

I get this error when building an app using ring-0.17.8

error[E0080]: evaluation of constant value failed
 --> /wrkdirs/usr/ports/www/sqlpage/work/SQLpage-0.20.0/cargo-crates/ring-0.17.8/src/cpu/[intel.rs:28](http://intel.rs:28/):9
  |
28 |         assert!(cfg!(target_feature = "sse") && cfg!(target_feature = "sse2"));
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: cfg!(target_feature = "sse") && cfg!(target_feature = "sse2")', /wrkdirs/usr/ports/www/sqlpage/work/SQLpage-0.20.0/cargo-crates/ring-0.17.8/src/cpu/[intel.rs:28](http://intel.rs:28/):9
  |
  = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0080`.
error: could not compile `ring` (lib) due to 1 previous error

I tried to find info using the three issue links in the code, but I don't have enough experience with rust, to be honest, I'm just packaging an application. It seems to me that the problem is with the assert call itself, so perhaps this is actually a bug?

Removing the check will make the code build.

Do you need more information, just specify what is needed. Build log attached.

sqlpage-0.20.0.log

briansmith commented 6 months ago

What are the steps to reproduce? cargo build --target=????

girgen commented 6 months ago

See the attached log above ☝️

Looks like

--target i686-unknown-freebsd

briansmith commented 6 months ago

What do you get when you do this?

$ rustc --print=cfg --target i686-unknown-freebsd | grep sse
debug_assertions
target_feature="sse"
target_feature="sse2"

For me, rustc indicates that sse and sse2 are there.

I see in your build log that there are warnings that CARGO_CFG_TARGET_FEATURE not set. THat might be a clue.

Make sure you're not disabling SSE/SSSE2 in .cargo/config.toml or otherwise.

girgen commented 6 months ago

% rustc --print=cfg --target i686-unknown-freebsd | grep sse
debug_assertions

Seems like rust is configured for pentiumpro (no SSEx) in the FreeBSD ports tree?

See https://github.com/freebsd/freebsd-ports/blob/main/lang/rust/files/patch-compiler_rustc__target_src_spec_targets_i686__unknown__freebsd.rs

briansmith commented 5 months ago

At https://www.freebsd.org/releases/14.0R/hardware/#proc-i386, it is written:

2.3. i386 Architecture Support FreeBSD maintains support for i386 (x86) as a Tier 2 architecture. It is not recommended for new installations.

Support for this architecture will be removed in FreeBSD 15.0.

dch commented 4 months ago

FreeBSD 15.0 hasn't been released yet, so this is in the future. We still have FreeBSD 13.3-RELEASE and 14.1-RELEASE active, which will run for several more years yet.

For me, this blocks e.g. lang/gleam on i386, which admittedly is neither a popular port, nor a very common platform yet.

Do we amend the i386 target on FreeBSD, for rust? @MikaelUrankar I know you're familiar with the FreeBSD rust porting, what do you suggest here?

Perhaps we should discuss this in bugzilla?

briansmith commented 4 months ago

Note that you can build ring with RUSTFLAGS="-C target-feature=+sse2" (or whatever) to get it to work on those targets. Note that it might have built before I added these assertions, but if you tried to run it it would execute SSE2 instructions without checking at runtime whether the system actually supported SSE2 instructions.

If you actually want ring to support x86 systems that don't have SSE2, then we'd need somebody to submit a PR that adds the runtime checking for SSE2 to every assembly language function called. They are easy to find; look for target_arch="x86" in the source code. To see how to add the check for SSE2, look at how the check for SSSE3 is done by searching for SSSE3.

he32 commented 3 months ago

I'll just note that I'm observing the same problem for the i586-unknown-netbsd target. It also does not support the SSE or SSE2 instructions, and targets Pentium and above, for maximal backward compatibility:

$ rustc --print=cfg --target i586-unknown-netbsd | grep sse
debug_assertions
$ rustc --print=cfg | grep sse
debug_assertions
$ uname -rps
NetBSD 9.3 i386
$ rustc --version
rustc 1.78.0 (9b00956e5 2024-04-29) (built from a source tarball)
$

This is seen for ring-0.17.8:

error[E0080]: evaluation of constant value failed
  --> /usr/pkgsrc/net/routinator/work/vendor/ring-0.17.8/src/cpu/intel.rs:28:9
   |
28 |         assert!(cfg!(target_feature = "sse") && cfg!(target_feature = "sse2"));
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: cfg!(target_feature = "sse") && cfg!(target_feature = "sse2")', /usr/pkgsrc/net/routinator/work/vendor/ring-0.17.8/src/cpu/intel.rs:28:9

briansmith commented 3 months ago

Yes, the also applies to that target.

guenhter commented 1 month ago

@briansmith I'd like to help out supporting the i586 target. After looking into the code, I have no clue what exactly I should do. Also looking at SSSE3 didn't really help. Do I only have to adapt the rust code or also the .pl stuff? Would it be possible to show me one example what I have to do, so that I can do it then on all other places where target_arch="x86" is set.

Fabian-Gruenbichler commented 3 weeks ago

this also affects Debian's i386 arch, since that has neither SSE nor SSE2. not building ring anymore there would remove a huge chunk of Rust packages in turn. is anybody already working on such a conditionalizing patch?

plugwash commented 2 weeks ago

I've written a patch that forces a generic no-asm build on x86 targets without sse2 and uploaded it to Debian.

https://salsa.debian.org/rust-team/debcargo-conf/-/blob/d586098f2597e5b5a1dcbf87af5268b3b84206e1/src/ring/debian/patches/use-generic-implementation-on-non-sse2-x86.patch

Obviously this is not optimal, runtime detection would be better but it stops this being a rc bug for us.

briansmith commented 1 week ago

I am open to addressing this issue but I don't intend to work on it myself any time soon except to review a PR that fixes it and which is tested in GitHub Actions CI in some way. I suggest we use i586-unknown-linux-gnu as the target to test in GitHub Actions. The PR should make the SSE/SSE2 detection work like other feature detection currently works and which matches the current (at the time of reviewing the PR) coding conventions in ring.

guenhter commented 1 week ago

@briansmith I'm completly open to make the PR. It would just be very helpful if you could show me a single location where action is required and what needs to be done. The rest can be done by me.

briansmith commented 1 week ago

@briansmith I'm completly open to make the PR. It would just be very helpful if you could show me a single location where action is required and what needs to be done. The rest can be done by me.

[ ] In cpu::intel, you need to do the analog of this to define a new SSE2, for target_arch = "x86" only. The mask value needs to be looked up in the intel reference:

#[allow(dead_code)]
pub(crate) const SSE41: Feature = Feature {
    word: 1,
    mask: 1 << 19,
};

[ ] Look at all the files named x86-*.pl or *-x86.pl. Each of these files declares some functions like &function_begin("bn_mul_mont");. If the function name doesn't start with an underscore then it is extern. For all such extern functions in these files (only), rename the function to add a suffix "_sse2", e.g. bn_mul_mont_sse2.
[ ] Change build.rs to add these *_sse2 functions' names to the list in SYMBOLS_TO_PREFIX.
[ ] For each prefixed_extern! { fn foo() } in the Rust code that mentions target_arch = "x86", you'll need to augment/replace it with #[cfg(target_arch = "x86")] prefixed_extern! { fn foo_sse2() } and you'll need to define a new implementation of foo that looks something like this:

#[cfg(target_arch = "x86")]
extern "C" unsafe fn foo(...) { 
         if cfg!(target_feature = "sse2") || cpu::intel::SSE2.available(cpu::features())) {
             unsafe { foo_sse2(....) };
         } else {
             <Call the fallback implementation that is used on targets that don't support assembly language.>
         }
}

I suggest you do this with bn_mul_mont first. You'll find it declared in montgomery.rs and defined (for 32-bit x86) in x86-mont.pl. In the case of bn_mul_mont you'll probably want to start by splitting the Rust implementation of bn_mul_mont into two parts, one being bn_mul_mont_fallback that contains the actual fallback implementation, and bn_mul_mont that is a thin wrapper around bn_mul_mont_fallback. Note that bn_mul_mont is special because it is also called by C code so it needs the prefixed_export! hack mentioned in montgomery.rs.

In some cases, the assembly code already depends on some other feature, like AES-NI. In that case, we can change OPENSSL_cpuid_setup so that no features are detected if SSE and SSE2 aren't detected; this might already be the case. For these cases where some other feature is being detected, you won't need to do any work. It is only when the fallback implementation on 32-bit x86 is an assembly function that you need to take the above steps.

You can get a sense for how much work this is by going through the code looking for "prefixed_extern!" that has a target_arch = "x86" in its cfg!; for each one, check the caller(s) to see if they are already doing feature detection for x86; if so, you don't need to touch that one. Otherwise, if the function is the fallback implementation, you need to take the above steps.

guenhter commented 5 days ago

Thx for this comprehensive guide. I'll do my best to implement that. From a timeline perspective I hope to be able to start with this in the next weeks.

briansmith commented 4 days ago

Great!

In terms of testing, look at how qemu is used in CI to test on older CPUs. Then you can run cargo test under QEMU with a very old CPU model specified to see that it is working.

briansmith / ring

assert!(cfg!(target_feature = "sse") && cfg!(target_feature = "sse2")) panic at build on i386, i586, FreeBSD, or many target_os=none x86 targets. #1999