Detect UB due to target feature caller/callee mismatch

[RalfJung]

@chorman0773 points out that one can cause ABI incompatibility UB by having a mismatch between the target features in the caller and callee. We should come up with a test case for this and then ensure that Miri can detect that UB.

[chorman0773]

I have a demo of the UB. Note that it requires 2 crates (or perhaps crate+some name mangling magic) to demonstrate: https://godbolt.org/z/3j4YhnzK5

[RalfJung]

Can't it be done in a single crate with the per-function target_feature attribute?

[chorman0773]

Can't it be done in a single crate with the per-function target_feature attribute?

It can indeed. https://godbolt.org/z/Tj3Gj43sq

I thought llvm was even weirder with function-specific #[target_feature], but I guess that's just clang.

[RalfJung]

I tried turning this into a Miri test case:

use std::arch::x86_64::*;
use std::mem::transmute;

#[no_mangle]
#[target_feature(enable="avx")]
pub unsafe extern "C" fn foo(_y: f32, x: __m256) -> __m256{
    x
}

pub fn bar(x: __m256) -> __m256 {
    // The first and second argument get mixed up here since caller
    // and callee do not have the same feature flags.
    unsafe{foo(0.0,x)}
}

fn assert_eq_m256(a: __m256, b: __m256) {
    unsafe {
        assert_eq!(transmute::<_, [f32; 8]>(a), transmute::<_, [f32; 8]>(b))
    }
}

fn main() {
    let input = unsafe { _mm256_set1_ps(1.0) };
    let copy = bar(input);
    assert_eq_m256(input, copy);
}

However, this already gets rejected:

error: Undefined Behavior: calling a function that requires unavailable target features: avx
  --> src/main.rs:23:26
   |
23 |     let input = unsafe { _mm256_set1_ps(1.0) };
   |                          ^^^^^^^^^^^^^^^^^^^ calling a function that requires unavailable target features: avx
   |

Does that mean we are good, or are there other ways to trigger this ABI mismatch?

[chorman0773]

You can construct it using an explicit #[target_feature] attribute, or core::mem::transmute.

Also, I'd probably compare [u32;8] or [u64;4] (since f32 bit patterns can be interpreted as a NaN).

[RalfJung]

What do you mean by constructing it with a transmute?

[chorman0773]

Unless the issue is #[target_feature(enable="avx")] in general, you can make a __m256 by transmuting an [f32;8] or [u32;8]. If the issue is that miri can't distinguish between compile-time enabled features and runtime enabled features, that may be an issue that needs to be fixed.

[RalfJung]

I am completely lost. Can you show some code that somehow uses transmute to cause an issue here?

Miri dynamically considers exactly those features to be available that are enabled via -C target-feature.

[chorman0773]

Ok yeah, you'd need to have a way to separate statically enabled and dynamically enabled features to check this particular UB.

[RalfJung]

Okay so I think Miri is good here for UB detection for now, but eventually it could be nice to support -Zmiri-target-feature to enable a feature dynamically (but not statically) and then this will become a concern again.

[chorman0773]

I'm mostly worried about code that will check is_<arch>_feature_enabled!() then cross the divide with #[target_feature(enable)]

[RalfJung]

Yeah, I understand. In Miri feature_enabled will always return false for non-statically-available features so there's no UB we are currently missing here. But ideally we'd support setting the set of dynamically available features.