cargo-miri builds proc-macro crate's unit tests as an executable and tries to interpret it as JSON

[ghost]

I tried this with today's nightly:

cargo new --lib cargo-miri-bug --vcs none
cd cargo-miri-bug
echo -e '[lib]\nproc-macro = true' >> Cargo.toml
cargo miri test

And it reported an error:

    Finished test [unoptimized + debuginfo] target(s) in 0.50s
     Running `/path/to/my/home/directory/.rustup/toolchains/nightly-2020-12-30-x86_64-unknown-linux-gnu/bin/cargo-miri /the/directory/cargo-miri-bug/target/debug/deps/cargo_miri_bug-90df6bde175539f3`
fatal error: file "/the/directory/cargo-miri-bug/target/debug/deps/cargo_miri_bug-90df6bde175539f3" contains outdated or invalid JSON; try `cargo clean`
error: test failed, to rerun pass '--lib'

But cargo_miri_bug-90df6bde175539f3 is not JSON at all:

$ file target/debug/deps/cargo_miri_bug-90df6bde175539f3
target/debug/deps/cargo_miri_bug-90df6bde175539f3: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=a50f714aa423145427213e658d992f246548e39d, for GNU/Linux 3.2.0, with debug_info, not stripped
$ LD_LIBRARY_PATH=~/.rustup/toolchains/nightly-2020-12-30-x86_64-unknown-linux-gnu/lib target/debug/deps/cargo_miri_bug-90df6bde175539f3

running 1 test
test tests::it_works ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

It's an executable. I can even run it manually.

