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

Miri test fails because of neon intrinsic in aho_corasick #138

Open
domenukk opened this issue Dec 26, 2023 · 12 comments
Open

Miri test fails because of neon intrinsic in aho_corasick #138

domenukk opened this issue Dec 26, 2023 · 12 comments

Comments

@domenukk
Copy link

We are using the Regex crate in LibAFL and when running a miri test on Mac M1, the tests fail due to a neon intrinsic. They work fine on Linux/x86.
See AFLplusplus/LibAFL#1762

test inputs::encoded::tests::test_input ... error: unsupported operation: can't call foreign function `llvm.aarch64.neon.tbl1.v16i8` on OS `macos`
    --> /Users/dmnk/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/aarch64/neon/mod.rs:2438:15
     |
2438 |     transmute(vqtbl1q(transmute(t), transmute(idx)))
     |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't call foreign function `llvm.aarch64.neon.tbl1.v16i8` on OS `macos`
     |
     = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
     = note: BACKTRACE:
     = note: inside `core::arch::aarch64::vqtbl1q_u8` at /Users/dmnk/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/aarch64/neon/mod.rs:2438:15: 2438:52
     = note: inside `aho_corasick::packed::vector::aarch64_neon::<impl aho_corasick::packed::vector::Vector for core::arch::aarch64::uint8x16_t>::shuffle_bytes` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aho-corasick-1.1.2/src/packed/vector.rs:665:13: 665:38
     = note: inside `aho_corasick::packed::teddy::generic::Mask::<core::arch::aarch64::uint8x16_t>::members2` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aho-corasick-1.1.2/src/packed/teddy/generic.rs:1071:23: 1071:53
     = note: inside `aho_corasick::packed::teddy::generic::Slim::<core::arch::aarch64::uint8x16_t, 2>::candidate` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aho-corasick-1.1.2/src/packed/teddy/generic.rs:230:28: 230:61
     = note: inside `aho_corasick::packed::teddy::generic::Slim::<core::arch::aarch64::uint8x16_t, 2>::find_one` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aho-corasick-1.1.2/src/packed/teddy/generic.rs:217:17: 217:43
     = note: inside `aho_corasick::packed::teddy::generic::Slim::<core::arch::aarch64::uint8x16_t, 2>::find` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aho-corasick-1.1.2/src/packed/teddy/generic.rs:194:30: 194:65
     = note: inside `<aho_corasick::packed::teddy::builder::aarch64::SlimNeon<2> as aho_corasick::packed::teddy::builder::SearcherT>::find` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aho-corasick-1.1.2/src/packed/teddy/builder.rs:770:21: 770:50
     = note: inside `aho_corasick::packed::teddy::builder::Searcher::find` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aho-corasick-1.1.2/src/packed/teddy/builder.rs:355:13: 355:70
     = note: inside `aho_corasick::packed::api::Searcher::find_in::<&[u8]>` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aho-corasick-1.1.2/src/packed/api.rs:540:17: 540:62
     = note: inside `<regex_automata::util::prefilter::teddy::Teddy as regex_automata::util::prefilter::PrefilterI>::find` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/util/prefilter/teddy.rs:93:13: 94:44
     = note: inside `<std::sync::Arc<dyn regex_automata::util::prefilter::PrefilterI> as regex_automata::util::prefilter::PrefilterI>::find` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/util/prefilter/mod.rs:481:9: 481:39
     = note: inside `regex_automata::util::prefilter::Prefilter::find` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/util/prefilter/mod.rs:347:13: 347:42
     = note: inside `regex_automata::hybrid::search::find_fwd_imp` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/hybrid/search.rs:74:15: 74:47
     = note: inside `regex_automata::hybrid::search::find_fwd` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/hybrid/search.rs:38:13: 38:56
     = note: inside `regex_automata::hybrid::dfa::DFA::try_search_fwd` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/hybrid/dfa.rs:595:24: 595:60
     = note: inside `regex_automata::hybrid::regex::Regex::try_search` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/hybrid/regex.rs:448:25: 448:69
     = note: inside `regex_automata::meta::wrappers::HybridEngine::try_search` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/meta/wrappers.rs:649:13: 649:44
     = note: inside `<regex_automata::meta::strategy::Core as regex_automata::meta::strategy::Strategy>::search` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/meta/strategy.rs:720:19: 720:57
     = note: inside `regex_automata::meta::regex::Regex::search_with` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/meta/regex.rs:1248:9: 1248:44
     = note: inside closure at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/meta/regex.rs:2079:31: 2079:59
     = note: inside `regex_automata::util::iter::Searcher::<'_>::try_advance::<{closure@<regex_automata::meta::regex::FindMatches<'_, '_> as core::iter::Iterator>::next::{closure#0}}>` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/util/iter.rs:431:27: 431:46
     = note: inside `regex_automata::util::iter::Searcher::<'_>::advance::<{closure@<regex_automata::meta::regex::FindMatches<'_, '_> as core::iter::Iterator>::next::{closure#0}}>` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/util/iter.rs:380:15: 380:39
     = note: inside `<regex_automata::meta::regex::FindMatches<'_, '_> as core::iter::Iterator>::next` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-automata-0.4.3/src/meta/regex.rs:2079:9: 2079:61
     = note: inside `<regex::Matches<'_, '_> as core::iter::Iterator>::next` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-1.10.2/src/regex/string.rs:2160:9: 2161:20
     = note: inside `<core::iter::Enumerate<regex::Matches<'_, '_>> as core::iter::Iterator>::next` at /Users/dmnk/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/iter/adapters/enumerate.rs:47:17: 47:33
     = note: inside closure at /Users/dmnk/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/iter/adapters/peekable.rs:217:43: 217:54
     = note: inside `core::option::Option::<core::option::Option<(usize, regex::Match<'_>)>>::get_or_insert_with::<{closure@core::iter::Peekable<core::iter::Enumerate<regex::Matches<'_, '_>>>::peek::{closure#0}}>` at /Users/dmnk/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/option.rs:1658:26: 1658:29
     = note: inside `core::iter::Peekable::<core::iter::Enumerate<regex::Matches<'_, '_>>>::peek` at /Users/dmnk/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/iter/adapters/peekable.rs:217:9: 217:55
     = note: inside `regex::Regex::replacen::<&str>` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-1.10.2/src/regex/string.rs:907:16: 907:25
     = note: inside `regex::Regex::replace_all::<&str>` at /Users/dmnk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/regex-1.10.2/src/regex/string.rs:837:9: 837:40
