Run the miri test suite on the aux builder and travis

[oli-obk]

Reopen of #38350

see #43340 (comment) for earlier discussion

Rationale for running miri's test suite in rustc's CI is that miri currently contains many features that we want in const eval in the future, and these features would break if the test suite is not run.

fixes #44077

r? @nikomatsakis

cc @eddyb

[Mark-Simulacrum]

How hard would it be to package up the MIR into an alternate rust-std? Users of miri could then do something like rustup component add rust-std-mir maybe?

[oli-obk]

How hard would it be to package up the MIR into an alternate rust-std? Users of miri could then do something like rustup component add rust-std-mir maybe?

We'd need one for every target, which doesn't scale again. Doing this only gives us an advantage over xargo, if it exists for every target.

[oli-obk]

I changed this PR to be Dev-channel only, since we need more flags for miri validate

[alexcrichton]

Thanks for the PR! The rationale here seems to be mostly about getting miri's test suite working, so I wonder if we could tackle that problem a little more surgically? For example we're already going to have to handle the ability to sometimes run the test suite and sometimes not!

I wonder if this could be a flag like --enable-miri-tests and then that, as a side effect, creates libraries with -Z always-encode-mir? That way we could change the x86_64-aux bot, for example, and this wouldn't need to worry much about nightly/dev/etc.

[oli-obk]

Are there more aux bots for other targets? Or would it be allright to build the other targets' libstd in the aux builder, too?

[eddyb]

@alexcrichton The problem, as I see it, is that there are many targets to be tested, not just one.

[alexcrichton]

Oh of course! That was just an example of what to do. If this is a quick-to-run test suite we can throw it on any bot. All CI configuration is in this repo and should be changeable.

[oli-obk]

Ok, so I didn't do the aux builder thing yet, but it should work after rust-lang/miri#300 is merged. Test suite timing:

real	2m38,609s
user	4m34,396s
sys	3m24,920s

[alexcrichton]

Ah yeah that's totally reasonable, we can throw that in the aux builder for sure.

[alexcrichton]

(or others)

[RalfJung]

Shouldn't this also set -Zmir-emit-validate=1? If it does not, some compile-fail tests in miri will fail.

Notice that this affects the generated artifacts. Doing this on the aux builder sounds fine, but for other builders we should at least have the discussion whether my completely in-development unstable MIR validation statements should really end up in release artifacts.

[RalfJung]

Actually I was wrong, the compile-fail tests do not rely on the libstd having validation turned on. No test currently does, however, it is through libstd code that the validation itself is mostly tested, making sure it accepts all sorts of things that should be accepted.

[RalfJung]

I realized there's another problem with this: What happens the next time something in rustc gets changed such that miri does not compile any more? The test suite will fail, obviously. However, we can't merge the fix to make it compile again into miri/master either, as that would make miri's test suite fail (it is still run against the latest nightly, after all). Seems like we are caught in a cycle, and need something like a staging branch in miri to push fixes for upcoming rustc PRs?

How is that handled with the other test suites rustc depends on already? Or is it not a problem there as they all only use stable, future-compatible interfaces?

[bors]

☔ The latest upstream changes (presumably #43820) made this pull request unmergeable. Please resolve the merge conflicts.

[oli-obk]

We don't have to merge until the next nightly, and the rustc branch can point to the PR commit. The only issue is that rustc will have to point to different people's repos and branches

[eddyb]

@oli-obk Eh, no, you can just have multiple branches in solson/miri.

[oli-obk]

@eddyb: and these are merged without CI then?

[eddyb]

@oli-obk Well, the CI would have to pass in this repo and when making a PR from that branch to master, when the nightly hits.

[oli-obk]

I have not been able to figure out how to configure it so a CI changes a setting of the config.toml. Any help is appreciated.

I want to set the test-miri setting to true.

While I've seen the optimize-tests setting in the config.toml.example, I have not found out how the configure script turns the --disable-optimize-tests into that setting.

[eddyb]

@oli-obk ./configure still outputs config.mk which src/bootstrap knows how to read.

[oli-obk]

Ok, so this should run on travis and on the aux builder now. The test suite itself will fail until #43859 is merged though.

I think this is ready for review.

@RalfJung we'll add -Zmir-emit-validate=1 once there are tests that depend on the libstd being compiled with it.

[alexcrichton]

It loks like miri relies on unstable language and compiler features, which makes me personally wary to land this just yet. We'd like the ability to integrate upstream dependencies which have unstable dependencies, but we haven't quite done the legwork I think to enable that.

@nrc I know the we've thought about this with repositories like the rls/rustfmt as well, but you may want to be aware of this.

Our rough thinking (IIRC) was that we'd have a file in the repository for "known bad test suites". If a test suite is known to fail we'd add it to that file, and then when the test suite passes again we'd delete it from that file. In this way we'd require that the stable (and probably beta) branches would have empty files. If you change the compiler which breaks miri we'd temporarily disable miri tests, land the change, wait for a nightly, update miri, and then un-break the miri tests in this repo.

@nrc do you have thoughts on blocking on that sort of system before landing this PR? I'd personally prefer to not land this with such a procedure in place, but if miri doesn't break much in practice it may be ok to land this ahead of time.

[alexcrichton]

@bors: r+

[bors]

📌 Commit f79d285 has been approved by alexcrichton

[bors]

⌛ Testing commit f79d285db36fba8336f7a4a55cab7448ae31ef18 with merge 79b49e4897dbac6d068d737738c965b72130b9aa...

[bors]

💔 Test failed - status-appveyor

[RalfJung]

eh...

stderr:
------------------------------------------
thread 'main' has overflowed its stack
------------------------------------------
thread '[mir-opt] mir-opt\issue-17877.rs' panicked at 'explicit panic', C:\Users\appveyor\.cargo\registry\src\github.com-1ecc6299db9ec823\compiletest_rs-0.2.10\src\runtest.rs:2374:8

[oli-obk]

I can reproduce on windows. The stacktrace points into rustc though, not into miri. It crashes before it's done compiling. I'm not sure what exactly is going on, since rustc itself doesn't reproduce the issue.

I will disable the test on windows for now and open an issue on the miri repository to investigate. It's only about an unstable feature (slice patterns) anyway. The stacktrace points into rustc_const_eval and looks infinitely recursive. I couldn't get more than that without a compiler with deb info, and that is still compiling (windows machine is slow).

[alexcrichton]

@oli-obk any luck with disabling that test on Windows?

[oli-obk]

Yes. Now i'm waiting for #44522 so I can merge rust-lang/miri#334

[bors]

☔ The latest upstream changes (presumably #44634) made this pull request unmergeable. Please resolve the merge conflicts.

[oli-obk]

Rebased

[oli-obk]

Also miri works on windows now! So this should go through (or is there a mac-aux builder, too? I couldn't find one).

[alexcrichton]

Nah we just test aux on Linux/Windows for now, so you should be good!

@bors: r+

[bors]

📌 Commit 01555b1 has been approved by alexcrichton

[bors]

⌛ Testing commit 01555b1 with merge caad256...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing caad256 to master...

[oli-obk]

Jippiee. Thanks for being so patient with me. this PR was quite the trip