`cargo miri test` is broken for doctests (Option 'sysroot' given more than once) #3404

RalfJung · 2024-03-24T09:56:50Z

rust-lang/rust#122840 changed the way rustdoc passes arguments to rustc for doctest builds, which causes cargo miri test to break. The first symptom is output like

---- src/lib.rs - my_fn (line 37) stdout ----
error: Option 'sysroot' given more than once

Couldn't compile the test.

The deeper cause is that Miri does a lot of scanning and patching of rustc command-line arguments, and if those arguments are hidden in a file then everything breaks.

The text was updated successfully, but these errors were encountered:

RalfJung · 2024-03-24T10:17:34Z

cargo miri test -v shows

[cargo-miri rustc inside rustdoc] going to run:
MIRI_BE_RUSTC="target" "/home/r/.rustup/toolchains/miri/bin/miri" "@/tmp/rustdoctestAMWaIS/rustdoc-cfgs" "--edition" "2018" "-o" "/tmp/rustdoctestyUJi8S/rust_out.miri" "--emit=metadata" "--target" "x86_64-unknown-linux-gnu" "--color" "always" "-"

That arg file contains

--crate-type=bin
--cfg=miri
--sysroot=/home/r/.cache/miri
-Ldependency=/home/r/src/rust/miri/test-cargo-miri/target/miri/x86_64-unknown-linux-gnu/debug/deps
-Ldependency=/home/r/src/rust/miri/test-cargo-miri/target/miri/debug/deps
--extern=byteorder_2=/home/r/src/rust/miri/test-cargo-miri/target/miri/x86_64-unknown-linux-gnu/debug/deps/libbyteorder-ee60f6ecbd530bd3.rmeta
--extern=byteorder=/home/r/src/rust/miri/test-cargo-miri/target/miri/x86_64-unknown-linux-gnu/debug/deps/libbyteorder-f178d57ea71fe0c8.rmeta
--extern=cargo_miri_test=/home/r/src/rust/miri/test-cargo-miri/target/miri/x86_64-unknown-linux-gnu/debug/deps/libcargo_miri_test-abd1c36147f6f274.rmeta
--extern=exported_symbol=/home/r/src/rust/miri/test-cargo-miri/target/miri/x86_64-unknown-linux-gnu/debug/deps/libexported_symbol-6d3a18b7c769098c.rmeta
--extern=issue_1567=/home/r/src/rust/miri/test-cargo-miri/target/miri/x86_64-unknown-linux-gnu/debug/deps/libissue_1567.rmeta
--extern=issue_1691=/home/r/src/rust/miri/test-cargo-miri/target/miri/x86_64-unknown-linux-gnu/debug/deps/libissue_1691-65d452f0d1689ec9.rmeta
--extern=issue_1705=/home/r/src/rust/miri/test-cargo-miri/target/miri/x86_64-unknown-linux-gnu/debug/deps/libissue_1705.rmeta
--extern=issue_1760=/home/r/src/rust/miri/test-cargo-miri/target/miri/debug/deps/libissue_1760-c4d1a96b539ee302.so
--extern=issue_rust_86261=/home/r/src/rust/miri/test-cargo-miri/target/miri/x86_64-unknown-linux-gnu/debug/deps/libissue_rust_86261-22dc9d7df84cf40e.rmeta
--extern=serde_derive=/home/r/src/rust/miri/test-cargo-miri/target/miri/debug/deps/libserde_derive-bc2328ee0791f66b.so
-Ccodegen-units=1
-Cembed-bitcode=no
-Zunstable-options

So... these arguments actually are all already patched properly as we are patching the --extern etc before invoking rustdoc.

The only actual issue is that Miri needs to run the rustc driver with a different default sysroot, and there's currently no way to do that other than patching command-line arguments. And annoyingly rustc bails when there are multiple --sysroot so we have to scan the arguments to see if some other sysroot is already set and of course that fails when there are argument files.

Ideally rustc_driver would just let us set a default sysroot so we don't need these command-line parsing hacks. (Well we still need them in cargo-miri but at least not in the miri driver. So in the future arg files may still cause more issues.)

RalfJung · 2024-03-24T11:11:00Z

Cc @GuillaumeGomez

