Merge pull request #550 from RalfJung/cargo-miri-test

Fix cargo miri test

Author: RalfJung

rust-version

@@ -1 +1 @@
-nightly-2018-12-14
+nightly-2018-12-18

src/bin/cargo-miri.rs

@@ -171,8 +171,7 @@ fn setup(ask_user: bool) {
         } else {
             println!("Installing xargo: `cargo install xargo -f`");
         }
-        // FIXME: Go back to using releases, once a 0.3.13 got released.
-        if !Command::new("cargo").args(&["install", "xargo", "-f", "--git", "https://github.com/japaric/xargo"]).status().unwrap().success() {
+        if !Command::new("cargo").args(&["install", "xargo", "-f"]).status().unwrap().success() {
             show_error(format!("Failed to install xargo"));
         }
     }
@@ -310,9 +309,8 @@ fn main() {
                 (MiriCommand::Test, "lib") => {
                     // For libraries we call `cargo rustc -- --test <rustc args>`
                     // Notice now that `--test` is a rustc arg rather than a cargo arg. This tells
-                    // rustc to build a test harness which calls all #[test] functions. We don't
-                    // use the harness since we execute each #[test] function's MIR ourselves before
-                    // compilation even completes, but this option is necessary to build the library.
+                    // rustc to build a test harness which calls all #[test] functions.
+                    // We then execute that harness just like any other binary.
                     if let Err(code) = process(
                         vec!["--".to_string(), "--test".to_string()].into_iter().chain(
                             args,

src/bin/miri.rs

@@ -23,8 +23,6 @@ use rustc_metadata::cstore::CStore;
 use rustc_driver::{Compilation, CompilerCalls, RustcDefaultCalls};
 use rustc_driver::driver::{CompileState, CompileController};
 use rustc::session::config::{self, Input, ErrorOutputType};
-use rustc::hir::{self, itemlikevisit};
-use rustc::ty::TyCtxt;
 use rustc_codegen_utils::codegen_backend::CodegenBackend;
 use syntax::ast;
 
@@ -115,43 +113,12 @@ fn after_analysis<'a, 'tcx>(
 
     let tcx = state.tcx.unwrap();
 
-    if std::env::args().any(|arg| arg == "--test") {
-        struct Visitor<'a, 'tcx: 'a> {
-            tcx: TyCtxt<'a, 'tcx, 'tcx>,
-            state: &'a CompileState<'a, 'tcx>,
-            validate: bool,
-        };
-        impl<'a, 'tcx: 'a, 'hir> itemlikevisit::ItemLikeVisitor<'hir> for Visitor<'a, 'tcx> {
-            fn visit_item(&mut self, i: &'hir hir::Item) {
-                if let hir::ItemKind::Fn(.., body_id) = i.node {
-                    if i.attrs.iter().any(|attr| {
-                        attr.name() == "test"
-                    })
-                    {
-                        let did = self.tcx.hir().body_owner_def_id(body_id);
-                        println!(
-                            "running test: {}",
-                            self.tcx.def_path_debug_str(did),
-                        );
-                        miri::eval_main(self.tcx, did, self.validate);
-                        self.state.session.abort_if_errors();
-                    }
-                }
-            }
-            fn visit_trait_item(&mut self, _trait_item: &'hir hir::TraitItem) {}
-            fn visit_impl_item(&mut self, _impl_item: &'hir hir::ImplItem) {}
-        }
-        state.hir_crate.unwrap().visit_all_item_likes(
-            &mut Visitor { tcx, state, validate }
-        );
-    } else if let Some((entry_node_id, _, _)) = *state.session.entry_fn.borrow() {
-        let entry_def_id = tcx.hir().local_def_id(entry_node_id);
-        miri::eval_main(tcx, entry_def_id, validate);
-
-        state.session.abort_if_errors();
-    } else {
-        println!("no main function found, assuming auxiliary build");
-    }
+    let (entry_node_id, _, _) = state.session.entry_fn.borrow().expect("no main function found!");
+    let entry_def_id = tcx.hir().local_def_id(entry_node_id);
+
+    miri::eval_main(tcx, entry_def_id, validate);
+
+    state.session.abort_if_errors();
 }
 
 fn init_early_loggers() {

src/fn_call.rs

@@ -426,6 +426,7 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
                 let paths = &[
                     (&["libc", "_SC_PAGESIZE"], Scalar::from_int(4096, dest.layout.size)),
                     (&["libc", "_SC_GETPW_R_SIZE_MAX"], Scalar::from_int(-1, dest.layout.size)),
+                    (&["libc", "_SC_NPROCESSORS_ONLN"], Scalar::from_int(1, dest.layout.size)),
                 ];
                 let mut result = None;
                 for &(path, path_value) in paths {
@@ -452,6 +453,10 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
                 }
             }
 
+            "isatty" => {
+                this.write_null(dest)?;
+            }
+
             // Hook pthread calls that go to the thread-local storage memory subsystem
             "pthread_key_create" => {
                 let key_ptr = this.read_scalar(args[0])?.to_ptr()?;
@@ -508,10 +513,6 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
                 this.write_null(dest)?;
             }
 
-            "_tlv_atexit" => {
-                // FIXME: Register the dtor
-            },
-
             // Determining stack base address
             "pthread_attr_init" | "pthread_attr_destroy" | "pthread_attr_get_np" |
             "pthread_getattr_np" | "pthread_self" | "pthread_get_stacksize_np" => {
@@ -549,7 +550,18 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
                 this.write_null(dest)?;
             }
 
-            // Windows API subs
+            // macOS API stubs
+            "_tlv_atexit" => {
+                // FIXME: Register the dtor
+            },
+            "_NSGetArgc" => {
+                this.write_scalar(Scalar::Ptr(this.machine.argc.unwrap()), dest)?;
+            },
+            "_NSGetArgv" => {
+                this.write_scalar(Scalar::Ptr(this.machine.argv.unwrap()), dest)?;
+            },
+
+            // Windows API stubs
             "AddVectoredExceptionHandler" => {
                 // any non zero value works for the stdlib. This is just used for stackoverflows anyway
                 this.write_scalar(Scalar::from_int(1, dest.layout.size), dest)?;
@@ -571,8 +583,6 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
                 // this is c::ERROR_CALL_NOT_IMPLEMENTED
                 this.write_scalar(Scalar::from_int(120, dest.layout.size), dest)?;
             },
-
-            // Windows TLS
             "TlsAlloc" => {
                 // This just creates a key; Windows does not natively support TLS dtors.
 

src/lib.rs

@@ -121,7 +121,11 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
 
     // Second argument (argc): 1
     let dest = ecx.eval_place(&mir::Place::Local(args.next().unwrap()))?;
-    ecx.write_scalar(Scalar::from_int(1, dest.layout.size), dest)?;
+    let argc = Scalar::from_int(1, dest.layout.size);
+    ecx.write_scalar(argc, dest)?;
+    let argc_place = ecx.allocate(dest.layout, MiriMemoryKind::Env.into())?;
+    ecx.write_scalar(argc, argc_place.into())?;
+    ecx.machine.argc = Some(argc_place.ptr.to_ptr()?);
 
     // FIXME: extract main source file path
     // Third argument (argv): &[b"foo"]
@@ -132,7 +136,11 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
     let foo_place = ecx.allocate(foo_layout, MiriMemoryKind::Env.into())?;
     ecx.write_scalar(Scalar::Ptr(foo), foo_place.into())?;
     ecx.memory_mut().mark_immutable(foo_place.to_ptr()?.alloc_id)?;
-    ecx.write_scalar(foo_place.ptr, dest)?;
+    let argv = foo_place.ptr;
+    ecx.write_scalar(argv, dest)?;
+    let argv_place = ecx.allocate(dest.layout, MiriMemoryKind::Env.into())?;
+    ecx.write_scalar(argv, argv_place.into())?;
+    ecx.machine.argv = Some(argv_place.ptr.to_ptr()?);
 
     assert!(args.next().is_none(), "start lang item has more arguments than expected");
 
@@ -253,6 +261,11 @@ pub struct Evaluator<'tcx> {
     /// Miri does not expose env vars from the host to the emulated program
     pub(crate) env_vars: HashMap<Vec<u8>, Pointer<Borrow>>,
 
+    /// Program arguments (`Option` because we can only initialize them after creating the ecx).
+    /// These are *pointers* to argc/argv because macOS.
+    pub(crate) argc: Option<Pointer<Borrow>>,
+    pub(crate) argv: Option<Pointer<Borrow>>,
+
     /// TLS state
     pub(crate) tls: TlsData<'tcx>,
 
@@ -267,6 +280,8 @@ impl<'tcx> Evaluator<'tcx> {
     fn new(validate: bool) -> Self {
         Evaluator {
             env_vars: HashMap::default(),
+            argc: None,
+            argv: None,
             tls: TlsData::default(),
             validate,
             stacked_borrows: stacked_borrows::State::default(),

test-cargo-miri/run-test.py

@@ -7,11 +7,11 @@
 
 import sys, subprocess
 
-def test_cargo_miri():
-    print("==> Testing `cargo miri run` <==")
+def test(name, cmd, stdout_ref, stderr_ref):
+    print("==> Testing `{}` <==".format(name))
     ## Call `cargo miri`, capture all output
     p = subprocess.Popen(
-        ["cargo", "miri", "run", "-q"],
+        cmd,
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE
     )
@@ -26,17 +26,19 @@ def test_cargo_miri():
     # Test for failures
     if p.returncode != 0:
         sys.exit(1)
-    if stdout != open('stdout.ref').read():
+    if stdout != open(stdout_ref).read():
         print("stdout does not match reference")
         sys.exit(1)
-    if stderr != open('stderr.ref').read():
+    if stderr != open(stderr_ref).read():
         print("stderr does not match reference")
         sys.exit(1)
 
+def test_cargo_miri_run():
+    test("cargo miri run", ["cargo", "miri", "run", "-q"], "stdout.ref", "stderr.ref")
+
 def test_cargo_miri_test():
-    print("==> Testing `cargo miri test` <==")
-    subprocess.check_call(["cargo", "miri", "test"])
+    test("cargo miri test", ["cargo", "miri", "test", "-q"], "test.stdout.ref", "test.stderr.ref")
 
-test_cargo_miri()
+test_cargo_miri_run()
 test_cargo_miri_test()
 sys.exit(0)

test-cargo-miri/test.stderr.ref

@@ -0,0 +1,7 @@
+
+running 2 tests
+test bar ... ok
+test baz ... ok
+
+test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
+

test-cargo-miri/test.stdout.ref

@@ -2,3 +2,10 @@
 fn bar() {
     assert_eq!(4, 4);
 }
+
+// Having more than 1 test does seem to make a difference
+// (i.e., this calls ptr::swap which having just one test does not).
+#[test]
+fn baz() {
+    assert_eq!(5, 5);
+}

test-cargo-miri/tests/foo.rs

@@ -1,6 +1,3 @@
-// FIXME: Validation disabled due to https://github.com/rust-lang/rust/issues/54957
-// compile-flags: -Zmiri-disable-validation
-
 #[derive(PartialEq, Eq, PartialOrd, Ord)]
 pub enum Foo {
     A(&'static str),