note: inside `<inputs::encoded::NaiveTokenizer as inputs::encoded::Tokenizer>::tokenize`
    --> libafl/src/inputs/encoded.rs:156:22
     |
156  |         let string = self.comment_re.replace_all(string, "").to_string();
     |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `<inputs::encoded::TokenInputEncoderDecoder as inputs::encoded::InputEncoder<inputs::encoded::NaiveTokenizer>>::encode`
    --> libafl/src/inputs/encoded.rs:64:22
     |
64   |         let tokens = tokenizer.tokenize(bytes)?;
     |                      ^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `inputs::encoded::tests::test_input`
    --> libafl/src/inputs/encoded.rs:277:21
     |
277  |           let input = ed
     |  _____________________^
278  | |             .encode("/* test */a = 'pippo baudo'; b=c+a\n".as_bytes(), &mut t)
     | |______________________________________________________________________________^
note: inside closure
    --> libafl/src/inputs/encoded.rs:274:20
     |
273  |     #[test]
     |     ------- in this procedural macro expansion
274  |     fn test_input() {
     |                    ^
     = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)
@BurntSushi
Copy link
Owner

This seems like a bug report (or feature request) for Miri, not this crate.

@BurntSushi BurntSushi closed this as not planned Won't fix, can't repro, duplicate, stale Dec 26, 2023
@domenukk
Copy link
Author

Well we could have non-optimized functions that would work on aarch64 miri, and also other architectures, but I can try over at Miri instead

@BurntSushi
Copy link
Owner

BurntSushi commented Dec 27, 2023

Those already exist. The tests that are failing are specifically testing the optimized paths.

Miri tests don't succeed on x86 either.

Fixing this on my end probably means going through and adding a bunch of miri conditional compilation things to skip certain tests. I do that on some crates (like memchr and regex-automata), but it's pretty annoying to do. It also somewhat defeats the purpose, because the optimized paths are exactly where you want miri the most.

