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

make miri usable as stage 0/1 tool #52856

Closed
RalfJung opened this issue Jul 30, 2018 · 19 comments · Fixed by rust-lang/miri#1405
Closed

make miri usable as stage 0/1 tool #52856

RalfJung opened this issue Jul 30, 2018 · 19 comments · Fixed by rust-lang/miri#1405
Labels
A-miri Area: The miri tool C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@RalfJung
Copy link
Member

RalfJung commented Jul 30, 2018

Moving miri to the compiler made development with miri significantly harder. To avoid rebuilding the compiler twice on every change in librustc_mir/interpret, I usually do ./x.py build src/rustc --keep-stage 0. This still does unnecessary work, however, when changing libstd (but that is likely hard to avoid?). Also --keep-stage is somewhat fragile. So overall, developing miri is much more cumbersome than previously (and that's not even talking about it being much harder to land patches, and much more likely to get merge conflicts, but that part is unavoidable).

It would be great if one could build, test and run miri in earlier stages. The building part actually works, but only with a trick:

$ CARGO_CFG_REGEX_DISABLE_AUTO_OPTIMIZATIONS=1 ./x.py --stage 0 build src/tools/miri

The env var is needed because stage 0 libstd does not contain std::arch. Could we change that?

Running works but is not very straight-forward

$ cp build/x86_64-unknown-linux-gnu/stage0-tools-bin/miri build/x86_64-unknown-linux-gnu/stage1/bin/
$ MIRI_SYSROOT=build/x86_64-unknown-linux-gnu/stage1/ build/x86_64-unknown-linux-gnu/stage1/bin/miri src/tools/miri/tests/run-pass/products.rs

Looks like RUST_SYSROOT is not properly set during build, so I have to give the sysroot manually?

Running the test suite, however, fails:

$ CARGO_CFG_REGEX_DISABLE_AUTO_OPTIMIZATIONS=1 ./x.py --stage 0 test src/tools/miri -v
[...]
running: "/home/r/src/rust/rustc/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-j" "8" "--release" "--manifest-path" "/home/r/src/rust/rustc/src/tools/miri/Cargo.toml"
[...]
error: failed to find a `codegen-backends` folder in the sysroot candidates:
* /home/r/src/rust/rustc/build/x86_64-unknown-linux-gnu
* /home/r/src/rust/rustc/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu
* /home/r/src/rust/rustc/build/x86_64-unknown-linux-gnu/stage0-sysroot

How could we do better (other than maybe enabling std::arch in stage 0)? @eddyb suggested that test src/tools/miri should behave more like test src/test/run-pass, and test the stuff produced by the previous stage, but I find that somewhat strange. The compiler itself has strange "leveling issues" in a staged build, but the tools should be more consistent.

I'd expect miri to behave more like libstd: build --stage 1 src/libstd builds using the compiler produced in stage 0, and test --stage 1 src/libstd --no-doc tests with that same compiler. So maybe build --stage 1 src/tools/miri should be built with the compiler produced in stage 0 and the libstd produced in stage 1 (similar to build --stage 1 src/rustc). build --stage 0 src/tools/miri would use the bootstrap compiler to build miri, which will likely not work but that's okay. test --stage 1 src/tools/miri would first do build --stage 1 src/tools/miri and then run the tests in the stage1 sysroot.
Or does that not work because miri depends on librustc?

Cc @oli-obk @eddyb (and maybe we should get a bootstrap expert in the loop?)

@eddyb
Copy link
Member

eddyb commented Aug 1, 2018

cc @Mark-Simulacrum

@Mark-Simulacrum
Copy link
Member

The env var is needed because stage 0 libstd does not contain std::arch. Could we change that?

Not really -- this is done so that the compiler can be more easily changed in how it handles SIMD. It would also be a degradation in overall compile times since stage 0 builds will take longer.

Looks like RUST_SYSROOT is not properly set during build, so I have to give the sysroot manually?

We do not set RUST_SYSROOT at all today -- I'm certain a patch to do so can be accepted, though. Should be relatively easy.

Running the test suite, however, fails: [...]

It looks like miri needs the codegen backends to function -- this is rather unexpected; the reason the test suite fails is that the codegen backends aren't built/linked into the right place. This should also be relatively easy to fix.


I'm not clear on what MIRI needs to work properly, but I suspect that we can make the situation better here. I think what would be most useful at this point is a summary of what you expect to happen after changing std, test, and rustc_mir, for example (i.e., what needs to be rebuilt). Does miri use proc macros?

@RalfJung
Copy link
Member Author

RalfJung commented Aug 2, 2018

It looks like miri needs the codegen backends to function -- this is rather unexpected; the reason the test suite fails is that the codegen backends aren't built/linked into the right place. This should also be relatively easy to fix.

I am also surprised that miri needs the codegen backends. It shouldn't codegen anything.

Does miri use proc macros?

miri itself does not (AFAIK), but cargo-miri depends on cargo_metadata which depends on serde. Somehow cargo test builds that, I am not sure why. I would be fine with cargo-miri not working in stage 0/1.

EDIT: Ah, miri's test suite needs compiletest which depends on serde as well, for some reason.

EDIT: I managed to get rid of the regex dependency, so SIMD should not be a problem any more. Never mind, env_logger still needs it.

Not really -- this is done so that the compiler can be more easily changed in how it handles SIMD. It would also be a degradation in overall compile times since stage 0 builds will take longer.

Compile time impact is <10s for me, but while I do not understand what the first part of your sentence means, I get that it might be annoying for compiler/SIMD development in the future.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 2, 2018

Ah dang compiletest-rs uses serde to parse rustc JSON output. So I guess yes we need proc macros. That's a no-go currently for stages <2, right?


I think what would be most useful at this point is a summary of what you expect to happen after changing std, test, and rustc_mir, for example (i.e., what needs to be rebuilt).

Ideally on a rustc_mir change, it should rebuild the stage0 compiler artifacts and then build a stage1 libstd with that compiler and then build miri with that.

When changing std, it'd start with step 2 above, i.e. not touch the compiler at all. I guess that needs a --keep-stage 0 as otherwise it'd also recompile the libstd used by the compiler. Not sure what you mean by changing "test", but I guess libtest works like libstd in that regard?

I realize this is off-by-1-stage compared to now: I am imagining stage 1 miri to be built with the stage 0 compiler artifacts. This is in line with how @eddyb told me a "stage" is defined; it is consistent e.g. with --stage 1 test src/run-pass which tests the stage 0 compiler artifacts.

@jonas-schievink jonas-schievink added A-miri Area: The miri tool T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Apr 21, 2019
@JohnTitor JohnTitor added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Oct 19, 2019
@RalfJung
Copy link
Member Author

RalfJung commented May 3, 2020

@Mark-Simulacrum said the situation changed, so I re-tested this.

My first command to run was ./x.py build --stage 1 src/tools/miri, but interestingly this builds the stage 1 compiler artifacts (unlike build --stage 1 src/libstd), so I cancelled that as that's not what I wanted. I thought I understood how our stages work but it seems I still do not... Cc #59864

So I went for ./x.py build --stage 0 src/tools/miri instead. And that passes! And it does actually seem to use the compiler produced by the stage 0 compiler artifacts, i.e., the compiler used to build stage 1, even though the flag seems to say otherwise. Weird, but whatever.

Unfortunately, ./x.py test --stage 0 src/tools/miri fails. And this probably has to do with this message:

     Running `build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/cargo-miri miri setup`
WARNING: the sysroot can't be built for the Beta channel. Switch to nightly.

This messages comes from xargo, which is not happy about the compiler we are using... which is strange because to build Miri it clearly used the compiler produced by the stage 0 compiler artifacts, but now xargo seems to pick up the bootstrap compiler and then bails out.

@oli-obk
Copy link
Contributor

oli-obk commented May 4, 2020

it uses the compiler produced by stage 0 (as a library), but it's not compiled by that compiler. Maybe we need to teach xargo about RUST_BOOTSTRAP=1?

@RalfJung
Copy link
Member Author

RalfJung commented May 4, 2020

it uses the compiler produced by stage 0 (as a library), but it's not compiled by that compiler.

Oh... 🤯
I... think that makes sense... maybe?

Maybe we need to teach xargo about RUST_BOOTSTRAP=1?

Or can we make xargo use the compiler produced by stage 0 to build libstd? Not sure if that was already assembled at this point.

@oli-obk
Copy link
Contributor

oli-obk commented May 4, 2020

Or can we make xargo use the compiler produced by stage 0 to build libstd? Not sure if that was already assembled at this point.

I think that will be even more confusing, as everything else is built by the stage 0 compiler (the beta compiler using RUST_BOOTSTRAP)

@RalfJung
Copy link
Member Author

RalfJung commented May 6, 2020

With this xargo patch, it succeeds at building a libstd for Miri.

However, later all tests fail when it tries to use that libstd:

error[E0514]: found crate `std` compiled by an incompatible version of rustc
  |
  = help: please recompile that crate using this compiler (rustc 1.45.0-dev)
  = note: the following crate versions were found:
          crate `std` compiled by rustc 1.44.0-beta.752 (a7d891e31 2020-04-21): /home/r/.cache/miri/HOST/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-b28cadfa61171505.rmeta

error: aborting due to previous error

That's expected I guess -- the libstd is loaded by the rustc built into Miri which is the one created by "stage 0 compiler artifacts", but that is not the rustc that built this libstd.

So I think we have to make xargo use the compiler produced by stage 0 to build libstd. I do not know how to achieve that though.

@oli-obk
Copy link
Contributor

oli-obk commented May 7, 2020

ah yes, that makes sense. Can we make xargo use the miri binary to produce the artifactsß

@RalfJung
Copy link
Member Author

RalfJung commented May 7, 2020

uh...
We'd need some magic flag or so to tell Miri to be a normal compiler and not an interpreter.

@RalfJung
Copy link
Member Author

RalfJung commented May 7, 2020

But yeah that is probably the cleanest solution. What is the best way to just say "behave like the rustc binary would"?

The main thing I am worried about is that it means all the codegen stuff in Miri will be reachable, so the binary might become bigger. But OTOH I doubt that the linker currently is clever enough to actually remove all that stuff, so... whatever.

@oli-obk
Copy link
Contributor

oli-obk commented May 7, 2020

the llvm backend is a dylib anyway, and the rest... yea, I doubt that gets optimized away.

What is the best way to just say "behave like the rustc binary would"?

Run

rustc_driver::main()

@RalfJung
Copy link
Member Author

RalfJung commented May 9, 2020

I like making Miri behaving like rustc when told so, then actually cargo-miri can just always invoke the Miri binary and thus we never have the problem that the rustc and Miri we are using are incompatible! I think we can kill the sysroot check when/if this works. :D

However, in the context of rustbuild it gets me all sort of build failures for the build scripts needed by libcore... they don't have a libstd, rather unsurprisingly. Almost as if we had a bootstrap problem here. ;) Right now I am setting the RUSTC env var for use by xargo, and that overwrites the rustc wrapper set up by bootstrap... I need to figure out a way to have both of them. Hm.

@oli-obk
Copy link
Contributor

oli-obk commented May 9, 2020

There's also the RUSTC_WRAPPER env var, which behaves slightly differently from the RUSTC env var.

@RalfJung
Copy link
Member Author

RalfJung commented May 9, 2020

Yes, but one env var just takes precedence over the other, and either choice is wrong.

We need the bootstrap RUSTC to pick up Miri as the rustc binary... maybe it needs its own env var for "wrapper chaining" or so. ;)

@RalfJung
Copy link
Member Author

RalfJung commented May 9, 2020

Ah, we have RUSTC_REAL, that'll do. :D

That gets us through the xargo phase. Now we get:

error[E0514]: found crate `std` compiled by an incompatible version of rustc
  |
  = help: please recompile that crate using this compiler (rustc 1.44.0-beta.752 (a7d891e31 2020-04-21))
  = note: the following crate versions were found:
          crate `std` compiled by rustc 1.45.0-dev: /home/r/.cache/miri/HOST/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-6aa1e7bd4327dd7a.rmeta

That's the opposite error. Something somewhere is using the host rustc for target (Miri) things...

@RalfJung
Copy link
Member Author

RalfJung commented May 9, 2020

Yay, I git it to work. :D
rust-lang/miri#1405

@RalfJung
Copy link
Member Author

RalfJung commented May 9, 2020

I confirmed that with latest Miri master, ./x.py test --stage 1 src/tools/miri works.

This makes me so happy. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-miri Area: The miri tool C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants