Generates illegal instructions

[baskerville]

I get a systematic crash when I run cargo-edit 0.3.0 on an Intel Core 2 Duo E7600 CPU.

The lldb backtrace seems to indicate that SIMD is involved:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
  * frame #0: 0x00000001002e8db6 cargo-add`core::coresimd::x86::xsave::_xgetbv::h7da7479f591ed348 at xsave.rs:95 [opt]
    frame #1: 0x00000001002eaaeb cargo-add`std::stdsimd::arch::detect::os::detect_features::hcd1961e6f91cf102 at x86.rs:157 [opt]
    frame #2: 0x00000001002ea94a cargo-add`std::stdsimd::arch::detect::os::check_for::h494500e4a19a5d3c [inlined] core::ops::function::FnOnce::call_once::h731b82db5225b805 at function.rs:223 [opt]
    frame #3: 0x00000001002ea945 cargo-add`std::stdsimd::arch::detect::os::check_for::h494500e4a19a5d3c [inlined] std::stdsimd::arch::detect::cache::test::h9f743f59f6a54bc9 at cache.rs:149 [opt]
    frame #4: 0x00000001002ea938 cargo-add`std::stdsimd::arch::detect::os::check_for::h494500e4a19a5d3c at x86.rs:16 [opt]
    frame #5: 0x000000010027b854 cargo-add`regex::literal::Matcher::new::h9088e543805253af + 324
    frame #6: 0x000000010027a931 cargo-add`regex::literal::LiteralSearcher::prefixes::h2920ca55b2b79fe1 + 641
    frame #7: 0x0000000100269e5e cargo-add`regex::exec::ExecBuilder::build::h72257c20c98ecf31 + 6510
    frame #8: 0x0000000100266762 cargo-add`regex::re_builder::unicode::RegexBuilder::build::h12d55fa7c52e206b + 210
    frame #9: 0x000000010029ad4b cargo-add`regex::re_unicode::Regex::new::h134009fc29889184 + 219
    frame #10: 0x00000001002470cc cargo-add`std::sync::once::Once::call_once::_$u7b$$u7b$closure$u7d$$u7d$::h17d5ef8a5bc6025f + 60
    frame #11: 0x00000001002f3a2e cargo-add`std::sync::once::Once::call_inner::h897ebf125b2dcf3e at once.rs:340 [opt]
    frame #12: 0x0000000100234f35 cargo-add`docopt::parse::Parser::new::h29e507fb9dde54d3 + 565
    frame #13: 0x000000010000561c cargo-add`cargo_add::main::h12221ffe12ffa004 + 44
    frame #14: 0x0000000100002dd6 cargo-add`std::rt::lang_start::_$u7b$$u7b$closure$u7d$$u7d$::hc5b18510be8e533e + 6
    frame #15: 0x00000001002f6b28 cargo-add`std::panicking::try::do_call::h780530fa434ed2ed [inlined] std::rt::lang_start_internal::_$u7b$$u7b$closure$u7d$$u7d$::hcfde3e1c19e6fa13 at rt.rs:59 [opt]
    frame #16: 0x00000001002f6b1c cargo-add`std::panicking::try::do_call::h780530fa434ed2ed at panicking.rs:310 [opt]
    frame #17: 0x00000001003028ff cargo-add`__rust_maybe_catch_panic at lib.rs:105 [opt]
    frame #18: 0x00000001002ea7a2 cargo-add`std::rt::lang_start_internal::h46f9f692ece7505b [inlined] std::panicking::try::h0c5a2273ff0725da at panicking.rs:289 [opt]
    frame #19: 0x00000001002ea76f cargo-add`std::rt::lang_start_internal::h46f9f692ece7505b [inlined] std::panic::catch_unwind::h3474c6e3bb49278a at panic.rs:374 [opt]
    frame #20: 0x00000001002ea76f cargo-add`std::rt::lang_start_internal::h46f9f692ece7505b at rt.rs:58 [opt]
    frame #21: 0x00000001000062fc cargo-add`main + 44
    frame #22: 0x00007fff52586015 libdyld.dylib`start + 1