@saethlin
Copy link

Miri tests don't succeed on x86 either.

Are you sure about that? Miri has decent x86_64 shims as of rather recently; with default flags the searcher fails to build (I didn't look into why yet, probably a runtime CPU feature detection fails) but this seems to work:

RUSTFLAGS=-Ctarget-cpu=x86-64-v2 cargo +nightly miri nextest run -j32

@BurntSushi
Copy link
Owner

with default flags the searcher fails to build

That's what I tried. I didn't dig into it.

@BurntSushi BurntSushi reopened this Dec 27, 2023
@BurntSushi
Copy link
Owner

I'll reopen this for now since it looks like some further investigation might be warranted.

With that said, the original error message for aarch64 is pretty clear. In that case, miri likely needs support for neon.

@saethlin
Copy link

I agree with that completely. Miri doesn't have much if any neon support and would need a lot for this use case.

@saethlin
Copy link

saethlin commented Dec 27, 2023

Ah; the reason tests don't work on Miri + x86 by default is that aho-corasick tries to do runtime CPU feature detection (in teddy) which in Miri doesn't get you the properties of the host, it gets you the properties of the configured target-cpu, and the Rust x86_64 baseline according to rustc --print cfg -Ctarget-cpu=x86-64 only has sse and sse2.

Up to you if you think there's something more reasonable to do in this case I guess? I don't know if there's something more reasonable that Miri could do here; using anything other than the target baseline by default seems dubious for a cross-interpreter.

@BurntSushi
Copy link
Owner

I don't know if there's something more reasonable that Miri could do here; using anything other than the target baseline by default seems dubious for a cross-interpreter.

Would it be possible to spell this out in more detail? I totally trust what you're saying here, I just don't completely grok it. I think my underlying question is, why can't Miri let runtime CPU feature detection actually inspect the CPU features of the host? Like, what is the blocker? (I say this with curiosity and ignorance, not with combativeness.)

It is great that Miri can run with RUSTFLAGS=-Ctarget-cpu=x86-64-v2 though. That's a useful tip, thank you.

@BurntSushi
Copy link
Owner

I think one of the issues I ran into here is that I didn't want to just silently skip the tests if building the searcher failed. I wanted that to be loud. Otherwise, it's too easy for a test that should fail to just silently pass. I don't think I dug in too much at this point and kind of just ran out of steam. And I don't think I knew the tidbit about Miri not doing runtime CPU feature detection, so was perhaps confused. But now that I know that, I think I can rejigger the tests a bit to make this work more nicely.

@saethlin
Copy link

I think my underlying question is, why can't Miri let runtime CPU feature detection actually inspect the CPU features of the host? Like, what is the blocker?

Miri tries rather hard to be a portable cross-interpreter. I can run Miri on an M1 and ask it for --target=s390x-unknown-linux-gnu. Without passing MIRIFLAGS=-Zmiri-disable-isolation, every aspect of the program behavior should only depend on the target, not the host (and also be exactly the same run-to-run).

With isolation disabled is a slightly more interesting question; it might make sense to default to "whatever the host has", but then it's not entirely clear what to do when cross-interpreting because the CPU feature detection in stdarch depends on architecture and OS. We couldn't just have Miri directly run the standard library code for is_X_feature_detected because everything gets cross-compiled for the target so that code can only possibly work if the target matches the host, including the OS. To at least solve the OS dimension, we could execute the feature detection code in the interpreter itself and return it from the macro by a shim. (and of course you can't pass through to the host for interpreting across architectures, and I wouldn't be surprised if we have targets that are the same architecture with different baseline CPU feature sets)

But then we have a UI problem where we need to figure out what people should do to give Miri access to the filesystem but not to runtime feature detection, so that we can have reproducible errors in programs that need to read a file but also need to do some SIMD. A lot of people globally disable isolation because so few real programs work with it, so it's really not clear what direction an "isolation includes CPU feature detection" flag should go.

I don't think there are any hard blockers, but passing through to the host is a sticky mess for a cross-interpreter.

@BurntSushi
Copy link
Owner

Gotya, thank you for explaining that. That makes sense! I see the problem now.

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