Auto merge of #1797 - atsmtat:env-isolation, r=RalfJung

fix: avoid stopping machine upon running env operations in isolation

get and set current dir operations used to halt the machine by
throwing an exception in isolation mode. This change updates them to
return a dummy `NotFound` error instead, and keep the machine running.

I started with a custom error using `ErrorKind::Other`, but since it
can't be mapped to a raw OS error, I dropped it. `NotFound` kind of make
sense for get operations, but not much for set operations. But that's
the only error supported for windows currently.

Author: bors

README.md

@@ -219,6 +219,13 @@ environment variable:
 * `-Zmiri-disable-isolation` disables host isolation.  As a consequence,
   the program has access to host resources such as environment variables, file
   systems, and randomness.
+* `-Zmiri-isolation-error=<action>` configures Miri's response to operations
+  requiring host access while isolation is enabled. `abort`, `hide`, `warn`,
+  and `warn-nobacktrace` are the supported actions. Default action is `abort`
+  which halts the machine. Rest of the actions configure it to return an error
+  code for the op and continue executing. `warn` prints backtrace that could
+  be used to trace the call. `warn-nobacktrace` is less verbose without
+  backtrace. `hide` hides the warning.
 * `-Zmiri-env-exclude=<var>` keeps the `var` environment variable isolated from
   the host so that it cannot be accessed by the program.  Can be used multiple
   times to exclude several variables.  On Windows, the `TERM` environment

src/bin/miri.rs

