rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.69k stars 12.75k forks source link

Warn if feature settings may break compilation #89586

Open workingjubilee opened 3 years ago

workingjubilee commented 3 years ago

Currently, if a user instructs the compiler to adjust the features for compilation, or uses #[target_feature] to set unusual feature settings for a function, or introduces a custom target, this can trigger miscompilations if the settings are improperly "aligned" with each other (to do correct parameter passing and so on). This mostly impacts x86, due to its particular architectural extensions, but it could affect other platforms.

Some examples we definitely want to warn on:

Some of these issues are currently caught by LLVM, but Rust programmers often find underlying LLVM errors surfacing to be mysterious and cryptic, and in this case we can definitely detect them and warn about them ourselves.

In addition, due to the desire for binary floating point conformance per https://github.com/rust-lang/rust/issues/10087, we probably want to emit a warning for any build configuration such that, despite having an FPU we consider to be conformant and desirable, disables the ability to use such a floating point unit, such that it could introduce non-conformant floating point code. However, that can be extended into a future issue when all the known-100%-bad bases are covered.

workingjubilee commented 3 years ago

This requires digging deep into the target_feature code and likely refactoring it. Much of it lives in https://github.com/rust-lang/rust/tree/master/compiler/rustc_codegen_llvm, most particularly this file, which handles things like the interface to LLVM's target features: https://github.com/rust-lang/rust/blob/master/compiler/rustc_codegen_llvm/src/llvm_util.rs Especially this: https://github.com/rust-lang/rust/blob/0fb1c371d4a14f9ce7a721d8aea683a6e6774f6c/compiler/rustc_codegen_llvm/src/llvm_util.rs#L324-L332 And this related function may also need to be reviewed, as it handles function-level setting of codegen features, like with #[target_feature]: https://github.com/rust-lang/rust/blob/0fb1c371d4a14f9ce7a721d8aea683a6e6774f6c/compiler/rustc_codegen_llvm/src/attributes.rs#L227-L229

And at least some of this logic should probably be, as part of this, hoisted into https://github.com/rust-lang/rust/tree/master/compiler/rustc_codegen_ssa. Accordingly, while it is a scoped piece of work, it is not a trivial task either.

Wardenfar commented 2 years ago

Hello @workingjubilee,

I'm trying to understand the issue and i have some questions.

bstrie commented 2 years ago

@Wardenfar You may want to try asking on the #t-compiler Zulip channel: https://rust-lang.zulipchat.com/#streams/131828/t-compiler

workingjubilee commented 2 years ago

My apologies for taking so long to answer. Yes, I am more reachable on Zulip.

    • Q: Should we detect incompatible features at the global feature level?
    • A: Yes, definitely.
    • Q: Should we detect incompatible features at the function feature level?
    • A: Ideally also yes.
    • Q. How is the x87 FPU denoted?
    • A. The x87 FPU feature uses the string "+x87" to enable and "-x87" to disable
    • Q. How to report an error?
    • A. Yes, sess.err or any of the other typical error emitters should be fine, including builders like struct_span_err.
workingjubilee commented 1 month ago

this is turning out to be way more complex than I thought it would possibly be, see: