miri isolation configuration using two flags

Instead of using a single flag for both to disable isolation and to
configure error behaviour, use a dedicated flag for each. For the
former, continue using `-Zmiri-disable-isolation`. For the latter,
replaced `-Zmiri-isolated-op` with `-Zmiri-isolation-error` flag to
control the error behaviour of miri when op is run in isolation.

Author: atsmtat

src/bin/miri.rs

@@ -203,6 +203,9 @@ fn main() {
     let mut miri_config = miri::MiriConfig::default();
     let mut rustc_args = vec![];
     let mut after_dashdash = false;
+
+    // If user has explicitly enabled isolation
+    let mut isolation_enabled: Option<bool> = None;
     for arg in env::args() {
         if rustc_args.is_empty() {
             // Very first arg: binary name.
@@ -228,8 +231,38 @@ fn main() {
                     miri_config.check_alignment = miri::AlignmentCheck::Symbolic;
                 }
                 "-Zmiri-disable-isolation" => {
+                    if let Some(true) = isolation_enabled {
+                        panic!(
+                            "-Zmiri-disable-isolation cannot be used along with -Zmiri-isolation-error"
+                        );
+                    } else {
+                        isolation_enabled = Some(false);
+                    }
                     miri_config.isolated_op = miri::IsolatedOp::Allow;
                 }
+                arg if arg.starts_with("-Zmiri-isolation-error=") => {
+                    if let Some(false) = isolation_enabled {
+                        panic!(
+                            "-Zmiri-isolation-error cannot be used along with -Zmiri-disable-isolation"
+                        );
+                    } else {
+                        isolation_enabled = Some(true);
+                    }
+
+                    miri_config.isolated_op = match arg
+                        .strip_prefix("-Zmiri-isolation-error=")
+                        .unwrap()
+                    {
+                        "abort" => miri::IsolatedOp::Reject(miri::RejectOpWith::Abort),
+                        "ignore" => miri::IsolatedOp::Reject(miri::RejectOpWith::NoWarning),
+                        "warn" => miri::IsolatedOp::Reject(miri::RejectOpWith::Warning),
+                        "warn-nobacktrace" =>
+                            miri::IsolatedOp::Reject(miri::RejectOpWith::WarningWithoutBacktrace),
+                        _ => panic!(
+                            "-Zmiri-isolation-error must be `abort`, `ignore`, `warn`, or `warn-nobacktrace`"
+                        ),
+                    };
+                }
                 "-Zmiri-ignore-leaks" => {
                     miri_config.ignore_leaks = true;
                 }
@@ -322,19 +355,6 @@ fn main() {
                     let measureme_out = arg.strip_prefix("-Zmiri-measureme=").unwrap();
                     miri_config.measureme_out = Some(measureme_out.to_string());
                 }
-                arg if arg.starts_with("-Zmiri-isolated-op=") => {
-                    let action = match arg.strip_prefix("-Zmiri-isolated-op=").unwrap() {
-                        "hide" => miri::IsolatedOp::Reject(miri::RejectOpWith::NoWarning),
-                        "warn" => miri::IsolatedOp::Reject(miri::RejectOpWith::Warning),
-                        "warn-nobacktrace" =>
-                            miri::IsolatedOp::Reject(miri::RejectOpWith::WarningWithoutBacktrace),
-                        "allow" => miri::IsolatedOp::Allow,
-                        _ => panic!(
-                            "-Zmiri-isolated-op must be `hide`, `warn`, `warn-nobacktrace`, or `allow`"
-                        ),
-                    };
-                    miri_config.isolated_op = action;
-                }
                 _ => {
                     // Forward to rustc.
                     rustc_args.push(arg);

src/diagnostics.rs

@@ -71,7 +71,7 @@ pub fn report_error<'tcx, 'mir>(
             #[rustfmt::skip]
             let helps = match info {
                 UnsupportedInIsolation(_) =>
-                    vec![format!("pass the flag `-Zmiri-disable-isolation` to disable isolation")],
+                    vec![format!("pass the flag `-Zmiri-disable-isolation` to disable isolation; or pass `-Zmiri-isolation-error=warn to configure miri to warn about isolation and continue instead of aborting")],
                 ExperimentalUb { url, .. } =>
                     vec![
                         format!("this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental"),

src/eval.rs

@@ -24,21 +24,27 @@ pub enum AlignmentCheck {
 
 #[derive(Copy, Clone, Debug, PartialEq)]
 pub enum RejectOpWith {
-    /// Do not print warning about rejected isolated op
+    /// Isolated op is rejected with an abort of the machine.
+    Abort,
+
+    /// If not Abort, miri returns an error for an isolated op.
+    /// Following options determine if user should be warned about such error.
+    /// Do not print warning about rejected isolated op.
     NoWarning,
 
-    /// Print a warning about rejected isolated op, with backtrace
+    /// Print a warning about rejected isolated op, with backtrace.
     Warning,
 
-    /// Print a warning about rejected isolated op, without backtrace
+    /// Print a warning about rejected isolated op, without backtrace.
     WarningWithoutBacktrace,
 }
 
 #[derive(Copy, Clone, Debug, PartialEq)]
 pub enum IsolatedOp {
-    /// Reject op requiring communication with the host. Usually, miri
-    /// returns an error code for such op, and prints a warning
-    /// about it. Warning levels are controlled by `RejectOpWith` enum.
+    /// Reject an op requiring communication with the host. By
+    /// default, miri rejects the op with an abort. If not, it returns
+    /// an error code, and prints a warning about it. Warning levels
+    /// are controlled by `RejectOpWith` enum.
     Reject(RejectOpWith),
 
     /// Execute op requiring communication with the host, i.e. disable isolation.
@@ -88,7 +94,7 @@ impl Default for MiriConfig {
             validate: true,
             stacked_borrows: true,
             check_alignment: AlignmentCheck::Int,
-            isolated_op: IsolatedOp::Reject(RejectOpWith::WarningWithoutBacktrace),
+            isolated_op: IsolatedOp::Reject(RejectOpWith::Abort),
             ignore_leaks: false,
             excluded_env_vars: vec![],
             args: vec![],

src/helpers.rs

@@ -392,28 +392,31 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     /// case.
     fn check_no_isolation(&self, name: &str) -> InterpResult<'tcx> {
         if !self.eval_context_ref().machine.communicate() {
-            isolation_error(name)?;
+            self.reject_in_isolation(name, RejectOpWith::Abort)?;
         }
         Ok(())
     }
 
     /// Helper function used inside the shims of foreign functions which reject the op
     /// when isolation is enabled. It is used to print a warning/backtrace about the rejection.
-    fn report_rejected_op(&self, op_name: &str, reject_with: RejectOpWith) {
+    fn reject_in_isolation(&self, op_name: &str, reject_with: RejectOpWith) -> InterpResult<'tcx> {
         let this = self.eval_context_ref();
         match reject_with {
+            RejectOpWith::Abort => isolation_abort_error(op_name),
             RejectOpWith::WarningWithoutBacktrace => {
                 let msg = format!("`{}` was made to return an error due to isolation", op_name);
                 let mut warn = this.tcx.sess.struct_warn(&msg);
-                warn.note("run with -Zmiri-isolated-op=warn to get warning with a backtrace");
-                warn.note("run with -Zmiri-isolated-op=hide to hide warning");
-                warn.note("run with -Zmiri-isolated-op=allow to disable isolation");
+                warn.note("run with -Zmiri-isolation-error=warn to get warning with a backtrace");
+                warn.note("run with -Zmiri-isolation-error=ignore to hide warning");
+                warn.note("run with -Zmiri-disable-isolation to disable isolation");
                 warn.emit();
+                Ok(())
             }
             RejectOpWith::Warning => {
                 register_diagnostic(NonHaltingDiagnostic::RejectedIsolatedOp(op_name.to_string()));
+                Ok(())
             }
-            RejectOpWith::NoWarning => {} // no warning
+            RejectOpWith::NoWarning => Ok(()), // no warning
         }
     }
 
@@ -664,10 +667,7 @@ pub fn check_abi<'a>(abi: Abi, exp_abi: Abi) -> InterpResult<'a, ()> {
     }
 }
 
-pub fn isolation_error(name: &str) -> InterpResult<'static> {
-    // FIXME: This function has been deprecated. It's only used for
-    // the ops which are not converted to return a proper error code
-    // in isolation.
+pub fn isolation_abort_error(name: &str) -> InterpResult<'static> {
     throw_machine_stop!(TerminationInfo::UnsupportedInIsolation(format!(
         "{} not available when isolation is enabled",
         name,

src/shims/env.rs

@@ -323,9 +323,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         );
 
         if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
+            this.reject_in_isolation("getcwd", reject_with)?;
             let err = Error::new(ErrorKind::NotFound, "rejected due to isolation");
             this.set_last_error_from_io_error(err)?;
-            this.report_rejected_op("getcwd", reject_with);
         } else {
             let buf = this.read_scalar(&buf_op)?.check_init()?;
             let size = this.read_scalar(&size_op)?.to_machine_usize(&*this.tcx)?;
@@ -355,9 +355,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         this.assert_target_os("windows", "GetCurrentDirectoryW");
 
         if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
+            this.reject_in_isolation("GetCurrentDirectoryW", reject_with)?;
             let err = Error::new(ErrorKind::NotFound, "rejected due to isolation");
             this.set_last_error_from_io_error(err)?;
-            this.report_rejected_op("GetCurrentDirectoryW", reject_with);
         } else {
             let size = u64::from(this.read_scalar(size_op)?.to_u32()?);
             let buf = this.read_scalar(buf_op)?.check_init()?;
@@ -383,9 +383,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         );
 
         if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
+            this.reject_in_isolation("chdir", reject_with)?;
             let err = Error::new(ErrorKind::NotFound, "rejected due to isolation");
             this.set_last_error_from_io_error(err)?;
-            this.report_rejected_op("chdir", reject_with);
 
             Ok(-1)
         } else {
@@ -412,9 +412,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         this.assert_target_os("windows", "SetCurrentDirectoryW");
 
         if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
+            this.reject_in_isolation("SetCurrentDirectoryW", reject_with)?;
             let err = Error::new(ErrorKind::NotFound, "rejected due to isolation");
             this.set_last_error_from_io_error(err)?;
-            this.report_rejected_op("SetCurrentDirectoryW", reject_with);
 
             Ok(0)
         } else {

src/shims/posix/fs.rs

@@ -127,7 +127,7 @@ impl FileDescriptor for io::Stdin {
     ) -> InterpResult<'tcx, io::Result<usize>> {
         if !communicate_allowed {
             // We want isolation mode to be deterministic, so we have to disallow all reads, even stdin.
-            helpers::isolation_error("`read` from stdin")?;
+            helpers::isolation_abort_error("`read` from stdin")?;
         }
         Ok(Read::read(self, bytes))
     }

tests/run-pass/current_dir_with_isolation.rs

@@ -1,3 +1,4 @@
+// compile-flags: -Zmiri-isolation-error=warn-nobacktrace
 use std::env;
 use std::io::ErrorKind;
 

tests/run-pass/current_dir_with_isolation.stderr

@@ -1,12 +1,12 @@
 warning: `getcwd` was made to return an error due to isolation
   |
-  = note: run with -Zmiri-isolated-op=warn to get warning with a backtrace
-  = note: run with -Zmiri-isolated-op=hide to hide warning
-  = note: run with -Zmiri-isolated-op=allow to disable isolation
+  = note: run with -Zmiri-isolation-error=warn to get warning with a backtrace
+  = note: run with -Zmiri-isolation-error=ignore to hide warning
+  = note: run with -Zmiri-disable-isolation to disable isolation
 
 warning: `chdir` was made to return an error due to isolation
   |
-  = note: run with -Zmiri-isolated-op=warn to get warning with a backtrace
-  = note: run with -Zmiri-isolated-op=hide to hide warning
-  = note: run with -Zmiri-isolated-op=allow to disable isolation
+  = note: run with -Zmiri-isolation-error=warn to get warning with a backtrace
+  = note: run with -Zmiri-isolation-error=ignore to hide warning
+  = note: run with -Zmiri-disable-isolation to disable isolation