It seems like rust-lang/rust#122993 may help though (hard to test since I haven't yet managed to run cargo-miri with doctests inside the rustc sysroot). But there could still be various other things going wrong in cargo-miri as it relies quite a bit on being able to scan the command-line arguments passed to rustc...

Does cargo itself ever use arg files?

dtolnay · 2024-03-25T02:35:03Z

For anyone looking for a workaround: use cargo miri test --all-targets for now, which skips doctests but includes unit tests, examples, benches, and integration tests in the library and all binaries as cargo miri test would.

RalfJung · 2024-03-25T07:30:44Z

Looking at what cargo-miri does here, even if rust-lang/rust#122993 resolves the --sysroot flag issue, there is still some command-line scanning going on for "the rustc invoked by rustdoc". So here are some flags where if those were passed inside an arg file, everything breaks:

--target
--crate-type=lib (I see you are passing the --crate-type=bin inside the arg file, that happens to work since bin is also the default when there is no flag -- Miri sees no flag and assumes the default)
--test
--emit
-o, --out-dir
--crate-name
-C extra-filename

Will rustdoc ever pass any of these in an arg file?

Probably Miri should "just" implement arg file handling, but not sure when we'll get around to doing that.

give rustc_driver users a chance to overwrite the default sysroot and then use that in Miri. This lets us get rid of an annoying arg-patching hack, and may help with rust-lang/miri#3404. Unfortunately the computation of `real_rust_source_base_dir` depended on knowing the default sysroot, so I had to move that around a bit. I have no idea how all this `Session` and `Options` stuff works so I hope this makes sense.

Rustup, fix rustdoc sysroot issue This uses a different approach to resolve #3404: we entirely move the responsibility of setting miri-sysroot to whatever *invokes* the Miri driver. cargo-miri knows whether it is inside rustdoc or not and can adjust accordingly. I previously avoided doing that because there are a bunch of places that are invoking the driver (cargo-miri, the ui test suite, `./miri run`, `./x.py run miri`) and they all need to be adjusted now. But it is also somewhat less fragile as we usually have more information there -- and we can just decide that `./miri run file.rs --sysroot path` is not supported.

GuillaumeGomez · 2024-03-25T10:28:19Z

So anything to change in rustdoc for this to work? I can move --sysroot back outside of the arg file, same for --crate-type if it can fix your case.

RalfJung · 2024-03-25T10:42:56Z

I think I found a solution in #3409. But yes moving --sysroot back out would also work. But maybe it's better to make the Miri driver less fragile here.

Looking at your PR, I think the only one that could still be an issue is -C extra-filename as you are passing all codegen flags via the args file. Not sure if that can ever be set in rustdoc though.

GuillaumeGomez · 2024-03-25T10:45:58Z

You can use -C with rustdoc (that's where we get these flags from).

RalfJung · 2024-03-25T10:52:54Z

Okay #3409 is hitting some issues that I can't investigate right now. @GuillaumeGomez if you could move --sysroot from the arg file to the regular CLI arguments for now, so that we get time to resolve this on the Miri side, that would be much appreciated.

GuillaumeGomez · 2024-03-25T10:54:46Z

Doing it right away. 👍

RalfJung · 2024-03-25T16:21:29Z

Damn, I am locally still seeing the error after updating...

RalfJung · 2024-03-25T16:32:18Z

Ah, it's harmless... previously rustdoc used --sysroot <path>, now it's --sysroot=<path>. I can make that work.

RalfJung · 2024-03-25T17:29:32Z

Okay that worked, thanks to @GuillaumeGomez for the help. :)

Rustup Fixes rust-lang/miri#3404

GuillaumeGomez · 2024-03-25T20:46:56Z

Glad it was fixed for you. :)

This was referenced Mar 24, 2024

Automatic Rustup #3401

Closed

rustdoc --test: Prevent reaching the maximum size of command-line by using files for arguments if there are too many rust-lang/rust#122840

Merged

RalfJung changed the title ~~cargo miri test is broken for doctests~~ cargo miri test is broken for doctests (Option 'sysroot' given more than once) Mar 24, 2024

RalfJung pinned this issue Mar 24, 2024

RalfJung mentioned this issue Mar 24, 2024

give rustc_driver users a chance to overwrite the default sysroot rust-lang/rust#122993

Closed

