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

_mm_movemask_epi8 unexpectedly triggers Undefined Behavior #1347

Closed
Nugine opened this issue Oct 26, 2022 · 7 comments · Fixed by #1354
Closed

_mm_movemask_epi8 unexpectedly triggers Undefined Behavior #1347

Nugine opened this issue Oct 26, 2022 · 7 comments · Fixed by #1354

Comments

@Nugine
Copy link
Contributor

Nugine commented Oct 26, 2022

Moved from rust-lang/miri#2617


playground

use core::arch::x86_64::*;

#[target_feature(enable = "sse2")]
pub unsafe fn is_ascii_sse2(a: &[u8; 16]) -> bool {
    let a = _mm_loadu_si128(a.as_ptr().cast());
    let m = _mm_movemask_epi8(a);
    m == 0
}

fn main() {
    assert!(cfg!(target_feature = "sse2"));
    let s = b"helloworld123456";
    assert!(unsafe { is_ascii_sse2(s) });
}
error: Undefined Behavior: each element of a SIMD mask must be all-0-bits or all-1-bits
    --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/x86/sse2.rs:1381:5
     |
1381 |     simd_bitmask::<_, u16>(a.as_i8x16()) as u32 as i32
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ each element of a SIMD mask must be all-0-bits or all-1-bits
     |
     = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
     = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
     = note: BACKTRACE:
     = note: inside `std::arch::x86_64::_mm_movemask_epi8` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/x86/sse2.rs:1381:5
note: inside `is_ascii_sse2` at src/main.rs:6:13

See also:


Related:


_mm[256]_movemask_epi8 has defined behavior:

Returns a mask of the most significant bit of each element in a.

(regardless of the other bits)

Changing the requirement may introduce unsoundness into existing crates.

@Amanieu
Copy link
Member

Amanieu commented Oct 26, 2022

I commented on the miri issue, I think that one should be reopened.

@yanns
Copy link

yanns commented Dec 15, 2022

Sorry for the naive question, but I still have this issue on nightly:

MIRIFLAGS="-Zmiri-disable-isolation -Zmiri-measureme=crox" cargo +nightly miri run
Preparing a sysroot for Miri (target: x86_64-apple-darwin)... done
    Finished dev [unoptimized + debuginfo] target(s) in 0.48s
     Running `/Users/yannsimon/.rustup/toolchains/nightly-x86_64-apple-darwin/bin/cargo-miri runner target/miri/x86_64-apple-darwin/debug/proto_refexp`
error: Undefined Behavior: each element of a SIMD mask must be all-0-bits or all-1-bits
    --> /Users/yannsimon/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/x86/avx2.rs:2004:5
     |
2004 |     simd_bitmask::<_, u32>(a.as_i8x32()) as i32
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ each element of a SIMD mask must be all-0-bits or all-1-bits

from:

pub unsafe fn _mm256_movemask_epi8(a: __m256i) -> i32 {
    simd_bitmask::<_, u32>(a.as_i8x32()) as i32
}

although this has been changed by #1354

Do we need some kind of release to use the new code?

@Amanieu
Copy link
Member

Amanieu commented Dec 15, 2022

The stdarch submodule in rust-lang/rust needs to be updated.

@yanns
Copy link

yanns commented Dec 15, 2022

Is it something I can do? Locally? Or it has to be done by the rust-lang?

@Amanieu
Copy link
Member

Amanieu commented Dec 15, 2022

You can submit a pull request to rust-lang/rust that updates the submodule.

@yanns
Copy link

yanns commented Dec 16, 2022

rust-lang/rust#105784

@yanns
Copy link

yanns commented Dec 16, 2022

I don't know if you can help in the rust PR.
It seems that there is a new feature tag:

error: the feature named `gfni` is not valid for this target
  --> library/core/src/../../stdarch/crates/core_arch/src/x86/gfni.rs:68:18
   |
68 | #[target_feature(enable = "gfni,avx512bw,avx512f")]
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `gfni` is not valid for this target

yanns added a commit to yanns/rust that referenced this issue Dec 29, 2022
This will allow using miri on simd instructions
rust-lang/stdarch#1347 (comment)
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 27, 2023
update stdarch

This will allow using miri on simd instructions
rust-lang/stdarch#1347 (comment)
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 27, 2023
update stdarch

This will allow using miri on simd instructions
rust-lang/stdarch#1347 (comment)
thomcc pushed a commit to tcdi/postgrestd that referenced this issue May 31, 2023
This will allow using miri on simd instructions
rust-lang/stdarch#1347 (comment)
thomcc pushed a commit to tcdi/postgrestd that referenced this issue May 31, 2023
update stdarch

This will allow using miri on simd instructions
rust-lang/stdarch#1347 (comment)
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

Successfully merging a pull request may close this issue.

3 participants