@@ -263,6 +263,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/disabled isolation
+    let mut isolation_enabled: Option<bool> = None;
     for arg in env::args() {
         if rustc_args.is_empty() {
             // Very first arg: binary name.
@@ -291,7 +294,37 @@ fn main() {
                     miri_config.check_abi = false;
                 }
                 "-Zmiri-disable-isolation" => {
-                    miri_config.communicate = true;
+                    if matches!(isolation_enabled, Some(true)) {
+                        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 matches!(isolation_enabled, Some(false)) {
+                        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),
+                        "hide" => 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`, `hide`, `warn`, or `warn-nobacktrace`"
+                        ),
+                    };
                 }
                 "-Zmiri-ignore-leaks" => {
                     miri_config.ignore_leaks = true;

src/diagnostics.rs

@@ -52,6 +52,14 @@ pub enum NonHaltingDiagnostic {
     CreatedCallId(CallId),
     CreatedAlloc(AllocId),
     FreedAlloc(AllocId),
+    RejectedIsolatedOp(String),
+}
+
+/// Level of Miri specific diagnostics
+enum DiagLevel {
+    Error,
+    Warning,
+    Note,
 }
 
 /// Emit a custom diagnostic without going through the miri-engine machinery
@@ -76,7 +84,7 @@ pub fn report_error<'tcx, 'mir>(
             #[rustfmt::skip]
             let helps = match info {
                 UnsupportedInIsolation(_) =>
-                    vec![(None, format!("pass the flag `-Zmiri-disable-isolation` to disable isolation"))],
+                    vec![(None, format!("pass the flag `-Zmiri-disable-isolation` to disable isolation; or pass `-Zmiri-isolation-error=warn to configure Miri to return an error code from isolated operations and continue with a warning"))],
                 ExperimentalUb { url, .. } =>
                     vec![
                         (None, format!("this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental")),
@@ -137,7 +145,7 @@ pub fn report_error<'tcx, 'mir>(
     let msg = e.to_string();
     report_msg(
         *ecx.tcx,
-        /*error*/ true,
+        DiagLevel::Error,
         &if let Some(title) = title { format!("{}: {}", title, msg) } else { msg.clone() },
         msg,
         helps,
@@ -174,18 +182,19 @@ pub fn report_error<'tcx, 'mir>(
 /// Also emits a full stacktrace of the interpreter stack.
 fn report_msg<'tcx>(
     tcx: TyCtxt<'tcx>,
-    error: bool,
+    diag_level: DiagLevel,
     title: &str,
     span_msg: String,
     mut helps: Vec<(Option<SpanData>, String)>,
     stacktrace: &[FrameInfo<'tcx>],
 ) {
     let span = stacktrace.first().map_or(DUMMY_SP, |fi| fi.span);
-    let mut err = if error {
-        tcx.sess.struct_span_err(span, title)
-    } else {
-        tcx.sess.diagnostic().span_note_diag(span, title)
+    let mut err = match diag_level {
+        DiagLevel::Error => tcx.sess.struct_span_err(span, title),
+        DiagLevel::Warning => tcx.sess.struct_span_warn(span, title),
+        DiagLevel::Note => tcx.sess.diagnostic().span_note_diag(span, title),
     };
+
     // Show main message.
     if span != DUMMY_SP {
         err.span_label(span, span_msg);
@@ -303,15 +312,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     CreatedCallId(id) => format!("function call with id {}", id),
                     CreatedAlloc(AllocId(id)) => format!("created allocation with id {}", id),
                     FreedAlloc(AllocId(id)) => format!("freed allocation with id {}", id),
+                    RejectedIsolatedOp(ref op) =>
+                        format!("`{}` was made to return an error due to isolation", op),
                 };
-                report_msg(
-                    *this.tcx,
-                    /*error*/ false,
-                    "tracking was triggered",
-                    msg,
-                    vec![],
-                    &stacktrace,
-                );
+
+                let (title, diag_level) = match e {
+                    RejectedIsolatedOp(_) =>
+                        ("operation rejected by isolation", DiagLevel::Warning),
+                    _ => ("tracking was triggered", DiagLevel::Note),
+                };
+
+                report_msg(*this.tcx, diag_level, title, msg, vec![], &stacktrace);
             }
         });
     }

src/eval.rs

@@ -22,6 +22,35 @@ pub enum AlignmentCheck {
     Int,
 }
 
+#[derive(Copy, Clone, Debug, PartialEq)]
+pub enum RejectOpWith {
+    /// 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.
+    Warning,
+
+    /// Print a warning about rejected isolated op, without backtrace.
+    WarningWithoutBacktrace,
+}
+
+#[derive(Copy, Clone, Debug, PartialEq)]
+pub enum IsolatedOp {
+    /// 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.
+    Allow,
+}
+
 /// Configuration needed to spawn a Miri instance.
 #[derive(Clone)]
 pub struct MiriConfig {
@@ -33,8 +62,8 @@ pub struct MiriConfig {
     pub check_alignment: AlignmentCheck,
     /// Controls function [ABI](Abi) checking.
     pub check_abi: bool,
-    /// Determines if communication with the host environment is enabled.
-    pub communicate: bool,
+    /// Action for an op requiring communication with the host.
+    pub isolated_op: IsolatedOp,
     /// Determines if memory leaks should be ignored.
     pub ignore_leaks: bool,
     /// Environment variables that should always be isolated from the host.
@@ -68,7 +97,7 @@ impl Default for MiriConfig {
             stacked_borrows: true,
             check_alignment: AlignmentCheck::Int,
             check_abi: true,
-            communicate: false,
+            isolated_op: IsolatedOp::Reject(RejectOpWith::Abort),
             ignore_leaks: false,
             excluded_env_vars: vec![],
             args: vec![],
@@ -233,7 +262,7 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->
                 }
                 SchedulingAction::ExecuteTimeoutCallback => {
                     assert!(
-                        ecx.machine.communicate,
+                        ecx.machine.communicate(),
                         "scheduler callbacks require disabled isolation, but the code \
                         that created the callback did not check it"
                     );

src/helpers.rs

@@ -140,7 +140,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         let mut data = vec![0; usize::try_from(len).unwrap()];
 
-        if this.machine.communicate {
+        if this.machine.communicate() {
             // Fill the buffer using the host's rng.
             getrandom::getrandom(&mut data)
                 .map_err(|err| err_unsup_format!("host getrandom failed: {}", err))?;
@@ -391,12 +391,32 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     /// disabled. It returns an error using the `name` of the foreign function if this is not the
     /// case.
     fn check_no_isolation(&self, name: &str) -> InterpResult<'tcx> {
-        if !self.eval_context_ref().machine.communicate {
-            isolation_error(name)?;
+        if !self.eval_context_ref().machine.communicate() {
+            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 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 => {
+                this.tcx
+                    .sess
+                    .warn(&format!("`{}` was made to return an error due to isolation", op_name));
+                Ok(())
+            }
+            RejectOpWith::Warning => {
+                register_diagnostic(NonHaltingDiagnostic::RejectedIsolatedOp(op_name.to_string()));
+                Ok(())
+            }
+            RejectOpWith::NoWarning => Ok(()), // no warning
+        }
+    }
+
     /// Helper function used inside the shims of foreign functions to assert that the target OS
     /// is `target_os`. It panics showing a message with the `name` of the foreign function
     /// if this is not the case.
@@ -440,15 +460,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         this.read_scalar(&errno_place.into())?.check_init()
     }
 
-    /// Sets the last OS error using a `std::io::Error`. This function tries to produce the most
+    /// Sets the last OS error using a `std::io::ErrorKind`. This function tries to produce the most
     /// similar OS error from the `std::io::ErrorKind` and sets it as the last OS error.
-    fn set_last_error_from_io_error(&mut self, e: std::io::Error) -> InterpResult<'tcx> {
+    fn set_last_error_from_io_error(&mut self, err_kind: std::io::ErrorKind) -> InterpResult<'tcx> {
         use std::io::ErrorKind::*;
         let this = self.eval_context_mut();
         let target = &this.tcx.sess.target;
         let target_os = &target.os;
         let last_error = if target.families.contains(&"unix".to_owned()) {
-            this.eval_libc(match e.kind() {
+            this.eval_libc(match err_kind {
                 ConnectionRefused => "ECONNREFUSED",
                 ConnectionReset => "ECONNRESET",
                 PermissionDenied => "EPERM",
@@ -464,18 +484,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 AlreadyExists => "EEXIST",
                 WouldBlock => "EWOULDBLOCK",
                 _ => {
-                    throw_unsup_format!("io error {} cannot be transformed into a raw os error", e)
+                    throw_unsup_format!(
+                        "io error {:?} cannot be transformed into a raw os error",
+                        err_kind
+                    )
                 }
             })?
         } else if target.families.contains(&"windows".to_owned()) {
             // FIXME: we have to finish implementing the Windows equivalent of this.
             this.eval_windows(
                 "c",
-                match e.kind() {
+                match err_kind {
                     NotFound => "ERROR_FILE_NOT_FOUND",
                     _ => throw_unsup_format!(
-                        "io error {} cannot be transformed into a raw os error",
-                        e
+                        "io error {:?} cannot be transformed into a raw os error",
+                        err_kind
                     ),
                 },
             )?
@@ -501,7 +524,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         match result {
             Ok(ok) => Ok(ok),
             Err(e) => {
-                self.eval_context_mut().set_last_error_from_io_error(e)?;
+                self.eval_context_mut().set_last_error_from_io_error(e.kind())?;
                 Ok((-1).into())
             }
         }
@@ -651,7 +674,7 @@ where
     throw_ub_format!("incorrect number of arguments: got {}, expected {}", args.len(), N)
 }
 
-pub fn isolation_error(name: &str) -> InterpResult<'static> {
+pub fn isolation_abort_error(name: &str) -> InterpResult<'static> {
     throw_machine_stop!(TerminationInfo::UnsupportedInIsolation(format!(
         "{} not available when isolation is enabled",
         name,

src/lib.rs

@@ -59,7 +59,9 @@ pub use crate::diagnostics::{
     register_diagnostic, report_error, EvalContextExt as DiagnosticsEvalContextExt,
     NonHaltingDiagnostic, TerminationInfo,
 };
-pub use crate::eval::{create_ecx, eval_main, AlignmentCheck, MiriConfig};
+pub use crate::eval::{
+    create_ecx, eval_main, AlignmentCheck, IsolatedOp, MiriConfig, RejectOpWith,
+};
 pub use crate::helpers::EvalContextExt as HelpersEvalContextExt;
 pub use crate::machine::{
     AllocExtra, Evaluator, FrameData, MemoryExtra, MiriEvalContext, MiriEvalContextExt,

src/machine.rs

@@ -263,9 +263,10 @@ pub struct Evaluator<'mir, 'tcx> {
     /// TLS state.
     pub(crate) tls: TlsData<'tcx>,
 
-    /// If enabled, the `env_vars` field is populated with the host env vars during initialization
-    /// and random number generation is delegated to the host.
-    pub(crate) communicate: bool,
+    /// What should Miri do when an op requires communicating with the host,
+    /// such as accessing host env vars, random number generation, and
+    /// file system access.
+    pub(crate) isolated_op: IsolatedOp,
 
     /// Whether to enforce the validity invariant.
     pub(crate) validate: bool,
@@ -314,7 +315,7 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
             argv: None,
             cmd_line: None,
             tls: TlsData::default(),
-            communicate: config.communicate,
+            isolated_op: config.isolated_op,
             validate: config.validate,
             enforce_abi: config.check_abi,
             file_handler: Default::default(),
@@ -328,6 +329,10 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
             exported_symbols_cache: FxHashMap::default(),
         }
     }
+
+    pub(crate) fn communicate(&self) -> bool {
+        self.isolated_op == IsolatedOp::Allow
+    }
 }
 
 /// A rustc InterpCx for Miri.