Actually I think, for unit tests, cargo-miri should build the entire proc-macro crate and use Miri to interpret it like normal crates, not building an executable with machine code. (That is, cargo_miri_bug-90df6bde175539f3 should be a JSON file, but it's currently not.)

[RalfJung]

Interesting, yet another corner case for the collection. Looks like our heuristics in cargo-miri need some more tweaking. Thanks for the minimal testcase!

[ghost]

Just noticed that this issue disappeared when I passed --target thumbv7a-pc-windows-msvc to cargo miri test (as shown in the output I pasted, I'm on x86_64-unknown-linux-gnu, not sure if this happens with --target x86_64-unknown-linux-gnu or other targets).

[RalfJung]

What happens when you pass --target x86_64-unknown-linux-gnu? (Never mind that this is your default target; this is not equivalent for cargo.)

[ghost]

What happens when you pass --target x86_64-unknown-linux-gnu? (Never mind that this is your default target; this is not equivalent for cargo.)

It's same: fatal error: file ... contains outdated or invalid JSON; try `cargo clean`.

[ghost]

Just noticed that this issue disappeared when I passed --target thumbv7a-pc-windows-msvc to cargo miri test (as shown in the output I pasted, I'm on x86_64-unknown-linux-gnu, not sure if this happens with --target x86_64-unknown-linux-gnu or other targets).

Note that I noticed the unit tests were still built as an executable and was run by the operating system instead of Miri.

[ghost]

cargo miri test -v shows this:

[cargo-miri miri] RUSTC_WRAPPER="/path/to/my/home/dir/.rustup/toolchains/nightly-2021-01-09-x86_64-unknown-linux-gnu/bin/cargo-miri"
[cargo-miri miri] CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="/path/to/my/home/dir/.rustup/toolchains/nightly-2021-01-09-x86_64-unknown-linux-gnu/bin/cargo-miri"
[cargo-miri miri] RUSTDOC="/path/to/my/home/dir/.rustup/toolchains/nightly-2021-01-09-x86_64-unknown-linux-gnu/bin/cargo-miri"
[cargo-miri miri] "/path/to/my/home/dir/.rustup/toolchains/nightly-2021-01-09-x86_64-unknown-linux-gnu/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-v"
   Compiling cargo-miri-bug v0.1.0 (/path/to/the/dir/cargo-miri-bug)
     Running `/path/to/my/home/dir/.rustup/toolchains/nightly-2021-01-09-x86_64-unknown-linux-gnu/bin/cargo-miri rustc --crate-name cargo_miri_bug --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type proc-macro --emit=dep-info,link -C prefer-dynamic -C embed-bitcode=no -C debuginfo=2 -C metadata=ac340a69673b37af -C extra-filename=-ac340a69673b37af --out-dir /path/to/the/dir/cargo-miri-bug/target/debug/deps -C incremental=/path/to/the/dir/cargo-miri-bug/target/debug/incremental -L dependency=/path/to/the/dir/cargo-miri-bug/target/debug/deps --extern proc_macro`
     Running `/path/to/my/home/dir/.rustup/toolchains/nightly-2021-01-09-x86_64-unknown-linux-gnu/bin/cargo-miri rustc --crate-name cargo_miri_bug --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C prefer-dynamic -C embed-bitcode=no -C debuginfo=2 --test -C metadata=90df6bde175539f3 -C extra-filename=-90df6bde175539f3 --out-dir /path/to/the/dir/cargo-miri-bug/target/debug/deps -C incremental=/path/to/the/dir/cargo-miri-bug/target/debug/incremental -L dependency=/path/to/the/dir/cargo-miri-bug/target/debug/deps --extern proc_macro`
[cargo-miri rustc] "/path/to/my/home/dir/.rustup/toolchains/nightly-2021-01-09-x86_64-unknown-linux-gnu/bin/miri" "--crate-name" "cargo_miri_bug" "--edition=2018" "src/lib.rs" "--error-format=json" "--json=diagnostic-rendered-ansi" "--crate-type" "proc-macro" "--emit=dep-info,link" "-C" "prefer-dynamic" "-C" "embed-bitcode=no" "-C" "debuginfo=2" "-C" "metadata=ac340a69673b37af" "-C" "extra-filename=-ac340a69673b37af" "--out-dir" "/path/to/the/dir/cargo-miri-bug/target/debug/deps" "-C" "incremental=/path/to/the/dir/cargo-miri-bug/target/debug/incremental" "-L" "dependency=/path/to/the/dir/cargo-miri-bug/target/debug/deps" "--extern" "proc_macro"
[cargo-miri rustc] "/path/to/my/home/dir/.rustup/toolchains/nightly-2021-01-09-x86_64-unknown-linux-gnu/bin/miri" "--crate-name" "cargo_miri_bug" "--edition=2018" "src/lib.rs" "--error-format=json" "--json=diagnostic-rendered-ansi" "--emit=dep-info,link" "-C" "prefer-dynamic" "-C" "embed-bitcode=no" "-C" "debuginfo=2" "--test" "-C" "metadata=90df6bde175539f3" "-C" "extra-filename=-90df6bde175539f3" "--out-dir" "/path/to/the/dir/cargo-miri-bug/target/debug/deps" "-C" "incremental=/path/to/the/dir/cargo-miri-bug/target/debug/incremental" "-L" "dependency=/path/to/the/dir/cargo-miri-bug/target/debug/deps" "--extern" "proc_macro"
    Finished test [unoptimized + debuginfo] target(s) in 0.48s
     Running `/path/to/my/home/dir/.rustup/toolchains/nightly-2021-01-09-x86_64-unknown-linux-gnu/bin/cargo-miri /path/to/the/dir/cargo-miri-bug/target/debug/deps/cargo_miri_bug-90df6bde175539f3`
fatal error: file "/path/to/the/dir/cargo-miri-bug/target/debug/deps/cargo_miri_bug-90df6bde175539f3" contains outdated or invalid JSON; try `cargo clean`
error: test failed, to rerun pass '--lib'

It seems that this is caused by Cargo built these tests for the host target (without --target) and tried to execute them using CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER (rust-lang/cargo#4336 may be relevant - I'm confused that it's about cross-compiling, but compiling for non-host targets seems fine here).

cargo-miri could exec the file as a fallback if it's not JSON, though I'd prefer to let Miri interpret these tests (that would be a separate Cargo issue).

[RalfJung]

It seems that this is caused by Cargo built these tests for the host target (without --target) and tried to execute them using CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER

That sounds like a bug -- build scripts and proc macros are not executed using a CARGO_*_RUNNER, so I would expect the same to be true for anything else that "must run on the host".

And then there's this other point...

Actually I think, for unit tests, cargo-miri should build the entire proc-macro crate and use Miri to interpret it like normal crates, not building an executable with machine code.

I tend to agree; the expectation with cargo miri is that tests are run in Miri. #1675 does not achieve this, so this is another reason for not using that approach IMO.

OTOH, for things like cargo miri test --target something, there's not much we can do -- the proc-macro and its dependencies are only built for the host, while Miri is set up to run target things, so it would be impossible to run the proc-macro tests in Miri.

So maybe what we should do is something like

• Detect proc-macro tests.
• For these tests, if Miri is run for the host target, interpret them in Miri.
• Otherwise, show an error or warning that proc-macro tests are not supported for cross-interpretation.

Do you think that would make sense?

[ghost]

OTOH, for things like cargo miri test --target something, there's not much we can do -- the proc-macro and its dependencies are only built for the host, while Miri is set up to run target things, so it would be impossible to run the proc-macro tests in Miri.

It would be possible if Cargo builds them for the target.

So maybe what we should do is something like

• Detect proc-macro tests.
• For these tests, if Miri is run for the host target, interpret them in Miri.
• Otherwise, show an error or warning that proc-macro tests are not supported for cross-interpretation.

Do you think that would make sense?

That sounds reasonable. Detecting the tests and building them (and their dependencies) for the interpreter seems tricky though, as that's not what Cargo does by default.

[RalfJung]

It would be possible if Cargo builds them for the target.

Yeah. But it doesn't and it would make little sense for Miri to diverge from that I think. cargo miri test should behave like cargo test in that regard -- which I guess means always running tests on the host, and showing an error when that's not possible (i.e., when --target got passed).

I wonder if people even expect proc-macro unit tests to be run in Miri, given that proc-macros themselves are not run in Miri? Maybe as a first step we should just skip these tests entirely with an appropriate message (similar to what we currently do for doctests). Is that possible without cargo metadata shenanigans or other hacks?

[ghost]

Is that possible without cargo metadata shenanigans or other hacks?

The easiest way to do this is to make the "outdated or invalid JSON" error non-fatal, but I think that's undesirable. I'll think some more.

[RalfJung]

Right, that's one stage after the mistake happened... there is a prior phase_cargo_rustc invocation where we are generating that binary. Is there anything about that invocation that can tell us that it is a proc-macro unit test? Can you figure out the arguments cargo is passing for that invocation? (It's probably already in the log that you posted above, but I am not sure which line.)

Btw, speaking of the log... note that cargo will cache the rustc output. So when you change verbosity, you have to cargo clean to make sure that phase_cargo_rustc actually gets re-executed.

[ghost]

Right, that's one stage after the mistake happened... there is a prior phase_cargo_rustc invocation where we are generating that binary. Is there anything about that invocation that can tell us that it is a proc-macro unit test? Can you figure out the arguments cargo is passing for that invocation? (It's probably already in the log that you posted above, but I am not sure which line.)

I remember that I tried to detect it by searching for --extern proc_macro and --test in #1674. I think that will work.

[RalfJung]

I remember that I tried to detect it by searching for --extern proc_macro and --test

Yes, that sounds reasonable. We have to generate something in phase_cargo_rustc (since cargo will later try to run it), but it could be a JSON file which just indicates "not a test we can run", so later phase_cargo_run can show a proper message to the user.

Should it be an error or a warning though? Is there a way to tell cargo test to not run proc-macro tests?

[ghost]

Should it be an error or a warning though? Is there a way to tell cargo test to not run proc-macro tests?

AFAIK no (but I'm not super familiar with Cargo), so it should be a warning (or at least a non-fatal error).