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

[girgen]

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]

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

[girgen]

See the attached log above ☝️

Looks like

--target i686-unknown-freebsd

[briansmith]

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]

% 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]

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]

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]

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]

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]

Yes, the also applies to that target.

[guenhter]

@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]

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]

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]

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]

@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]

@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]

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]

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.

[jackpot51]

@briansmith would it help if a PR was made with the patch that debian is using? It looks okay to me.

Here is the comparison, it needs to be rebased but will look the same: https://github.com/briansmith/ring/compare/main...redox-os:ring:redox-0.17.8

[briansmith]

After PR #2295 is merged, it will be very clear what needs to be done to resolve this properly:

• Review ChaCha20_ctr32_nohw to see if it uses SIMD instructions.
• Add a Sse2 to cpu::intel analogous to Ssse3.
• Rename bn_mul_mont to bn_mul_mont_sse2 in x86-mont.pl.
• Add bn_mul_mont_sse2 to the list of functions to prefix in build.rs.
• Add some kind of non-SSE2-capable QEMU-based testing to the coverage job in ci.yml; see the other QEMU-based x86 coverage jobs in that file to see how it is done. I am not sure if it will actually work though, since I think most OSs that might be usable within QEMU require SSE2.
• Change limbs_mul_mont to check for Sse2 before calling bn_mul_mont_see2 on x86, or otherwise call the fallback of bn_mul_mont that is also implemented in the same file.
• Review https://github.com/rust-lang/rust/issues/114479 to see how it would affect the correctness of ring, if at all.
• Remove the assertion mentioned in the subject of this issue.

Note that BoringSSL, last year, discarded the non-SSE2 code in bn_mul_,mont because they were never testing it. That makes me am glad we discarded it beforehand.

[briansmith]

@briansmith would it help if a PR was made with the patch that debian is using? It looks okay to me.

Here is the comparison, it needs to be rebased but will look the same: main...redox-os:ring:redox-0.17.8

That patch doesn't look good to me, as it disables all CPU feature detection on x86, AFAICT. That will destroy performance. What we really want is to use the fast implementations when SIMD is available based on dynamic CPU feature detection, and fall back to the slow fallback implementations only when absolutely necessary.

[briansmith]

PR #2346 lays the foundation for adding dynamic detection of SSE/SSE2 to ring to targets that don't enable SSE/SSE2 by default. In that PR (and the ones leading up to it) I have cited the relevant sections of the Intel documentation and left breadcrumbs regarding what needs to be updated if/when we add SSE/SSE2 dynamic detection. I would appreciate people taking a look at that PR.

Also, in ci.yml, in the coverage section, there are several jobs like these:

          - target: i686-unknown-linux-gnu
            cpu_model: coreduo-v1
            features: --features=std
            mode: --release
            rust_channel: nightly
            host_os: ubuntu-24.04

...

          - target: i686-unknown-linux-gnu
            cpu_model: Conroe-v1
            features: --features=std
            mode: --release
            rust_channel: nightly
            host_os: ubuntu-24.04

The cpu_model field is a qemu-i386 CPU name; see qemu-i386 -cpu help for a list of names. We should look up the last model that didn't support SSE, and the first model that did support SSE, and add entries for those two models. Similarly, we should find the last model that supported SSE but not SSE2. and add that model to the list. (I think I've already added entries for the first models that supported SSE2?)

[briansmith]

Hi @plugwash, thank you for your work adapting ring to work with Debian's platform support policies. I saw your patch that you linked above. I think it would be great if we could have the Debian patch adjusted so that on systems where SSE2 isn't the baseline (target_feature = "sse2" is false), ring would still use the SSE2 (and SSSE3, and AES-NI, etc.) implementations if it detects that the CPU supports the SIMD features. Here are some notes on your patch:

• In the cases where the ring code is already checking for SSSE3 or some other CPU feature before calling the assembly language function, you maybe don't need to patch anything, as we've already dynamically checked for those CPU features in cpu_intel.c/cpu::intel. The CPU won't advertise SSSE3 or AES-NI, etc., if it doesn't support SSE2. This applies to your patch to 0.17.8 and any upcoming changes. So, for example, in your patch you have:

#[cfg(any(target_arch = "x86_64", target_arch = "x86"))]
+    #[cfg(any(target_arch = "x86_64", all(target_arch = "x86", target_feature = "sse2")))]
     {
         if cpu::intel::AES.available(cpu_features) {
             return Implementation::HWAES;
         }
     }

