Skip to content

-C target-feature/-C target-cpu are unsound #64609

Open

Description

In a nutshell, target-features are part of the call ABI, but Rust does not take that into account, and that's the underlying issue causing #63466, #53346, and probably others (feel free to refer them here).

For example, for an x86 target without SSE linking these two crates shows the issue:

// crate A
pub fn foo(x: f32) -> f32 { x }

// crate B
extern "Rust" {
     #[target_feature(enable = "sse")]  fn foo(x: f32) -> f32;
}
unsafe { assert_eq!(foo(42.0), 42.0) }; // UB

The ABI of B::foo is "sse" "Rust" but the ABI defined in A::foo is just "Rust", no SSE, since the SSE feature is not enabled globally. That's an ABI mismatch and results in UB of the form "calling a function with an incompatible call ABI". For this particular case, B::foo expects the f32 in an xmm register, while A::foo expects it in an x87 register, so the roundtrip of 42.0 via foo will return garbage.

Now, this example is not unsound, because it requires an incompatible function declaration, and unsafe code to call it - and arguably, the unsafe asserts that the declaration is, at least correct (note: we forbid assigning #[target_feature] functions to function pointers and only allow enabling features and using white-listed features because of this).

However, you can cause the exact same issue, but on a larger scale, by using -C target-feature/-C target-cpu to change the features that a Rust crate assumes the "Rust" ABI has, without any unsafe code:

// crate A: compiled without -C target-feature
pub fn foo(x: f32) -> f32 { x }

// crate B: compiled with -C target-feature=+sse
// this is now safe Rust code:
assert_eq!(A::foo(42.0), 42.0) }; // UB

So that's the soundness bug. Safe Rust can exhibit undefined behavior of the form "calling a function with an incompatible call ABI".

This is an issue, because when RUSTFLAGS="-C target-cpu=native" is used, not all crates in the dependency graph are compiled with those features. In particular, libstd and its dependencies are not recompiled at all, so their "Rust" ABI might be different than what the rest of the dependency graph uses. -C target-feature also allows disabling features, -C target-feature/target-cpu allow enabling/disabling features that are not white-listed (e.g. avx512f if your CPU supports it can be enabled using -C target-feature and will be enabled using -C target-cpu=skylake or =nativeeven though the avx512f feature is unstable).

How important is fixing this ? I'd say moderately important, because many features are compatible. For example, the "sse2" "Rust" ABI has the same calling convention as the "sse3" "Rust", so even though the call ABIs aren't identical, they are compatible. That is, this bug is unlikely to cause issues in practice, unless you happen to end up with two crates where the difference in target features changes the ABI.

I think that rustc should definitely detect that the ABIs are incompatible, and at least, error at compile-time when this happen, explaining what went wrong, which target-features differed in an incompatible way, etc.

We could make such code work, e.g., by just treating target-features as part of the calling convention, and following the calling convention properly. I don't think fixing this would be useful for people doing -C target-feature globally, because when that happens, chances are that they wanted to compile their whole binary in a specific way, as opposed to having different parts of the binary somehow interoperate.

It would however be useful for improving how we currently handle SIMD vectors. For example, a vector types like f64x4 will be passed in different registers (2x 128-bit or 1x 256-bit) depending on whether sse or avx are available. We currently "solve" this problem by always passing vectors to functions indirectly by memory. That is, on the caller side we spill the vector registers into the stack, create a pointer to it, pass it to the callee, and then the callee loads the content into the appropriate vectors. Since target-features are not part of the calling convention, we always do this, as opposed to, only when the calling convention is incompatible. So having target-features be part of the calling convention might be a way to improve that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    A-ABIArea: Concerning the application binary interface (ABI)Area: Concerning the application binary interface (ABI)A-codegenArea: Code generationArea: Code generationA-target-featureArea: Enabling/disabling target features like AVX, Neon, etc.Area: Enabling/disabling target features like AVX, Neon, etc.C-bugCategory: This is a bug.Category: This is a bug.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-mediumMedium priorityMedium priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions