some better error handling, check-only builds, fewer UTF-8 assumptions

Author: RalfJung

cargo-miri/bin.rs

@@ -1,5 +1,5 @@
 use std::env;
-use std::ffi::OsString;
+use std::ffi::{OsStr, OsString};
 use std::fs::{self, File};
 use std::io::{self, BufRead, BufReader, BufWriter, Write};
 use std::ops::Not;
@@ -81,27 +81,29 @@ fn show_error(msg: String) -> ! {
 
 // Determines whether a `--flag` is present.
 fn has_arg_flag(name: &str) -> bool {
-    let mut args = std::env::args().take_while(|val| val != "--");
-    args.any(|val| val == name)
+    let mut args = std::env::args_os().take_while(|val| val.to_str() != Some("--"));
+    args.any(|val| val.to_str() == Some(name))
 }
 
 /// Gets the value of a `--flag`.
 fn get_arg_flag_value(name: &str) -> Option<String> {
     // Stop searching at `--`.
-    let mut args = std::env::args().take_while(|val| val != "--");
+    let mut args = std::env::args_os().take_while(|val| val.to_str() != Some("--"));
     loop {
         let arg = match args.next() {
             Some(arg) => arg,
             None => return None,
         };
-        if !arg.starts_with(name) {
+        if !arg.to_string_lossy().starts_with(name) {
             continue;
         }
+        let arg = arg.to_str().expect("non-UTF-8 value not supported for flag we have to work with");
         // Strip leading `name`.
         let suffix = &arg[name.len()..];
         if suffix.is_empty() {
             // This argument is exactly `name`; the next one is the value.
-            return args.next();
+            let val = args.next().expect("value missing for flag");
+            return Some(val.into_string().expect("non-UTF-8 value not supported for flag we have to work with"));
         } else if suffix.starts_with('=') {
             // This argument is `name=value`; get the value.
             // Strip leading `=`.
@@ -420,7 +422,7 @@ fn phase_cargo_miri(mut args: env::ArgsOs) {
     // Run cargo.
     if verbose {
         cmd.env("MIRI_VERBOSE", ""); // this makes `inside_cargo_rustc` verbose.
-        eprintln!("+ {:?}", cmd);
+        eprintln!("[cargo-miri miri] {:?}", cmd);
     }
     exec(cmd)
 }
@@ -470,19 +472,41 @@ fn phase_cargo_rustc(args: env::ArgsOs) {
             // (and cargo passes this before the filename so it should be unique)
             get_arg_flag_value("extra-filename").unwrap_or(String::new()),
         ));
+        if verbose {
+            eprintln!("[cargo-miri rustc] writing run info to {:?}", path.display());
+        }
 
         let file = File::create(&path)
-            .unwrap_or_else(|_| show_error(format!("Cannot create {}", path.display())));
+            .unwrap_or_else(|_| show_error(format!("Cannot create {:?}", path.display())));
         let file = BufWriter::new(file);
         serde_json::ser::to_writer(file, &info)
-            .unwrap_or_else(|_| show_error(format!("Cannot write to {}", path.display())));
+            .unwrap_or_else(|_| show_error(format!("Cannot write to {:?}", path.display())));
         return;
     }
 
     let mut cmd = miri();
-    // Forward arguments.
-    cmd.args(args);
-    // FIXME: Make the build check-only!
+    // Forward arguments, but (only for target crates!) remove "link" from "--emit" to make this a check-only build.
+    let emit_flag = "--emit";
+    for arg in args {
+        if target_crate && arg.to_string_lossy().starts_with(emit_flag) {
+            // Patch this argument. First, extract its value.
+            let arg = arg.to_str().expect("`--emit` must be UTF-8");
+            let val = &arg[emit_flag.len()..];
+            assert!(val.starts_with("="), "`cargo` should pass `--emit=X` as one argument");
+            let val = &val[1..];
+            let mut val: Vec<_> = val.split(',').collect();
+            // Now make sure "link" is not in there, but "metadata" is.
+            if let Some(i) = val.iter().position(|&s| s == "link") {
+                val.remove(i);
+                if !val.iter().any(|&s| s == "metadata") {
+                    val.push("metadata");
+                }
+            }
+            cmd.arg(format!("{}={}", emit_flag, val.join(",")));
+        } else {
+            cmd.arg(arg);
+        }
+    }
 
     // We make sure to only specify our custom Xargo sysroot for target crates - that is,
     // crates which are needed for interpretation by Miri. proc-macros and build scripts
@@ -500,36 +524,50 @@ fn phase_cargo_rustc(args: env::ArgsOs) {
 
     // Run it.
     if verbose {
-        eprintln!("+ {:?}", cmd);
+        eprintln!("[cargo-miri rustc] {:?}", cmd);
     }
     exec(cmd)
 }
 
-fn phase_cargo_runner(binary: &str, args: env::ArgsOs) {
+fn phase_cargo_runner(binary: &OsStr, binary_args: env::ArgsOs) {
     let verbose = std::env::var_os("MIRI_VERBOSE").is_some();
 
     let file = File::open(binary)
-        .unwrap_or_else(|_| show_error(format!("File {:?} not found, or cargo-miri invoked incorrectly", binary)));
+        .unwrap_or_else(|_| show_error(format!("File {:?} not found or `cargo-miri` invoked incorrectly; please only invoke this binary through `cargo miri`", binary)));
     let file = BufReader::new(file);
     let info: CrateRunInfo = serde_json::from_reader(file)
         .unwrap_or_else(|_| show_error(format!("File {:?} does not contain valid JSON", binary)));
-    // FIXME: remove the file.
+    fs::remove_file(binary)
+        .unwrap_or_else(|_| show_error(format!("Unable to remove file {:?}", binary)));
 
     let mut cmd = miri();
-    // Forward rustc arguments,with our sysroot.
-    cmd.args(info.args);
+    // Forward rustc arguments, with our sysroot. We need to patch "--extern" filenames because
+    // we forced a check-only build without cargo knowing about that: replace `.rlib` suffix by `.rmeta`.
+    let mut args = info.args.into_iter();
+    let extern_flag = "--extern";
+    while let Some(arg) = args.next() {
+        if arg.to_str() == Some(extern_flag) {
+            let next_arg = args.next().expect("`--extern` shoulw be followed by a filename");
+            let next_arg = next_arg.to_str().expect("Cannot work with non-UTF-8 extern filenames");
+            let next_arg = next_arg.strip_suffix(".rlib").expect("all extern filenames should end in `.rlib`");
+            cmd.arg(extern_flag);
+            cmd.arg(format!("{}.rmeta", next_arg));
+        } else {
+            cmd.arg(arg);
+        }
+    }
     let sysroot =
         env::var_os("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT");
     cmd.arg("--sysroot");
     cmd.arg(sysroot);
 
     // Then pass binary arguments.
     cmd.arg("--");
-    cmd.args(args);
+    cmd.args(binary_args);
 
     // Run it.
     if verbose {
-        eprintln!("+ {:?}", cmd);
+        eprintln!("[cargo-miri runner] {:?}", cmd);
     }
     exec(cmd)
 }
@@ -556,10 +594,10 @@ fn main() {
     //   binary crates for later interpretation.
     // - When we are executed due to CARGO_TARGET_RUNNER, we start interpretation based on the
     //   flags that were stored earlier.
-    // FIXME: report errors for these unwraps.
-    match &*args.next().unwrap().to_str().unwrap() {
-        "miri" => phase_cargo_miri(args),
-        "rustc" => phase_cargo_rustc(args),
-        binary => phase_cargo_runner(binary, args),
+    match args.next().as_deref() {
+        Some(s) if s.to_str() == Some("miri") => phase_cargo_miri(args),
+        Some(s) if s.to_str() == Some("rustc") => phase_cargo_rustc(args),
+        Some(binary) => phase_cargo_runner(binary, args),
+        _ => show_error(format!("`cargo-miri` called without first argument; please only invoke this binary through `cargo miri`")),
     }
 }