Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-C target-feature is unsafe #679

Open
gnzlbg opened this issue Sep 17, 2019 · 1 comment
Open

-C target-feature is unsafe #679

gnzlbg opened this issue Sep 17, 2019 · 1 comment
Labels
A-unsafe Area: Unsafety

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Sep 17, 2019

We'd need to incorporate the documentation of rust-lang/rust#64145 or cross-reference it at some point.

The issue is that #[target_feature]s are part of a function ABI, so two functions with different target-features do not necessarily have the same ABI. For example:

#[repr(simd)] F64x4([f64; 4]);
#[target_feature(enable = "sse")] fn foo(F64x4);
#[target_feature(enable = "avx")] fn bar(F64x4);

The ABI of foo is #[target_feature(enable = "sse")] extern "Rust while the ABI of bar is #[target_feature(enable = "avx")] extern "Rust", and these two ABIs are incompatible, foo expects its F64x4 argument in two 128-bit wide registers, while bar expects its argument in one 256-bit wide register.

That is, code like this:

// crate A:
#[target_feature(enable = "avx")] fn bar(F64x4) { ... }

// crate B:
extern "Rust" {
    #[target_feature(enable = "sse")] fn bar(F64x4);
}
bar(...);

might exhibit undefined behavior of the form "wrong call ABI" (we kind of work around this being UB by passing SIMD vectors behind a pointer all the time).

We should document this. In particular, libstd is compiled with the base features for the target, e.g., on x86_64, that's sse2, etc. but external crates can be compiled with different target features, e.g. using -C target-feature=-sse (removing SSE).

When that happens and a crate calls a libstd function, it does so through an extern "Rust" declaration without the SSE, but the libstd definition has SSE. That is, that's undefined behavior of this particular form, and will break all code using floats on ABIs because when SSE is available floats are passed in SSE registers, but when they are disabled they are passed in x87 FPU registers.

So we should not only document that target features can be used to exhibit this particular form of undefined behavior, but that the -C target-featureCLI option makes it trivial to do so, and that we have no checks nor general workarounds for this.

@crlf0710
Copy link
Member

Why don't we just record all the active target-features in crate metadata and compare them during the compilation?

@ehuss ehuss added the A-unsafe Area: Unsafety label Apr 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-unsafe Area: Unsafety
Projects
None yet
Development

No branches or pull requests

3 participants