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

Report UB instead of ICEing when SIMD operands have incompatible size #3076

Closed
saethlin opened this issue Sep 23, 2023 · 6 comments
Closed

Comments

@saethlin
Copy link
Member

In a lot of our SIMD intrinsics, we assert that operands which must be equal size are equal size. Such a situation can arise because of an implementation mistake which would justify an ICE, but they can also arise if someone tries to declare their own LLVM intrinsics and gets it wrong. For example:

#![feature(link_llvm_intrinsics, simd_ffi)]
#[allow(improper_ctypes)]
use std::arch::x86_64::*;

extern "C" {
    #[link_name = "llvm.x86.sse2.sqrt.pd"]
    fn oof(x: __m128d) -> __m256d;
}

fn main() {
    unsafe {
        oof(_mm_set_pd(0.0, 0.0));
    };
}

This should report UB instead of ICEing.

@RalfJung
Copy link
Member

We generally ICE when intrinsics are used the wrong way. I don't think we should make an exception for SIMD intrinsics.

link_llvm_intrinsics sounds like a feature on par with feature(intrinsics): if you use it wrong, don't expect the compiler to give nice error messages.

@saethlin
Copy link
Member Author

Hm. My concern is that we attach this text to an ICE:

error: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/miri/issues/new

Which is not appropriate if the ICE is actually user error.

@RalfJung
Copy link
Member

Yeah, that text is wrong when intrinsics are involved (and possibly with other internal features as well), see e.g. this example.

I'm fairly sure it's also possible to ICE rustc by misusing intrinsics, so this is not really a Miri issue.

@RalfJung
Copy link
Member

Here's a rustc ICE that's not-a-bug due to being related to intrinsics.

We should clarify the message printed on an ICE (but getting access to enough state to figure out if any internal feature was enabled might be tricky). But that's a rustc issue, not a Miri issue.

@RalfJung
Copy link
Member

This now prints

warning: the feature `link_llvm_intrinsics` is internal to the compiler or standard library
 --> src/main.rs:1:12
  |
1 | #![feature(link_llvm_intrinsics, simd_ffi)]
  |            ^^^^^^^^^^^^^^^^^^^^
  |
  = note: using it is strongly discouraged
  = note: using it is strongly discouraged
  = note: `#[warn(internal_features)]` on by default

FWIW with a regular rustc build it doesn't even ICE, it hits an LLVM assertion. So I don't think there is anything else we want to do in the Miri side here.

rust-lang/rust#116217 tracks the ICE message.

@saethlin
Copy link
Member Author

Thanks! I agree that making this an internal feature is a good improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants