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

-Zmiri-env-forward=RUST_BACKTRACE hint is not very helpful #3838

Closed
clarfonthey opened this issue Aug 24, 2024 · 6 comments
Closed

-Zmiri-env-forward=RUST_BACKTRACE hint is not very helpful #3838

clarfonthey opened this issue Aug 24, 2024 · 6 comments

Comments

@clarfonthey
Copy link
Contributor

Let me explain the order of operations I went through:

  1. Run cargo miri test. Realise there are no backtraces.
  2. I try again with env RUST_BACKTRACE=1 cargo miri test.
  3. This still doesn't work. I double-check the output and I get this little nugget:
    note: in Miri, you may have to set `-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
    
  4. env RUST_BACKTRACE=1 cargo miri test -Zmiri-env-forward=RUST_BACKTRACE
  5. error: unknown `-Z` flag specified: miri-env-forward
    
    For available unstable features, see https://doc.rust-lang.org/nightly/cargo/reference/unstable.html
    If you intended to use an unstable rustc feature, try setting `RUSTFLAGS="-Zmiri-env-forward"
    
  6. env RUST_BACKTRACE=1 RUSTFLAGS=-Zmiri-env-forward=RUST_BACKTRACE cargo miri test
  7. Still doesn't work. At this point, I have to search for -Zmiri-env-forward and ultimately stumble on the readme for this repo mentioning I should use MIRIFLAGS, not RUSTFLAGS.

This is… very frustrating. Not to mention I have a particularly large test suite I was running this on, and miri was running all tests by default, and it was quite slow between each step. It would be nice if any of the following applied:

  1. RUST_BACKTRACE was just special and always passed to miri, at least via cargo miri.
  2. cargo miri accepted -Z flags to be passed directly to miri.
  3. The comment mentioning RUST_BACKTRACE=1 just explicitly said RUST_BACKTRACE=1 MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE.

This could potentially also be a cargo change, but since this message is being generated by miri itself, I figured I'd start here first.

@RalfJung
Copy link
Member

RalfJung commented Aug 24, 2024

Thanks for taking the time to report this! I agree the current experience is not great.

RUST_BACKTRACE was just special and always passed to miri, at least via cargo miri.

I'm afraid I don't want to do this. This would make Miri's RNG take an entirely different route through the program when that env var is set, which is something we do not want to occur -- Miri should be deterministic.

cargo miri accepted -Z flags to be passed directly to miri.

That's #2051. I am not sure yet about this, it's not at all how cargo usually works, and the implementation would necessarily be a bit hacky, but I am not fundamentally opposed.

The comment mentioning RUST_BACKTRACE=1 just explicitly said RUST_BACKTRACE=1 MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE.

rust-lang/rust#129501 should help here. (The message is actually not generated by Miri.)

@RalfJung
Copy link
Member

RalfJung commented Aug 24, 2024

On a somewhat unrelated note:

and miri was running all tests by default

Miri doesn't even know what a test suite is. It just runs the test harness as if it was any other binary. You can use the usual means of running only a particular subset of tests, same as with cargo test: cargo miri test -- filter1 filter2. There's little we can do here. Check out cargo nextest for an alternative where a lot of the test harness logic lives outside the interpreted program; that is supported in Miri via cargo miri nextest as well.

@clarfonthey
Copy link
Contributor Author

clarfonthey commented Aug 24, 2024

I'm afraid I don't want to do this. This would make Miri's RNG take an entirely different route through the program when that env var is set, which is something we do not want to occur -- Miri should be deterministic.

Hmm, is that because miri relies on the environment variables to seed the RNG, or because the presence of backtraces affects the RNG? Since I would assume that backtraces wouldn't affect anything, but I guess they probably do.

@RalfJung
Copy link
Member

RalfJung commented Aug 24, 2024

It is because putting the env var into the machine state creates an allocation to hold the env var value, which fetches a number from the RNG and just changes what is returned by every subsequent RNG query.

The presence of this variable in the heap also shifts around the memory locations of subsequence heap allocations, so even the length of an env var value changes the rest of the execution.

@clarfonthey
Copy link
Contributor Author

Huh, I hadn't even realised that miri uses RNG to do allocations; it makes a lot of sense, but I hadn't thought of it.

Maybe this is an indication that there should be some external way to set RUST_BACKTRACE without any allocations. Although I'm not sure if there would be a way to store and do the backtraces then, since I don't think they're actually stored without the flag.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 24, 2024
…ratrieb

panicking: improve hint for Miri's RUST_BACKTRACE behavior

Should help with rust-lang/miri#3838
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 24, 2024
…ratrieb

panicking: improve hint for Miri's RUST_BACKTRACE behavior

Should help with rust-lang/miri#3838
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Aug 25, 2024
Rollup merge of rust-lang#129501 - RalfJung:miri-rust-backtrace, r=Noratrieb

panicking: improve hint for Miri's RUST_BACKTRACE behavior

Should help with rust-lang/miri#3838
github-actions bot pushed a commit that referenced this issue Aug 26, 2024
panicking: improve hint for Miri's RUST_BACKTRACE behavior

Should help with #3838
@RalfJung
Copy link
Member

RalfJung commented Sep 5, 2024

rust-lang/rust#129501 merged, and #2051 tracks making these flags easier to set -- I think there's nothing else left to do here (I don't think we want to forward this env var by default), so I'll close the issue.

@RalfJung RalfJung closed this as completed Sep 5, 2024
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

2 participants