Support `is_x86_feature_detected!()`

[abonander]

Miri for me is choking on usages of is_x86_feature_detected!() (specifically the inline assembly it uses to invoke cpuid) in two different crates:

• memchr
• httparse

These crates of course can be patched to use #[cfg(miri)] but it seems to me like Miri could also just override the internals of is_x86_feature_detected!() to return false for all features it doesn't support, so these crates automatically fall back to implementations that Miri may already be able to evaluate. memchr may need further patching to work correctly in Miri but I think this would be worth the effort anyway.

[RalfJung]

Yeah inline assembly won't happen any time soon...

Do you know where is_x86_feature_detected is implemented?

[bjorn3]

https://doc.rust-lang.org/stable/src/std/up/stdsimd/crates/std_detect/src/detect/arch/x86.rs.html#82-242

The macro first checks if it is enabled for the whole crate using cfg!(target_feature="...") and if not it goes to std_detect. You can prevent the cpuid intrinsic from being called by patching has_cpuid to return false. https://github.com/bjorn3/rustc_codegen_cranelift/blob/1018a34662e0b8d9dfa650ed0ee1dfd84242ac37/patches/0016-Disable-cpuid-intrinsic.patch

[RalfJung]

So maybe we could convince the stdarch maintainers to add a cfg(miri) there to disable cpuid? Does someone want to take that over?

[lu-zero]

ideally we could add a cfg(miri) in std::detect::os::detect_features.

[lu-zero]

Or actually in the function calling it :)

[RalfJung]

rust-lang/stdarch#803 landed. What does that mean for this issue? Can we land a new test in Miri for this?

[lu-zero]

You may have to wait until the change enters libstd, I'm afraid.

[gnzlbg]

So the way is_x86_feature_detected!("foo") is implemented for miri is as follows: it returns the same value as cfg!(target_feature = "foo") would.

This means that, if cfg!(target_feature = "sse") is true, e.g., because the target enables the SSE feature like x86_64-unknown-linux-gnu does, or because the user enables it in its cargo config or RUSTFLAGS, e.g., using RUSTFLAGS="-C target-feature=+sse", then branches for is_x86_feature_detected!("sse") will be taken by the program execution, causing miri to try to execute SSE intrinsics, which would fail if miri doesn't support the intrinsic.

This implementation of is_x86_feature_detected! is correct. It was proposed to have is_x86_feature_detected! to always return false when running on miri, but I think that this implementation strategy would be incorrect because it would mean that the moment the binary starts execution under miri the behavior is undefined. The reason is that the "miri CPU" that the binary runs on does not support a target-feature that the compiler assumes to be supported by any hardware running such a binary (and this is UB according to the language reference, see the section on behavior considered undefined).

From a language spec perspective, a user cannot use cfg!(miri) to avoid this undefined behavior. AFAICT the only way to avoid this is to compile the whole binary without the target features that are not supported by miri. For example, by using RUSTFLAGS="-C target-feature=-sse". Since miri probably doesn't support any or many features, I think that the most practical solution for this problem would be to add a "miri CPU" to Rust, so that miri can call rustc with -C target-cpu=miri instead, and that CPU would not have any of the features that miri does not support enabled.

This would mean that targeting, e.g., x86_64-unknown-linux-gnu under miri wouldn't really target that (since that target has SSE, SSE2, etc. enabled), but instead it would target x86_64-unknown-linux-gnu + -C target-cpu=miri which is a base CPU that's less "featureful" than what the base CPU of the target specification requires. This might be problematic in its own ways, but AFAICT this is already the case anyways when code runs under miri, and one could always fix that by making the "miri CPU" much powerful, e.g., by implementing features for it.

(cc @bjorn3 - this doesn't solve the related Cranelift issue at all which from this perspective is more complicated to solve I think, but it would be better to discuss how to solve that in a different issue).

[RalfJung]

@gnzlbg That all makes a lot of sense, thank you.