Passing argument without -- to cargo miri run *bypasses* miri

[pnkfelix]

Check this out:

% cat src/main.rs
use std::ffi::c_char;

extern "C" {
    fn printf(fmt: *const c_char, ...);
}

fn main() {
    dbg!(std::env::args().skip(1).take(1).collect::<String>());
    let fmt = b"Hello, world!\n\0".as_ptr() as *const c_char;
    unsafe {
        printf(fmt);
    }
}

You'd expect miri run ... to error on the above code.

But:

% cargo +nightly miri run whoa
Preparing a sysroot for Miri (target: x86_64-unknown-linux-gnu)... done
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `target/debug/hello whoa --target-dir /local/home/pnkfelix/Dev/Rust/hello/target/miri --target x86_64-unknown-linux-gnu --config 'target.'\''cfg(all())'\''.runner=["/local/home/pnkfelix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri", '\''runner'\'']' --`
[src/main.rs:8] std::env::args().skip(1).take(1).collect::<String>() = "whoa"
Hello, world!

Now, if someone is reading the above output properly, they might get a hint as to what's actually happening here: it's building and running a debug binary, rather than running cargo-miri itself.

(I.e., you need to pass inputs with an intervening --, like so: cargo +nightly miri run -- whoa, which does what you expect and issues a error at the point where printf is called.)

But what is the scenario where you want the current behavior when -- is omitted? Should miri run provide an error or warning when -- is omitted and the extra arguments are not recognized by miri's argument processor?

[RalfJung]

I am confused, probably because I am tired... what is happening here? All these arguments should just be forwarded to cargo run, while a whole bunch of env vars are being set to make binaries be executed by Miri.

[oli-obk]

The issue is that the actual binary gets executed:

Running `target/debug/hello

And not miri

I don't know how a binary even gets created (maybe left over from a previous non-miri run?).

[RalfJung]

This is caused by very strange cargo behavior: in cargo run foo --config blah, cargo ignores the --config and probably passes it to the program instead. I would expect all --flags before the -- to be interpreted by cargo.

With this cargo behavior, I don't know if the bug is fixable. It is not possible for Miri to know whether an argument is present before the --; that would require knowing whether some --flag takes a parameter or not. In particular cargo run arg -- is a big problem: cargo will currently probably pass the -- to the program, so how is cargo-miri supposed to know whether it needs to add a -- or not? Also cargo run arg --target-dir=foo, since Miri needs to patch the target-dir but we have no way of knowing whether this flag is meant for cargo or the program.

We'd have to completely re-parse the arguments and carefully keep that in sync with cargo's parser, which is quite terrible. I would hope the API to be more compositional than that...

Cc @ehuss

[ehuss]

When cargo run sees a standalone argument, that disables argument parsing for all following arguments. This is a standard behavior of clap (and other libraries).

I would suggest structuring the way arguments are forwarded in the following way:

1. Starting with cmd = cargo().

2. Add the cargo command cmd.arg("run").

3. Add anything that is cargo-specific, such as cmd.arg("--config"), etc.

4. Scan the args for anything that is cargo-specific, but that cargo-miri wants to know about (like --target-dir). If you find these special flags, strip them from the args, and add them to cmd.arg(…). You'll need to collect the args into a temporary Vec to do this.

5. After you have filtered the args, pass the args list as-is to cmd.arg(…).

The current structure of forwarding the args before adding cargo-specific flags is the cause of the problem.

Let me know if that isn't clear or you have any questions.

[RalfJung]

When we passed the arguments in a different order, that broke cargo nextest 😂 .

But anyway the main problem is this:

Scan the args for anything that is cargo-specific, but that cargo-miri wants to know about (like --target-dir). If you find these special flags, strip them from the args, and add them to cmd.arg(…). You'll need to collect the args into a temporary Vec to do this.

Here we have to stop at the first argument that is passed to the program. So e.g. cargo miri run -- foo --target-dir=blah, we need to stop at the --. That already works. But with cargo miri run foo --target-dir=blah we need to stop at the foo. Worse, with cargo miri run --flag foo --target-dir=blah we need to stop at the foo if and only if --flag does not take a 'value'. In other words, we need to copy-paste the entire cargo argument parsing logic (at least which flags exist and whether they they an argument or not). That's quite bad. :(

[ehuss]

I personally wouldn't worry too much about having cargo miri run perfectly match the behavior of cargo run. If the concern is about these flags colliding with the user's program, I also wouldn't worry too much about that since they can use -- to avoid those collisions (and they seem like they would be unlikely).

An alternative is to use clap in cargo-miri, and just parse the cargo run flags. There aren't very many of them, and they don't change very often. That's what cargo expand does. I realize that isn't ideal, but otherwise there aren't a lot of options.

[RalfJung]

An alternative is to use clap in cargo-miri, and just parse the cargo run flags. There aren't very many of them, and they don't change very often. That's what cargo expand does. I realize that isn't ideal, but otherwise there aren't a lot of options.

I was living under the illusion that argument parsing worked out in a way that was just modular enough to make this work, but as this issue shows that was just wrong. :(

---

If we are willing to ignore the issues around cargo miri run foo --target-dir=blah, things are still not entirely trivial: if we add our --config immediately after the 'verb', we end up with cargo nextest --config ... run, which errors. That's why we moved to passing the --config at the end in the first place.