I'm also going to ping @BurntSushi as advised in the related cargo-edit issue.

I remember that I once got the same crash while playing with rustc's optimisation flags to build one of my projects.

[gnzlbg]

Thanks for reporting this!

Do you have instructions to reproduce? Ideally, starting with installing cargo-edit down to the error? (it's ok if you need to download a particular project to trigger this, just add git clone ... to the steps).

---

From a first look _xgetbv is causing this when called here. Note that _xgetbv requires xsave target feature, but it is only called after we have tested that the feature is available here.

To test for xsave support we need to query cpuid with eax=1 and read the 26-th bit of ecx, which is exactly what that code does so to me it appears that this part is at least correct.

The 27-th bit of ECX is osxsave which denotes whether the operating system has enabled xsave support and is tested right afterwards.

But... we unconditionally call _xgetbv independently of whether the OS has enabled osxsave or not... IIRC this was ok, but maybe it isn't? Worth triple-checking it.

cc @stevecheckoway @parched @petrochenkov

[gnzlbg]

https://software.intel.com/en-us/articles/introduction-to-intel-advanced-vector-extensions states:

Verify that the operating system supports XGETBV using CPUID.1:ECX.OSXSAVE bit 27 = 1.

So it seems that we can't call xgetbv if osxsave is disabled. I'll prepare a fix.

[baskerville]

Reproducing the crash is straightforward:

cargo install cargo-edit
cargo add --help

[boozook]

Same problem on macOS 10.12.6 (16G1036) on the CPU 2.13 GHz Intel Core 2 Duo.

Any call of is_x86_feature_detected method crashes with illegal instruction: 4.

Affected rustc versions:

• rustc 1.27.0 (3eda71b00 2018-06-19)
• rustc 1.28.0-nightly (60efbdead 2018-06-23)
• rustc 1.28.0-nightly (e3bf634e0 2018-06-28) - current

screencast.gif

isolated test in playground

[alexcrichton]

@fzzr- I think the fix for this issue hasn't made its way into the nightly releases yet unfortunately

[boozook]

And not today unfortunately. 😩

[gnzlbg]

@alexcrichton do you think it is worth to expand the CI to cover this? We can choose which CPU qemu-user uses, so we could actually add two x86_64 build bots, one with a CPU that only supports SSE2, and one with a CPU that only supports SSE4.2, to catch this.

The ones on travis support AVX2 IIRC, so the normal build bots just catch that case.

[stevecheckoway]

I meant to look into this more closely but I’m on holiday. Isn’t there a cpuid bit to test for xgetbv separate from xsave?

From a quick google search, CPUID.(EAX=0DH,ECX=1):EAX.XG1[bit 2] = 1 vs CPUID.01H:ECX.XSAVE[bit 26] = 1. I guess xgetbv also requires xsave to be supported and enabled but it’s not clear that that is sufficient.

[gnzlbg]

@stevecheckoway this should be fixed on master, stdsimd on rustc has been updated, but the update just hasn't landed on nightly yet.

I meant to look into this more closely but I’m on holiday. Isn’t there a cpuid bit to test for xgetbv separate from xsave?

Before this issue we just checked for xsave support, and if the CPU supported it, we used xgetbv. IIRC this is what caused the SIGILL on systems where the CPU supported xsave, but the operating system did not set the osxsave bit.

We now check both xsave and osxsave before using xgetbv, that is, the CPU must support xsave, and the operating system must have enabled xsave support in user-space by setting the osxsave cpu-bit.

It is hard to tell whether this is sufficient. There are multiple sources of truth from Intel, none of them focusing on general run-time feature detection in user-space (e.g. for language run-times), but rather focused only on detecting avx/avx512 support, and IIRC I didn't found one explaining the procedure in a step-by-step fashion, but more like, "you need to check all these things in no particular order". FWIW we were checking for osxsave support for avx and avx512 later on in the run-time detection code, we just weren't doing it before using xgetbv, which was only guarded on xsave and not osxsave.