`#[target_feature]` mismatch on unsafe trait fn vs its impl causes sneaky UB

[obi1kenobi]

I tried this code: (playground)

// This trait is unsealed. Imagine it's in crate A.
pub trait Example {
    unsafe fn demo(&self) {}
}

// Imagine this type is in crate B,
// which depends on A.
pub struct S;

impl Example for S {
    // This isn't applied to the trait's fn!
    #[target_feature(enable = "avx2,aes")]
    unsafe fn demo(&self) {}
}

// Imagine this function is in crate C,
// which also depends on A but might or might not be used
// together with B.
//
// It seems to be impossible to write the safety comment below.
pub fn accept_dyn(value: &dyn Example) {
    // SAFETY: umm ???
    // Who knows what #[target_feature]
    // attributes the trait impl has imposed?!
    unsafe { value.demo() }
}

The "imagine this type is in crate X" is not strictly necessary — this is still a problem within a single crate too. It's just much easier to spot the problem when the code is all together instead of in 3 different and separately evolving crates.

I expected to see this happen: to soundly use unsafe items from a trait, reading the safety comments on the trait and its items should generally be sufficient. A safe (non-unsafe) attribute shouldn't be able to be used in a manner that causes hard-to-find unsoundness and UB. This code should either be rejected outright for tightening the trait's safety requirements in the impl, or at least require #[unsafe(target_feature)] plus raise a lint for the requirements-tightening.

Instead, this happened: this code compiles fine with no warning. The attribute is not unsafe. Even an unintentional "editing error" is likely to silently cause impls' requirements to diverge from the trait's requirements and lead to unsoundness.

Meta

rustc --version --verbose:

1.88.0-nightly (2025-04-02 d5b4c2e4f19b6d703737)

From the playground.

[hanna-kruppe]

I agree this can be a footgun: the safety precondition implied by the #[target_feature(enable=...)] may be missed because it didn't occur to the author when they added the unsafe. I don't think this is a language soundness issue in the technical sense: ultimately there's no UB unless a caller writes unsafe {} to call the method, and they can do so soundly if they know the safety conditions. It's just easier than it should be for them to not know those conditions, and that's worth addressing to make it easier to write sound unsafe code.

Unfortunately, it seems difficult to automatically distinguish mistakes from sound usage:

• The safety contract of the method may, in fact, already require the caller to ensure the target features required by the respective impl are available. memchr's internal Vector trait kinda does this for abstracting over different SIMD instruction sets. In that case the impls don't actually add #[target_feature] themselves, but the trait still requires callers to ensure the target features of the relevant impl are enabled. (This trait can afford to be vague about which target features those are because it's for internal use only and only used via static dispatch, so the rest of the library can actually ensure this.)
• The impl in question may, in fact, discharge the safety precondition itself, e.g., by ensuring values of the Self type are only constructed if the target features are available. It's a bit unfortunate that this requires marking the trait method as unsafe (possibly with a safety precondition of "this is always safe to call, ignore rustc"). However, the trait method is the optimal place to introduce the target feature in the case of dynamic dispatch. You could write the method body as unsafe { the_actual_impl(self, arg1, arg2, ...) } where only the_actual_impl has the target feature, but then that call can't be inlined due to mismatched target features, so anyone calling the method through the vtable would unnecessarily do two calls (first an indirect one to the method, then another one to the_actual_impl).

Making the attribute unsafe is not quite right: ultimately UB only comes from a call to the method, which requires the caller to write an unsafe block. This is different from existing unsafe attributes, which can introduce UB just by applying them to an item, even if the item is never used at runtime. In contrast, #[target_feature] is only another way to add a precondition for an unsafe function, not by itself unsafe to invoke. This is similar to how an unsafe fn can lay out safety preconditions for callers without any unsafe blocks or attributes in the immediate vicinity. Vec::set_len is the canonical example: only unsafe-to-call because it's relevant for Vec's safety invariant, not because it does anything unsafe on its own. If and when unsafe fields are adopted, this could change. But unsafe fields are a tool to support writing unsafe code more confidently, they weren't needed to plug any soundness holes. I see this issue similarly.

In analogy to unsafe fields, I could see an argument for making #[target_feature] some kind of unsafe attribute. It doesn't need to be unsafe for soundness, but there's a trend in Rust's evolution towards more fine-grained annotations for both introducing and discharging safety-related proof obligations (unsafe fields, unsafe_op_in_unsafe_fn lint, unsafe extern blocks with a mix of safe and unsafe items, etc.). From this POV, it might make sense to call out #[target_feature] separately in the same way that unsafe fields are useful for keeping track of where safety invariants need to be upheld.

[obi1kenobi]

I agree with substantially everything you said.

I'm curious about your opinion on a possible rustc warn-by-default lint for when a trait impl introduces more target feature requirements than the pub trait being implemented.

[hanna-kruppe]

I think that would definitely be good idea as a clippy lint. The bar for rustc lints is much higher and it’s always possible to uplift a clippy lint into rustc later. This pattern doesn’t always indicate a mistake, and when it’s actually what you want I don’t think you can silence the lint by writing the code slightly differently, so this lint would probably end up allowed/expected in some places.

[oli-obk]

Maybe the attribute should be unsafe to apply to impl methods. rust-lang/rfcs#3715