Skip to content

std::arch::x86::has_cpuid() alternates between true and false when CPUID is supported #51691

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

Closed
stevecheckoway opened this issue Jun 21, 2018 · 4 comments

Comments

@stevecheckoway
Copy link

The code in question is here. The bug is on line 99 where the OR should be XOR.

This code demonstrates the bug.

#![feature(stdsimd)]
use std::arch::x86;

fn main() {
    assert_eq!(x86::has_cpuid(), x86::has_cpuid());
}

I expected to see this happen: x86::has_cpuid() should be idempotent and thus the assertion should be satisfied.

Instead, this happened:

$ rustc --target i686-apple-darwin bug.rs
$ ./bug
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `true`,
 right: `false`', bug.rs:5:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

Meta

rustc 1.28.0-nightly (f28c7aef7 2018-06-19)
binary: rustc
commit-hash: f28c7aef7fbff1909c2d0257186cd7a5f0c6aa4b
commit-date: 2018-06-19
host: x86_64-apple-darwin
release: 1.28.0-nightly
LLVM version: 6.0
@stevecheckoway
Copy link
Author

Come to think of it, it's not at all clear that this test can be safely implemented in Rust for exactly the reason given at the link in the comment.

Implementing this routine in for example C can lead to issues, because the compiler may change EFLAGS at any time.

I don't know that LLVM has any guarantees about the values of eflags. The should be easy to write in inline assembly though.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Jun 21, 2018

Oh yeah, fiddling with any (non-reserved) register like that is completely broken. This really should be an inline asm sequence, and honestly these intrinsics (edit: __{read,write}eflags) seem impossible to use correctly for anything.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 23, 2018

rust-lang/stdarch#492 should fix this.

@stevecheckoway
Copy link
Author

Looks good modulo the comment I left about test_has_cpuid being broken.

@est31 est31 mentioned this issue Jun 23, 2018
gnzlbg added a commit to rust-lang/stdarch that referenced this issue Jun 25, 2018
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 25, 2018
TheIronBorn pushed a commit to TheIronBorn/stdsimd that referenced this issue Jun 29, 2018
TheIronBorn pushed a commit to TheIronBorn/stdsimd that referenced this issue Jun 29, 2018
bors added a commit that referenced this issue Jul 3, 2018
lu-zero pushed a commit to lu-zero/stdarch that referenced this issue Jul 10, 2018
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

3 participants