Skip to content

Commit

Permalink
Auto merge of #1540 - RalfJung:cargo-miri-redone, r=oli-obk
Browse files Browse the repository at this point in the history
Redo cargo-miri logic

This rewrite the cargo-miri logic for running the requested crate(s) following what we outlined in #739: `cargo miri run/test $FLAGS` (almost) literally invokes `cargo run/test $FLAGS` but with some environment variables set so that we can control what happens:
* `RUSTC_WRAPPER` is set so that we get invoked instead of `rustc`. We use that power to mess with the flags being used for the build (things to be interpreted by Miri use a different sysroot), and when we are detecting a binary crate meant to be run by Miri, we grab the info we care about and put it into a JSON file for later use.
* `CARGO_TARGET_$TARGET_RUNNER` is set so what we get invoked when cargo wants to run a binary. At that point we take that JSON info from before and use it to invoke Miri.

Overall this works great! We get all sorts of cargo magic for free, and do not even need `cargo-metadata` any more. There's one annoying point though, which I have not managed to entirely work around yet: this means we are doing a full build, not just a check-build. Given that our sysroot is MIR-only, I was surprised that this even worked, but still -- this means we are doing more work than we should. So I also added some patching of arguments to make sure `--emit` does not contain `link`, and then more patching was required of the `--extern` flags for the binary because those referenced the `.rlib` files but now only `.rmeta` exists, and that is still not fully working because cargo seems to expect those `.rmeta` files and now triggers a rebuild each time as those files are still missing. My current plan is to make our wrapper create some empty dummy files with the right names, but the amount of hacks we are stacking on top of each other here is getting worrysome.^^ `@ehuss` your input would be welcome on this issue.

Due to re-using cargo more literally, this also changes flag parsing to match `cargo`. So `-Zmiri` flags now have to be passed via an environment variable (Cc #1416).

This PR is not ready yet, but the parts that are there I think can be reviewed already. TODO:
* [x] [Fix Windows](#1540 (comment)).
* [x] Figure out how we can do check-only builds without the rebuild problem above. ~~I am also worried about cases where `build.rs` script might detect check-only builds and then do less work; I think some crates in rustc are doing that and if this is a thing in the wider ecosystem we need to find a way to support this as well.~~ (postponed that until we have a concrete example)
* [x] Currently cargo runs doctests as well, and they are not run in Miri. We should at least figure out a way to not run them at all (resolving #584 is left for a future PR).
* [x] For some time, detect the old way of passing `-Zmiri` flags and warn that this will need updating. For some simple situations we can probably make it still support the old way, but I plan to remove those hacks after a bit. This is just to give people and me time to go around and send PRs to all projects that use Miri on CI, and update their use of the flags.
* [x] Add a test for stdin handling (#1505). This should work now but we should be sure.
* [x] Add a test for cargo env vars (#1515).
* [x] Check if #1516 is resolved.
* [x] Check if #1001 and #1514 are resolved.
* [x] Check if #1312 is resolved (not sure if it is wort adding a test).
* [x] Check if #1512 is resolved (not sure if it is wort adding a test).

Fixes #700.
Fixes #739.
Fixes #1001.
Fixes #1312 (without a test, as we run without cargo's stdin/stdout wrapping now, as the test for stdin confirms).
Fixes #1416.
Fixes #1505.
Fixes #1512 (without a test, as cargo now does all that handling anyway, which various other tests confirm).
Fixes #1514.
Fixes #1516.

Cc `@alecmocatta` who reported many of the bugs above; would be great if you could help with the tests e.g. by providing some small examples I could try.
r? `@oli-obk`
  • Loading branch information
bors committed Sep 17, 2020
2 parents 79f023e + ae859c3 commit ce29fbf
Show file tree
Hide file tree
Showing 31 changed files with 598 additions and 416 deletions.
20 changes: 12 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,10 @@ Now you can run your project in Miri:
The first time you run Miri, it will perform some extra setup and install some
dependencies. It will ask you for confirmation before installing anything.

You can pass arguments to Miri after the first `--`, and pass arguments to the
interpreted program or test suite after the second `--`. For example, `cargo
miri run -- -Zmiri-disable-stacked-borrows` runs the program without checking
the aliasing of references. To filter the tests being run, use `cargo miri test
-- -- filter`.
`cargo miri run/test` supports the exact same flags as `cargo run/test`. You
can pass arguments to Miri via `MIRIFLAGS`. For example,
`MIRIFLAGS="-Zmiri-disable-stacked-borrows" cargo miri run` runs the program
without checking the aliasing of references.

Miri supports cross-execution: if you want to run the program as if it was a
Linux program, you can do `cargo miri run --target x86_64-unknown-linux-gnu`.
Expand Down Expand Up @@ -163,7 +162,8 @@ up the sysroot. If you are using `miri` (the Miri driver) directly, see the
## Miri `-Z` flags and environment variables
[miri-flags]: #miri--z-flags-and-environment-variables

Miri adds its own set of `-Z` flags:
Miri adds its own set of `-Z` flags, which are usually set via the `MIRIFLAGS`
environment variable:

* `-Zmiri-disable-alignment-check` disables checking pointer alignment, so you
can focus on other failures, but it means Miri can miss bugs in your program.
Expand Down Expand Up @@ -229,21 +229,25 @@ Moreover, Miri recognizes some environment variables:

* `MIRI_LOG`, `MIRI_BACKTRACE` control logging and backtrace printing during
Miri executions, also [see above][testing-miri].
* `MIRIFLAGS` (recognized by `cargo miri` and the test suite) defines extra
flags to be passed to Miri.
* `MIRI_SYSROOT` (recognized by `cargo miri` and the test suite)
indicates the sysroot to use. To do the same thing with `miri`
directly, use the `--sysroot` flag.
* `MIRI_TEST_TARGET` (recognized by the test suite) indicates which target
architecture to test against. `miri` and `cargo miri` accept the `--target`
flag for the same purpose.
* `MIRI_TEST_FLAGS` (recognized by the test suite) defines extra flags to be
passed to Miri.

The following environment variables are internal, but used to communicate between
different Miri binaries, and as such worth documenting:

* `MIRI_BE_RUSTC` when set to any value tells the Miri driver to actually not
interpret the code but compile it like rustc would. This is useful to be sure
that the compiled `rlib`s are compatible with Miri.
* `MIRI_CWD` when set to any value tells the Miri driver to change to the given
directory after loading all the source files, but before commencing
interpretation. This is useful if the interpreted program wants a different
working directory at run-time than at build-time.

## Miri `extern` functions

Expand Down
24 changes: 1 addition & 23 deletions cargo-miri/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion cargo-miri/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ test = false # we have no unit tests
doctest = false # and no doc tests

[dependencies]
cargo_metadata = "0.11"
directories = "2.0"
rustc_version = "0.2.3"
serde_json = "1.0.40"
Expand Down
Loading

0 comments on commit ce29fbf

Please sign in to comment.