Skip to content

Commit

Permalink
miri isolation configuration using two flags
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
atsmtat committed Jun 5, 2021
1 parent 2089976 commit f28165e
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 42 deletions.
46 changes: 33 additions & 13 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 isolation
let mut isolation_enabled: Option<bool> = None;
for arg in env::args() {
if rustc_args.is_empty() {
// Very first arg: binary name.
Expand Down Expand Up @@ -291,8 +294,38 @@ fn main() {
miri_config.check_abi = false;
}
"-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;
}
Expand Down Expand Up @@ -385,19 +418,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);
Expand Down
2 changes: 1 addition & 1 deletion src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,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 warn about isolation and continue instead of aborting"))],
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")),
Expand Down
20 changes: 13 additions & 7 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -93,7 +99,7 @@ impl Default for MiriConfig {
stacked_borrows: true,
check_alignment: AlignmentCheck::Int,
check_abi: true,
isolated_op: IsolatedOp::Reject(RejectOpWith::WarningWithoutBacktrace),
isolated_op: IsolatedOp::Reject(RejectOpWith::Abort),
ignore_leaks: false,
excluded_env_vars: vec![],
args: vec![],
Expand Down
33 changes: 23 additions & 10 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -663,10 +666,20 @@ where
throw_ub_format!("incorrect number of arguments: got {}, expected {}", args.len(), N)
}

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.
/// Check that the ABI is what we expect.
pub fn check_abi<'a>(abi: Abi, exp_abi: Abi) -> InterpResult<'a, ()> {
if abi == exp_abi {
Ok(())
} else {
throw_ub_format!(
"calling a function with ABI {} using caller ABI {}",
exp_abi.name(),
abi.name()
)
}
}

pub fn isolation_abort_error(name: &str) -> InterpResult<'static> {
throw_machine_stop!(TerminationInfo::UnsupportedInIsolation(format!(
"{} not available when isolation is enabled",
name,
Expand Down
8 changes: 4 additions & 4 deletions src/shims/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down Expand Up @@ -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()?;
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/shims/posix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
1 change: 1 addition & 0 deletions tests/run-pass/current_dir_with_isolation.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// compile-flags: -Zmiri-isolation-error=warn-nobacktrace
use std::env;
use std::io::ErrorKind;

Expand Down
12 changes: 6 additions & 6 deletions tests/run-pass/current_dir_with_isolation.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit f28165e

Please sign in to comment.