From f163dd3c8499ef8648b11bc42928b8154197356c Mon Sep 17 00:00:00 2001 From: norepimorphism Date: Sat, 23 Sep 2023 19:03:12 -0400 Subject: [PATCH 1/2] take first steps toward build-std support --- README.md | 1 + cargo-miri/src/phases.rs | 46 +++++++++++---- cargo-miri/src/util.rs | 17 ++++++ src/bin/miri.rs | 57 ++++++++++++++----- test-cargo-miri/Cargo.toml | 1 + test-cargo-miri/issue-2705/.cargo/config.toml | 5 ++ test-cargo-miri/issue-2705/Cargo.toml | 5 ++ test-cargo-miri/issue-2705/ee.json | 18 ++++++ test-cargo-miri/issue-2705/src/lib.rs | 5 ++ 9 files changed, 132 insertions(+), 23 deletions(-) create mode 100644 test-cargo-miri/issue-2705/.cargo/config.toml create mode 100644 test-cargo-miri/issue-2705/Cargo.toml create mode 100644 test-cargo-miri/issue-2705/ee.json create mode 100644 test-cargo-miri/issue-2705/src/lib.rs diff --git a/README.md b/README.md index f6a023181d..35bc799db2 100644 --- a/README.md +++ b/README.md @@ -511,6 +511,7 @@ binaries, and as such worth documenting: perform verbose logging. * `MIRI_HOST_SYSROOT` is set by bootstrap to tell `cargo-miri` which sysroot to use for *host* operations. +* `MIRI_STD_AWARE_CARGO` is set by `cargo-miri` when Cargo has `build-std` set to tell the Miri driver not to use the value of `MIRI_SYSROOT` and to inject compiler flags typically used for building the Miri sysroot whenever a crate from the standard library is compiled. [testing-miri]: CONTRIBUTING.md#testing-the-miri-driver diff --git a/cargo-miri/src/phases.rs b/cargo-miri/src/phases.rs index 80ce632558..32ee299e6a 100644 --- a/cargo-miri/src/phases.rs +++ b/cargo-miri/src/phases.rs @@ -93,8 +93,30 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { let target = get_arg_flag_value("--target"); let target = target.as_ref().unwrap_or(host); - // We always setup. - let miri_sysroot = setup(&subcommand, target, &rustc_version, verbose); + let has_build_std = is_build_std_set(); + if has_build_std { + if env::var_os("MIRI_SYSROOT").is_some() { + println!("WARNING: `MIRI_SYSROOT` has no effect when Cargo has `build-std` enabled."); + } + if env::var_os("MIRI_NO_STD").is_some() { + println!("WARNING: `MIRI_NO_STD` has no effect when Cargo has `build-std` enabled."); + } + } + let cargo_cmd = match subcommand { + MiriCommand::Forward(s) => s, + MiriCommand::Setup => { + if has_build_std { + println!( + "NOTE: `cargo miri setup` is unnecessary when Cargo has `build-std` enabled. \ + libstd and friends will be built on-the-fly when `cargo miri run` or `cargo \ + miri test` is invoked." + ); + } else { + setup(&subcommand, target, &rustc_version, verbose); + } + return; // `cargo miri setup` stops here. + } + }; // Invoke actual cargo for the job, but with different flags. // We re-use `cargo test` and `cargo run`, which makes target and binary handling very easy but @@ -107,10 +129,6 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { .into_os_string() .into_string() .expect("current executable path is not valid UTF-8"); - let cargo_cmd = match subcommand { - MiriCommand::Forward(s) => s, - MiriCommand::Setup => return, // `cargo miri setup` stops here. - }; let metadata = get_cargo_metadata(); let mut cmd = cargo(); cmd.arg(&cargo_cmd); @@ -159,8 +177,12 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { // Forward all further arguments (not consumed by `ArgSplitFlagValue`) to cargo. cmd.args(args); - // Let it know where the Miri sysroot lives. - cmd.env("MIRI_SYSROOT", miri_sysroot); + if has_build_std { + cmd.env("MIRI_STD_AWARE_CARGO", "1"); + } else { + // Let it know where the Miri sysroot lives. + cmd.env("MIRI_SYSROOT", setup(&subcommand, target, &rustc_version, verbose)); + } // Set `RUSTC_WRAPPER` to ourselves. Cargo will prepend that binary to its usual invocation, // i.e., the first argument is `rustc` -- which is what we use in `main` to distinguish // the two codepaths. (That extra argument is why we prefer this over setting `RUSTC`.) @@ -606,8 +628,12 @@ pub fn phase_rustdoc(mut args: impl Iterator) { // which are disabled by default. We first need to enable them explicitly: cmd.arg("-Z").arg("unstable-options"); - // rustdoc needs to know the right sysroot. - cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); + if let Some(sysroot) = env::var_os("MIRI_SYSROOT") { + // rustdoc needs to know the right sysroot. + cmd.arg("--sysroot").arg(sysroot); + } else if env::var_os("MIRI_STD_AWARE_CARGO").is_none() { + show_error!("`MIRI_SYSROOT` must be set unless `MIRI_STD_AWARE_CARGO` is set"); + }; // make sure the 'miri' flag is set for rustdoc cmd.arg("--cfg").arg("miri"); diff --git a/cargo-miri/src/util.rs b/cargo-miri/src/util.rs index 6381a4db86..f214120ffd 100644 --- a/cargo-miri/src/util.rs +++ b/cargo-miri/src/util.rs @@ -252,3 +252,20 @@ pub fn debug_cmd(prefix: &str, verbose: usize, cmd: &Command) { } eprintln!("{prefix} running command: {cmd:?}"); } + +/// Checks if Cargo's build-std setting is enabled for one or more crates. +pub fn is_build_std_set() -> bool { + let cmd = cargo() + // TODO: remove the -Z argument when this subcommand is stabilized; + // see + .args(&["-Zunstable-options", "config", "get", "--format=json-value", "unstable.build-std"]) + .output() + .expect("failed to get Cargo config"); + // TODO: Cargo will exit with an error code if the `unstable.build-std` key is not set. Should + // we still return false if an unrelated error has occurred? + if !cmd.status.success() { + return false; + } + + cmd.stdout.get(0..2) == Some(b"[\"") +} diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 8f90e4566a..19241f4d94 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -242,21 +242,34 @@ fn run_compiler( callbacks: &mut (dyn rustc_driver::Callbacks + Send), ) -> ! { if target_crate { - // Miri needs a custom sysroot for target crates. - // If no `--sysroot` is given, the `MIRI_SYSROOT` env var is consulted to find where - // that sysroot lives, and that is passed to rustc. - let sysroot_flag = "--sysroot"; - if !args.iter().any(|e| e == sysroot_flag) { - // Using the built-in default here would be plain wrong, so we *require* - // the env var to make sure things make sense. - let miri_sysroot = env::var("MIRI_SYSROOT").unwrap_or_else(|_| { - show_error!( - "Miri was invoked in 'target' mode without `MIRI_SYSROOT` or `--sysroot` being set" - ) + if env::var_os("MIRI_STD_AWARE_CARGO").is_some() { + let crate_name = parse_crate_name(&args).unwrap_or_else(|_| { + show_error!("`MIRI_STD_AWARE_CARGO` is set but `--crate-name` is not") }); + if matches!(crate_name, "std" | "core" | "alloc" | "panic_abort" | "test" | "compiler_builtins") { + // We are compiling the standard library. + args.extend(&["-Cdebug-assertions=off", "-Coverflow-checks=on"]); + } + if crate_name == "panic_abort" { + args.push("-Cpanic=abort".into()); + } + } else { + // Miri needs a custom sysroot for target crates. + // If no `--sysroot` is given, the `MIRI_SYSROOT` env var is consulted to find where + // that sysroot lives, and that is passed to rustc. + let sysroot_flag = "--sysroot"; + if !args.iter().any(|e| e == sysroot_flag) { + // Using the built-in default here would be plain wrong, so we *require* + // the env var to make sure things make sense. + let miri_sysroot = env::var("MIRI_SYSROOT").unwrap_or_else(|_| { + show_error!( + "Miri was invoked in 'target' mode without `MIRI_SYSROOT` or `--sysroot` being set" + ) + }); - args.push(sysroot_flag.to_owned()); - args.push(miri_sysroot); + args.push(sysroot_flag.to_owned()); + args.push(miri_sysroot); + } } } @@ -284,6 +297,24 @@ fn parse_comma_list(input: &str) -> Result, T::Err> { input.split(',').map(str::parse::).collect() } +fn parse_crate_name(args: &Vec) -> Option<&str> { + let mut iter = args.iter(); + while let Some(e) = iter.next() { + let Some(("", tail)) = e.split_once("--crate-name") else { + continue; + }; + if let Some(("", val)) = tail.split_once("=") { + return Some(val); + } else if tail == "" { + return Some(iter.next().as_ref().unwrap_or_else(|| { + show_error!("--crate-name is missing required argument") + })); + } + show_error!("--crate-name argument is invalid") + } + None +} + fn main() { let handler = EarlyErrorHandler::new(ErrorOutputType::default()); diff --git a/test-cargo-miri/Cargo.toml b/test-cargo-miri/Cargo.toml index 1688096fd9..29e01e9936 100644 --- a/test-cargo-miri/Cargo.toml +++ b/test-cargo-miri/Cargo.toml @@ -16,6 +16,7 @@ issue_1567 = { path = "issue-1567" } issue_1691 = { path = "issue-1691" } issue_1705 = { path = "issue-1705" } issue_1760 = { path = "issue-1760" } +issue_2705 = { path = "issue-2705" } issue_rust_86261 = { path = "issue-rust-86261" } [dev-dependencies] diff --git a/test-cargo-miri/issue-2705/.cargo/config.toml b/test-cargo-miri/issue-2705/.cargo/config.toml new file mode 100644 index 0000000000..7dc67e151a --- /dev/null +++ b/test-cargo-miri/issue-2705/.cargo/config.toml @@ -0,0 +1,5 @@ +[build] +target = "ee.json" + +[unstable] +build-std = ["core"] diff --git a/test-cargo-miri/issue-2705/Cargo.toml b/test-cargo-miri/issue-2705/Cargo.toml new file mode 100644 index 0000000000..c150549a9b --- /dev/null +++ b/test-cargo-miri/issue-2705/Cargo.toml @@ -0,0 +1,5 @@ +[package] +name = "issue_2705" +version = "0.1.0" +authors = ["Miri Team"] +edition = "2018" diff --git a/test-cargo-miri/issue-2705/ee.json b/test-cargo-miri/issue-2705/ee.json new file mode 100644 index 0000000000..10ad1d0dfb --- /dev/null +++ b/test-cargo-miri/issue-2705/ee.json @@ -0,0 +1,18 @@ +{ + "arch": "mips", + "cpu": "mips2", + "data-layout": "e-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64", + "features": "+mips2,+soft-float", + "linker-flavor": "ld.lld", + "llvm-abiname": "o32", + "llvm-target": "mipsel-none-elf", + "max-atomic-width": 32, + "os": "none", + "panic-strategy": "abort", + "position-independent-executables": false, + "relro-level": "full", + "target-c-int-width": "32", + "target-endian": "little", + "target-pointer-width": "32", + "relocation-model": "static" +} diff --git a/test-cargo-miri/issue-2705/src/lib.rs b/test-cargo-miri/issue-2705/src/lib.rs new file mode 100644 index 0000000000..64633490f8 --- /dev/null +++ b/test-cargo-miri/issue-2705/src/lib.rs @@ -0,0 +1,5 @@ +use byteorder::{ByteOrder, LittleEndian}; + +pub fn use_the_dependency() { + let _n = ::read_u32(&[1, 2, 3, 4]); +} From 01b95d827b4dfa2495577be10d3a437cab2a9376 Mon Sep 17 00:00:00 2001 From: norepimorphism Date: Sat, 23 Sep 2023 21:15:08 -0400 Subject: [PATCH 2/2] oops that was embarrassing --- cargo-miri/src/phases.rs | 4 ++-- src/bin/miri.rs | 26 +++++++++++++++++++++----- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/cargo-miri/src/phases.rs b/cargo-miri/src/phases.rs index 32ee299e6a..6e83fe2110 100644 --- a/cargo-miri/src/phases.rs +++ b/cargo-miri/src/phases.rs @@ -103,7 +103,7 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { } } let cargo_cmd = match subcommand { - MiriCommand::Forward(s) => s, + MiriCommand::Forward(ref s) => s, MiriCommand::Setup => { if has_build_std { println!( @@ -131,7 +131,7 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { .expect("current executable path is not valid UTF-8"); let metadata = get_cargo_metadata(); let mut cmd = cargo(); - cmd.arg(&cargo_cmd); + cmd.arg(cargo_cmd); // In nextest we have to also forward the main `verb`. if cargo_cmd == "nextest" { cmd.arg( diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 19241f4d94..17ef5fee8f 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -243,13 +243,28 @@ fn run_compiler( ) -> ! { if target_crate { if env::var_os("MIRI_STD_AWARE_CARGO").is_some() { - let crate_name = parse_crate_name(&args).unwrap_or_else(|_| { + let crate_name = parse_crate_name(&args).unwrap_or_else(|| { show_error!("`MIRI_STD_AWARE_CARGO` is set but `--crate-name` is not") - }); - if matches!(crate_name, "std" | "core" | "alloc" | "panic_abort" | "test" | "compiler_builtins") { + }) + .to_owned(); + // This list should be kept in sync with the one from + // `cargo/core/compiler/standard_lib.rs`. + if matches!( + crate_name.as_str(), + "std" + | "core" + | "alloc" + | "proc_macro" + | "panic_abort" + | "panic_unwind" + | "test" + | "compiler_builtins" + ) { // We are compiling the standard library. - args.extend(&["-Cdebug-assertions=off", "-Coverflow-checks=on"]); + args.push("-Cdebug-assertions=off".into()); + args.push("-Coverflow-checks=on".into()); } + // See `cargo_miri::phase_rustc`. if crate_name == "panic_abort" { args.push("-Cpanic=abort".into()); } @@ -297,6 +312,7 @@ fn parse_comma_list(input: &str) -> Result, T::Err> { input.split(',').map(str::parse::).collect() } +/// Extracts the value associated with the `--crate-name` argument from the given command line. fn parse_crate_name(args: &Vec) -> Option<&str> { let mut iter = args.iter(); while let Some(e) = iter.next() { @@ -310,7 +326,7 @@ fn parse_crate_name(args: &Vec) -> Option<&str> { show_error!("--crate-name is missing required argument") })); } - show_error!("--crate-name argument is invalid") + show_error!("--crate-name argument is invalid"); } None }