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

ghost · 2020-12-30T13:32:01Z

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.)

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

RalfJung · 2020-12-30T13:37:52Z

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 · 2020-12-30T13:41:30Z

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 · 2020-12-30T13:51:10Z

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 · 2020-12-30T14:01:18Z

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 · 2020-12-30T14:52:53Z

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 · 2021-01-15T17:58:45Z

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 · 2021-01-15T20:56:27Z

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 · 2021-01-15T21:11:56Z

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 · 2021-01-17T19:26:49Z

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 · 2021-01-18T09:45:28Z

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 · 2021-01-18T10:07:52Z

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 · 2021-01-18T10:13:22Z

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 · 2021-01-18T10:18:13Z

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 · 2021-01-18T10:21:18Z

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).

RalfJung added A-cargo Area: affects the cargo wrapper (cargo miri) C-bug Category: This is a bug. labels Dec 30, 2020

This was referenced Jan 15, 2021

Build proc-macro crate as "target" crate when unit testing it #1674

Closed

[cargo-miri] Skip unit tests of proc-macro crates #1675

Merged

ghost mentioned this issue Jan 16, 2021

[cargo-miri] Interpret unit tests of proc-macro crates #1677

Closed

bors closed this as completed in 853254f Jan 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ghost commented Dec 30, 2020 •

edited by ghost

Loading

RalfJung commented Dec 30, 2020

ghost commented Dec 30, 2020

RalfJung commented Dec 30, 2020

ghost commented Dec 30, 2020 •

edited by ghost

Loading

ghost commented Dec 30, 2020 •

edited by ghost

Loading

ghost commented Jan 15, 2021 •

edited by ghost

Loading

RalfJung commented Jan 15, 2021

ghost commented Jan 15, 2021 •

edited by ghost

Loading

RalfJung commented Jan 17, 2021

ghost commented Jan 18, 2021

RalfJung commented Jan 18, 2021

ghost commented Jan 18, 2021

RalfJung commented Jan 18, 2021

ghost commented Jan 18, 2021 •

edited by ghost

Loading

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

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

Comments

ghost commented Dec 30, 2020 • edited by ghost Loading

RalfJung commented Dec 30, 2020

ghost commented Dec 30, 2020

RalfJung commented Dec 30, 2020

ghost commented Dec 30, 2020 • edited by ghost Loading

ghost commented Dec 30, 2020 • edited by ghost Loading

ghost commented Jan 15, 2021 • edited by ghost Loading

RalfJung commented Jan 15, 2021

ghost commented Jan 15, 2021 • edited by ghost Loading

RalfJung commented Jan 17, 2021

ghost commented Jan 18, 2021

RalfJung commented Jan 18, 2021

ghost commented Jan 18, 2021

RalfJung commented Jan 18, 2021

ghost commented Jan 18, 2021 • edited by ghost Loading

ghost commented Dec 30, 2020 •

edited by ghost

Loading

ghost commented Dec 30, 2020 •

edited by ghost

Loading

ghost commented Dec 30, 2020 •

edited by ghost

Loading

ghost commented Jan 15, 2021 •

edited by ghost

Loading

ghost commented Jan 15, 2021 •

edited by ghost

Loading

ghost commented Jan 18, 2021 •

edited by ghost

Loading