RalfJung mentioned this issue Mar 25, 2024

Automatic Rustup #3408

Closed

RalfJung mentioned this issue Mar 25, 2024

x.py: Test cargo miri test for doctests rust-lang/rust#123028

Closed

elenakrittik mentioned this issue Mar 25, 2024

deps: update reqwest requirement from 0.11.23 to 0.12.1 elenakrittik/gdtk#37

Closed

RalfJung mentioned this issue Mar 25, 2024

Rustup, fix rustdoc sysroot issue #3409

Closed

GuillaumeGomez mentioned this issue Mar 25, 2024

Move --sysroot argument out of the argument file to fix miri issue rust-lang/rust#123030

Merged

RalfJung mentioned this issue Mar 25, 2024

Support argfiles in all our argument munching #3412

Open

2 tasks

hunger mentioned this issue Mar 25, 2024

live preview: Add some testing to element selection slint-ui/slint#4929

Merged

bors closed this as completed in rust-lang-ci/rust@cb7c636 Mar 25, 2024

RalfJung reopened this Mar 25, 2024

RalfJung mentioned this issue Mar 25, 2024

Rustup #3414

Merged

bors closed this as completed in c941fcd Mar 25, 2024

RalfJung pushed a commit to RalfJung/rust that referenced this issue Mar 25, 2024

Auto merge of rust-lang#3414 - RalfJung:rustup, r=RalfJung

cf1eb93

Rustup Fixes rust-lang/miri#3404

DaniPopes mentioned this issue Mar 25, 2024

fix(json-abi): correct to_sol for arrays of contracts alloy-rs/core#586

Merged

Robbepop mentioned this issue Mar 25, 2024

Merge copy instructions if possible wasmi-labs/wasmi#969

Merged

MaxCWhitehead mentioned this issue Mar 25, 2024

CI: Miri started failing fishfolk/bones#357

Closed

RalfJung unpinned this issue Mar 27, 2024

Robbepop mentioned this issue Mar 28, 2024

Optimize shadow stack instruction sequences wasmi-labs/wasmi#928

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`cargo miri test` is broken for doctests (Option 'sysroot' given more than once) #3404

`cargo miri test` is broken for doctests (Option 'sysroot' given more than once) #3404

RalfJung commented Mar 24, 2024 •

edited

Loading

RalfJung commented Mar 24, 2024 •

edited

Loading

RalfJung commented Mar 24, 2024

dtolnay commented Mar 25, 2024

RalfJung commented Mar 25, 2024 •

edited

Loading

GuillaumeGomez commented Mar 25, 2024

RalfJung commented Mar 25, 2024

GuillaumeGomez commented Mar 25, 2024

RalfJung commented Mar 25, 2024 •

edited

Loading

GuillaumeGomez commented Mar 25, 2024

RalfJung commented Mar 25, 2024

RalfJung commented Mar 25, 2024

RalfJung commented Mar 25, 2024

GuillaumeGomez commented Mar 25, 2024

cargo miri test is broken for doctests (Option 'sysroot' given more than once) #3404

cargo miri test is broken for doctests (Option 'sysroot' given more than once) #3404

Comments

RalfJung commented Mar 24, 2024 • edited Loading

RalfJung commented Mar 24, 2024 • edited Loading

RalfJung commented Mar 24, 2024

dtolnay commented Mar 25, 2024

RalfJung commented Mar 25, 2024 • edited Loading

GuillaumeGomez commented Mar 25, 2024

RalfJung commented Mar 25, 2024

GuillaumeGomez commented Mar 25, 2024

RalfJung commented Mar 25, 2024 • edited Loading

GuillaumeGomez commented Mar 25, 2024

RalfJung commented Mar 25, 2024

RalfJung commented Mar 25, 2024

RalfJung commented Mar 25, 2024

GuillaumeGomez commented Mar 25, 2024

`cargo miri test` is broken for doctests (Option 'sysroot' given more than once) #3404

`cargo miri test` is broken for doctests (Option 'sysroot' given more than once) #3404

RalfJung commented Mar 24, 2024 •

edited

Loading

RalfJung commented Mar 24, 2024 •

edited

Loading

RalfJung commented Mar 25, 2024 •

edited

Loading

RalfJung commented Mar 25, 2024 •

edited

Loading