Skip to content

fix: avoid stopping machine upon running env operations in isolation #1797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 34 additions & 1 deletion 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/disabled 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,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;
Expand Down
41 changes: 26 additions & 15 deletions src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
});
}
Expand Down
37 changes: 33 additions & 4 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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![],
Expand Down Expand Up @@ -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"
);
Expand Down
47 changes: 35 additions & 12 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))?;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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",
Expand All @@ -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
),
},
)?
Expand All @@ -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())
}
}
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
13 changes: 9 additions & 4 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand All @@ -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.
Expand Down
Loading