This patch isn't necessary to keep ring working on SSE2-less systems. And it is harmful because it prevents ring from using the AES-NI implementation, which is much, much, much faster.

• In 0.17.8, the version you patched for Debian, many of these check for CPU features were in the assembly language code. I worked with BoringSSL upstream to pull these checks out of the assembly language code and move them into Rust code (C code in BoringSSL) in part to make it easier for maintainers like you to understand and improve the behavior. Now you only need to patch the fallback case for x86 where we call an assembly language function without checking for any CPU features. Thus, even without adding any new dynamic SSE2-detecting code, you can eliminate most of your patch, and you can minimize the negative effects of the patching to cases where there is no SSSE3. Here is an example of the current ring main branch code:

            } else if #[cfg(target_arch = "x86")] {
                use cpu::{GetFeature as _, intel::Ssse3};
                if in_out.len() >= 1 {
                    if let Some(cpu) = cpu.get_feature() {
                        chacha20_ctr32_ffi!(
                            unsafe { (1, Ssse3, &mut [u8]) => ChaCha20_ctr32_ssse3 },
                            self, counter, in_out.copy_within(), cpu)
                    } else {
                        chacha20_ctr32_ffi!(
                            unsafe { (1, (), &mut [u8]) => ChaCha20_ctr32_nohw },
                            self, counter, in_out.copy_within(), ())
                    }
                }
            } else if #[cfg(target_arch = "x86_64")] {

Here, the final else branch can be patched to something like:

                    } else if cfg!(target_feature = "sse2") {
                        chacha20_ctr32_ffi!(
                            unsafe { (1, (), &mut [u8]) => ChaCha20_ctr32_nohw },
                            self, counter, in_out.copy_within(), ())
                    } else {
                         fallback::ChaCha20_ctr32(self, counter, in_out)
                    }

This will keep ring working well for the vast majority of people who have SSSE3 and later features, while keeping it working for people who don't.

• In cases where the ring code needs to be patched for if cfg!(target_feature = "sse2"), we can and should eventually change this to if let Some(cpu) = cpu.get_feature() where this will detect SSE2 dynamically. it is hard to add the dynamic SSE2 detection in the code currently because of how the cpu_intel.c code from BoringSSL is structured. But, it will be easier if/when PR #2346 is merged.

However, I really think the 0.17.8 patch should be improved to minimize the harm it does. Or, I can reprioritize the release of 0.17.9 and we can work together to minimize the harm of the 0.17.9 patching.

[briansmith]

Also, since you're a Debian maintainer, this is just as good of a place as any to note that I intend for the 0.17.9 release to also have MSRV 1.63, (ring 0.17.8 MSRV was 1.61, IIRC, but Debian Stable has Rust 1.63, IIUC.)

[briansmith]

 } else if cfg!(target_feature = "sse2") {
                       chacha20_ctr32_ffi!(
                           unsafe { (1, (), &mut [u8]) => ChaCha20_ctr32_nohw },
                           self, counter, in_out.copy_within(), ())
                  } else {
                        fallback::ChaCha20_ctr32(self, counter, in_out)
                   }

Bad example, as that function actually doesn't require SSE. chacha.rs doesn't need to be patched at all, for x86, in the main branch, or in 0.17.8.

[briansmith]

AFAICT, montgomery.rs is the only place that needs to be patched for SSE2, under the very reasonable assumption that every AES-NI-capable CPU will support SSE2 (the CPU and OS must already support SSE registers for AES-NI to work).

[briansmith]

To make the patching easier, I've submitted PR #2358.

[briansmith]

PR #2361 brings in BoringSSL's checks for SSE2 on the C side, and changes build.rs to automatically pass "-msse2" to the C compiler for x86 targets. PR #2363 tries to reduce the negative effects of removing the SSE2/SSE2 checks by adding minimal dynamic SSE2 feature detection. If you've patched ring to remove the SSE2 requirement, and especially if you're using the patches linked above, please test that out and report back.

[briansmith]

In PR #2359, I added a requirement that SSSE3 be detected in order to use AES-NI. (Technically SSE2 is all that is required but I don't think there are any non-SSSE3 AES-NI-capable systems.)

[briansmith]

I've rebased PR #2346 on top of all the changes I merged yesterday, mentioned above.

[briansmith]

In PR #2368 I add a test that you should ensure passes (except on ancient CPUs) in your patched versions of ring.