pascalkuthe / imara-diff

Reliably performant diffing
Apache License 2.0
106 stars 9 forks source link

Crashes in Rust Nightly #7

Closed jayay closed 1 year ago

jayay commented 1 year ago

Running the test with Miri using MIRIFLAGS="-Zmiri-disable-isolation" cargo +nightly miri test for this repo results in a SIGKILL.

running 5 tests
test tests::complex_diffs ... error: test failed, to rerun pass `--lib`

Caused by:
  process didn't exit successfully: `/usr/local/rustup/toolchains/nightly-aarch64-unknown-linux-gnu/bin/cargo-miri runner /data/target/miri/aarch64-unknown-linux-gnu/debug/deps/imara_diff-b916e6c16bd4f65d` (signal: 9, SIGKILL: kill)

This might have something to do with the issue reported in https://github.com/rust-lang/rust/issues/112171.

saethlin commented 1 year ago

That sounds like Miri is just running your system out of memory. If Miri were encountering a problem, you'd get a clear diagnostic.

Also, the issue linked is about disabling a MIR optimization, and Miri disables all optimizations by default (and emits a warning if you turn them back on).

I tried compiling this repo but I get this error:

error[E0282]: type annotations needed
   --> /home/ben/.cargo/registry/src/index.crates.io-6f17d22bba15001f/git-glob-0.4.2/src/wildmatch.rs:251:62
    |
251 | ...                   let class = &pattern.as_ref()[p_idx + 2..closing_bracket_idx - 1];
    |                                            ^^^^^^
    |
help: try using a fully qualified path to specify the expected types
    |
251 |                                         let class = &<bstr::BStr as std::convert::AsRef<T>>::as_ref(pattern)[p_idx + 2..closing_bracket_idx - 1];
    |                                                      +++++++++++++++++++++++++++++++++++++++++++++++       ~

Which supposedly can be hacked around by downgrading bstr to 1.2.0. After doing that, I get this:

error[E0310]: the parameter type `impl Progress` may not live long enough
   --> /home/ben/.cargo/registry/src/index.crates.io-6f17d22bba15001f/git-pack-0.24.0/src/data/output/entry/iter_from_counts.rs:132:5
    |
132 |     parallel::reduce::Stepwise::new(
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...so that the type `impl Progress` will meet its required lifetime bounds
    |
help: consider adding an explicit lifetime bound...
    |
39  |     mut progress: impl Progress + 'static,
    |   

:shrug:

pascalkuthe commented 1 year ago

Thanks for looking I to this! I will do some of my own testing later. It is quite likely that the sigfaults was caused by OOM since that particular test is a fairly complex diff/computation which night consume a lot of memory with miri.

@saethlin the build failures seem to happen in gitoxide. I only use gitoxide for the benchmarks (currently) and not for the test cases so you could just remove gitoxide from the dependency list. Altough it's odd that gitoxide fails to build. I would guess someone did a breaking change on a patch version by accidwnt

saethlin commented 1 year ago

The first break is https://github.com/BurntSushi/bstr/issues/149, which is... messy

jayay commented 1 year ago

That sounds like Miri is just running your system out of memory. If Miri were encountering a problem, you'd get a clear diagnostic.

You're right, that might be the issue. Sorry for the confusion!

I tried compiling this repo but I get this error:

Oh yes, I temporarily fixed that by commenting out the dependency. I should have mentioned that.

I'm closing this issue, since this wasn't the right place to file it. @pascalkuthe also created PR https://github.com/helix-editor/helix/pull/7227. Thanks!