Auto merge of #1048 - Aaron1011:panic-rustup, r=<try>

Rustup for panic changes

This gets Miri working again, but doesn't actually implement unwinding

Author: bors

rust-version

@@ -1 +1 @@
-56237d75b4271a8a2e0f47d86ea76ebf6d966152
+a333eed7fc0c903df9d6befcfb40af02148bf255

src/bin/cargo-miri.rs

@@ -6,7 +6,7 @@ use std::path::{PathBuf, Path};
 use std::process::Command;
 use std::ops::Not;
 
-const XARGO_MIN_VERSION: (u32, u32, u32) = (0, 3, 17);
+const XARGO_MIN_VERSION: (u32, u32, u32) = (0, 3, 18);
 
 const CARGO_MIRI_HELP: &str = r#"Interprets bin crates and tests in Miri
 
@@ -80,7 +80,36 @@ fn get_arg_flag_value(name: &str) -> Option<String> {
     }
 }
 
-fn list_targets() -> impl Iterator<Item=cargo_metadata::Target> {
+fn is_build_dep(mut args: impl Iterator<Item = String>) -> bool {
+    args.any(|arg| arg.starts_with("--emit=") && arg.contains("link"))
+}
+
+// Returns whether or not Cargo invoked the wrapper (this binary) to compile
+// the final, target crate (either a test for 'cargo test', or a binary for 'cargo run')
+// Right now, this is an awful hack that checks several different pieces of information
+// to try to figure out if the crate being compiled is the right one.
+// Ideally, Cargo would set en environment variable indicating whether or
+// not the wrapper is being invoked on the target crate.
+// For now, this is the best we can do
+fn is_target_crate(is_build_script: bool) -> bool {
+    // Cargo sets this to the directory containing the manifest of the crate
+    // the wrapper is being invoekd to compile. This should be unique
+    // across the entire build (except for build scripts, which we handle below).
+    // We cannot check the crate name, since this may not be unique
+    // (e.g. if the build contains multiple versions of the same crate,
+    // or the same crate from multiple sources)
+    let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").ok();
+
+    // The manifest directory for our target crate. This is set by `cargo-miri`
+    // (the original invoation by the user) by using `cargo_metadata` to locate
+    // the manifest.
+    let expected_dir = std::env::var("MIRI_MAGIC_DIR").expect("MIRI_MAGIC_DIR not set!");
+
+    manifest_dir == Some(expected_dir) && !is_build_script
+
+}
+
+fn read_cargo_metadata() -> (impl Iterator<Item=cargo_metadata::Target>, String) {
     // We need to get the manifest, and then the metadata, to enumerate targets.
     let manifest_path = get_arg_flag_value("--manifest-path").map(|m|
         Path::new(&m).canonicalize().unwrap()
@@ -119,7 +148,7 @@ fn list_targets() -> impl Iterator<Item=cargo_metadata::Target> {
     let package = metadata.packages.remove(package_index);
 
     // Finally we got the list of targets to build
-    package.targets.into_iter()
+    (package.targets.into_iter(), metadata.workspace_root.to_string_lossy().to_string())
 }
 
 /// Returns the path to the `miri` binary
@@ -266,7 +295,7 @@ fn setup(ask_user: bool) {
             show_error(format!("Your xargo is too old; please upgrade to the latest version"))
         }
         let mut cmd = cargo();
-        cmd.args(&["install", "xargo", "-f"]);
+        cmd.args(&["install", "-f", "--git", "https://github.com/Aaron1011/xargo", "--branch", "feature/cargo-mode"]);
         ask_to_run(cmd, ask_user, "install a recent enough xargo");
     }
 
@@ -294,6 +323,7 @@ fn setup(ask_user: bool) {
     // The interesting bit: Xargo.toml
     File::create(dir.join("Xargo.toml")).unwrap()
         .write_all(br#"
+cargo_mode = "check"
 [dependencies.std]
 default_features = false
 # We need the `panic_unwind` feature because we use the `unwind` panic strategy.
@@ -318,7 +348,12 @@ path = "lib.rs"
     let target = get_arg_flag_value("--target");
     let print_sysroot = !ask_user && has_arg_flag("--print-sysroot"); // whether we just print the sysroot path
     let mut command = xargo();
-    command.arg("build").arg("-q");
+    // This may seen somewhat suprising - we are 'building' libstd
+    // by running (the equivalent of) `cargo check`. It turns out
+    // that `cargo check` has exactly the behavior that we want:
+    // it emits crate metadata (including MIR) without running any
+    // codegen.
+    command.arg("check").arg("-q");
     command.current_dir(&dir);
     command.env("RUSTFLAGS", miri::miri_default_args().join(" "));
     command.env("XARGO_HOME", dir.to_str().unwrap());
@@ -336,6 +371,7 @@ path = "lib.rs"
         show_error(format!("Failed to run xargo"));
     }
 
+
     // That should be it! But we need to figure out where xargo built stuff.
     // Unfortunately, it puts things into a different directory when the
     // architecture matches the host.
@@ -404,8 +440,10 @@ fn in_cargo_miri() {
         return;
     }
 
+    let (targets, root_dir) = read_cargo_metadata();
+
     // Now run the command.
-    for target in list_targets() {
+    for target in targets {
         let mut args = std::env::args().skip(skip);
         let kind = target.kind.get(0).expect(
             "badly formatted cargo metadata: target::kind is an empty array",
@@ -414,7 +452,7 @@ fn in_cargo_miri() {
         // change to add additional arguments. `FLAGS` is set to identify
         // this target.  The user gets to control what gets actually passed to Miri.
         let mut cmd = cargo();
-        cmd.arg("rustc");
+        cmd.arg("check");
         match (subcommand, kind.as_str()) {
             (MiriCommand::Run, "bin") => {
                 // FIXME: we just run all the binaries here.
@@ -441,14 +479,17 @@ fn in_cargo_miri() {
             }
             cmd.arg(arg);
         }
-        // Add `--` (to end the `cargo` flags), and then the user flags. We add markers around the
-        // user flags to be able to identify them later.  "cargo rustc" adds more stuff after this,
-        // so we have to mark both the beginning and the end.
-        cmd
-            .arg("--")
-            .arg("cargo-miri-marker-begin")
-            .args(args)
-            .arg("cargo-miri-marker-end");
+
+        let mut prefixed_args = String::new();
+        for arg in args {
+            prefixed_args += &arg.len().to_string();
+            prefixed_args.push(';');
+            prefixed_args += &arg;
+        }
+
+        cmd.env("MIRI_MAGIC_ARGS", prefixed_args);
+        cmd.env("MIRI_MAGIC_DIR", root_dir.clone());
+
         let path = std::env::current_exe().expect("current executable path invalid");
         cmd.env("RUSTC_WRAPPER", path);
         if verbose {
@@ -470,27 +511,41 @@ fn in_cargo_miri() {
 fn inside_cargo_rustc() {
     let sysroot = std::env::var("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT");
 
+
     let rustc_args = std::env::args().skip(2); // skip `cargo rustc`
-    let mut args: Vec<String> = rustc_args
-        .chain(Some("--sysroot".to_owned()))
-        .chain(Some(sysroot))
-        .collect();
-    args.splice(0..0, miri::miri_default_args().iter().map(ToString::to_string));
+
+    let in_build_script = is_build_dep(std::env::args().skip(2));
+
+    let mut args = if in_build_script {
+        rustc_args.collect()
+    } else {
+        let mut args: Vec<String> = rustc_args
+            .chain(Some("--sysroot".to_owned()))
+            .chain(Some(sysroot))
+            .collect();
+        args.splice(0..0, miri::miri_default_args().iter().map(ToString::to_string));
+        args
+    };
 
     // See if we can find the `cargo-miri` markers. Those only get added to the binary we want to
     // run. They also serve to mark the user-defined arguments, which we have to move all the way
     // to the end (they get added somewhere in the middle).
-    let needs_miri = if let Some(begin) = args.iter().position(|arg| arg == "cargo-miri-marker-begin") {
-        let end = args
-            .iter()
-            .position(|arg| arg == "cargo-miri-marker-end")
-            .expect("cannot find end marker");
-        // These mark the user arguments. We remove the first and last as they are the markers.
-        let mut user_args = args.drain(begin..=end);
-        assert_eq!(user_args.next().unwrap(), "cargo-miri-marker-begin");
-        assert_eq!(user_args.next_back().unwrap(), "cargo-miri-marker-end");
-        // Collect the rest and add it back at the end.
-        let mut user_args = user_args.collect::<Vec<String>>();
+    let needs_miri = if is_target_crate(in_build_script) {
+        let raw_args = std::env::var("MIRI_MAGIC_ARGS").expect("Missing magic!");
+        let mut user_args = vec![];
+        let mut slice = raw_args.as_str();
+        loop {
+            match slice.find(';') {
+                Some(pos) => {
+                    let len: usize = slice[..(pos)].parse().unwrap();
+                    let arg = slice[(pos+1)..=(pos+len)].to_string();
+                    user_args.push(arg);
+                    slice = &slice[(pos+len+1)..];
+                },
+                None => break
+            }
+        }
+
         args.append(&mut user_args);
         // Run this in Miri.
         true

src/eval.rs

@@ -213,7 +213,7 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) {
             };
             e.print_backtrace();
             if let Some(frame) = ecx.stack().last() {
-                let block = &frame.body.basic_blocks()[frame.block];
+                let block = &frame.body.basic_blocks()[frame.block.unwrap()];
                 let span = if frame.stmt < block.statements.len() {
                     block.statements[frame.stmt].source_info.span
                 } else {

src/machine.rs

@@ -173,6 +173,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
         args: &[OpTy<'tcx, Tag>],
         dest: Option<PlaceTy<'tcx, Tag>>,
         ret: Option<mir::BasicBlock>,
+        _unwind: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> {
         ecx.find_fn(instance, args, dest, ret)
     }
@@ -194,8 +195,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
         span: Span,
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx, Tag>],
-        dest: PlaceTy<'tcx, Tag>,
+        dest: Option<PlaceTy<'tcx, Tag>>,
+        _ret: Option<mir::BasicBlock>,
+        _unwind: Option<mir::BasicBlock>
     ) -> InterpResult<'tcx> {
+        let dest = match dest {
+            Some(dest) => dest,
+            None => throw_ub!(Unreachable)
+        };
         ecx.call_intrinsic(span, instance, args, dest)
     }
 
@@ -353,13 +360,15 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
     fn stack_pop(
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
         extra: stacked_borrows::CallId,
-    ) -> InterpResult<'tcx> {
-        Ok(ecx
+        _unwinding: bool
+    ) -> InterpResult<'tcx, StackPopInfo> {
+        ecx
             .memory
             .extra
             .stacked_borrows
             .borrow_mut()
-            .end_call(extra))
+            .end_call(extra);
+        Ok(StackPopInfo::Normal)
     }
 
     #[inline(always)]

src/shims/foreign_items.rs

@@ -335,7 +335,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     mir,
                     Some(ret_place),
                     // Directly return to caller.
-                    StackPopCleanup::Goto(Some(ret)),
+                    StackPopCleanup::Goto { ret: Some(ret), unwind: None },
                 )?;
                 let mut args = this.frame().body.args_iter();
 

src/shims/intrinsics.rs

@@ -22,7 +22,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         dest: PlaceTy<'tcx, Tag>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
-        if this.emulate_intrinsic(span, instance, args, dest)? {
+        if this.emulate_intrinsic(span, instance, args, Some(dest))? {
             return Ok(());
         }
         let tcx = &{this.tcx.tcx};

src/shims/mod.rs

@@ -27,7 +27,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         );
 
         // First, run the common hooks also supported by CTFE.
-        if this.hook_fn(instance, args, dest)? {
+        if this.hook_panic_fn(instance, args, dest)? {
             this.goto_block(ret)?;
             return Ok(None);
         }