search

rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.16k stars 318 forks source link

Add the machine seed to the diagnostic output #3582

Closed RossSmyth closed 1 month ago

RossSmyth commented 1 month ago

I think this could be a good idea. It makes it easier to reproduce issues just by viewing the diagnostic output.

One thing that could possibly changed is if -Zmiri-seed is not passed, then the seed will be zero every time, so don't display it. But showing it unconditionally that makes it explicit that the seed is zero and not some magic number miri creates.

Anyways let me know your thoughts.

RalfJung commented 1 month ago

Thanks for the PR!

I have to say I am not entirely convinced. Reproducing the issue will require not just the seed but the full set of -Zmiri flags, so if we print anything it should be all those flags (or at least most of them). However, if I look around at other tools, I don't think they usually print all their flags when there is a user error -- e.g., rustc does not. Flags are printed on ICEs, but that should already be the case since we are using the same ICE logic as rustc.

@rust-lang/miri what do you think?

RossSmyth commented 1 month ago

Interestingly I cannot get the filesystem shim tests to pass on my computer. Setting the temp directories to a custom value doesn't fix it, and there are no files as indicated in the comments. I will look in to this later.

saethlin commented 1 month ago

what do you think?

Well, @RossSmyth are you submitting this because you think that this could theoretically be useful, or because it would have adverted a problem you ran into?

RalfJung commented 1 month ago

Interestingly I cannot get the filesystem shim tests to pass on my computer. Setting the temp directories to a custom value doesn't fix it, and there are no files as indicated in the comments. I will look in to this later.

Please file an issue with the exact commands you ran, the output you get, and any other details that may be relevant.

I don't see how that's related to this PR, though.

RossSmyth commented 1 month ago

Given that you guys don't seem keen on this, and I have already spent several hours just trying to make what happens on my computer match what happens in CI I will close this.

I don't see how that's related to this PR, though.

Well given that I am unable to get the tests to pass I would say it is.

RalfJung commented 1 month ago

Are you having trouble getting the tests to pass on unchanged Miri master? If yes, then please file an issue. That's something we should definitely look into. That can't be related to the seeds though, we're fixing the seed for every single test. So I don't see how printing the seed helps fix your test issues.