Skip to content

Add support for panicking in the emulated application when unsupported functionality is encountered #1818

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
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
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,11 @@ environment variable:
This can be used to find which parts of your program are executing slowly under Miri.
The profile is written out to a file with the prefix `<name>`, and can be processed
using the tools in the repository https://github.com/rust-lang/measureme.
* `-Zmiri-panic-on-unsupported` will makes some forms of unsupported functionality,
such as FFI and unsupported syscalls, panic within the context of the emulated
application instead of raising an error within the context of Miri (and halting
execution). Note that code might not expect these operations to ever panic, so
this flag can lead to strange (mis)behavior.
* `-Zmiri-seed=<hex>` configures the seed of the RNG that Miri uses to resolve
non-determinism. This RNG is used to pick base addresses for allocations.
When isolation is enabled (the default), this is also used to emulate system
Expand Down
3 changes: 3 additions & 0 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,9 @@ fn main() {
"-Zmiri-ignore-leaks" => {
miri_config.ignore_leaks = true;
}
"-Zmiri-panic-on-unsupported" => {
miri_config.panic_on_unsupported = true;
}
"-Zmiri-track-raw-pointers" => {
miri_config.track_raw = true;
}
Expand Down
3 changes: 3 additions & 0 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ pub struct MiriConfig {
/// If `Some`, enable the `measureme` profiler, writing results to a file
/// with the specified prefix.
pub measureme_out: Option<String>,
/// Panic when unsupported functionality is encountered
pub panic_on_unsupported: bool,
}

impl Default for MiriConfig {
Expand All @@ -80,6 +82,7 @@ impl Default for MiriConfig {
data_race_detector: true,
cmpxchg_weak_failure_rate: 0.8,
measureme_out: None,
panic_on_unsupported: false,
}
}
}
Expand Down
17 changes: 17 additions & 0 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,23 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
== this.tcx.def_path(start_fn).krate
})
}

/// Handler that should be called when unsupported functionality is encountered.
/// This function will either panic within the context of the emulated application
/// or return an error in the Miri process context
///
/// Return value of `Ok(bool)` indicates whether execution should continue.
fn handle_unsupported<S: AsRef<str>>(&mut self, error_msg: S) -> InterpResult<'tcx, ()> {
let this = self.eval_context_mut();
if this.machine.panic_on_unsupported {
// message is slightly different here to make automated analysis easier
let error_msg = format!("unsupported Miri functionality: {}", error_msg.as_ref());
this.start_panic(error_msg.as_ref(), StackPopUnwind::Skip)?;
return Ok(());
} else {
throw_unsup_format!("{}", error_msg.as_ref());
}
}
}

/// Check that the number of args is what we expect.
Expand Down
6 changes: 6 additions & 0 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,11 @@ pub struct Evaluator<'mir, 'tcx> {

/// Cache of `Instance` exported under the given `Symbol` name.
pub(crate) exported_symbols_cache: FxHashMap<Symbol, Instance<'tcx>>,

/// Whether to raise a panic in the context of the evaluated process when unsupported
/// functionality is encountered. If `false`, an error is propagated in the Miri application context
/// instead (default behavior)
pub(crate) panic_on_unsupported: bool,
}

impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
Expand Down Expand Up @@ -326,6 +331,7 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
profiler,
string_cache: Default::default(),
exported_symbols_cache: FxHashMap::default(),
panic_on_unsupported: config.panic_on_unsupported,
}
}
}
Expand Down
10 changes: 8 additions & 2 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
if let Some(body) = this.lookup_exported_symbol(link_name_sym)? {
return Ok(Some(body));
}
throw_unsup_format!("can't call (diverging) foreign function: {}", link_name);
this.handle_unsupported(format!(
"can't call (diverging) foreign function: {}",
link_name
))?;
return Ok(None);
}
},
Some(p) => p,
Expand All @@ -276,7 +280,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
if let Some(body) = this.lookup_exported_symbol(link_name_sym)? {
return Ok(Some(body));
}
throw_unsup_format!("can't call foreign function: {}", link_name);

this.handle_unsupported(format!("can't call foreign function: {}", link_name))?;
return Ok(None);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/shims/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
}
}

/// Starta a panic in the interpreter with the given message as payload.
/// Start a panic in the interpreter with the given message as payload.
fn start_panic(&mut self, msg: &str, unwind: StackPopUnwind) -> InterpResult<'tcx> {
let this = self.eval_context_mut();

Expand Down
5 changes: 4 additions & 1 deletion src/shims/posix/linux/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
id if id == sys_futex => {
futex(this, args, dest)?;
}
id => throw_unsup_format!("Miri does not support syscall ID {}", id),
id => {
this.handle_unsupported(format!("can't execute syscall with ID {}", id))?;
return Ok(EmulateByNameResult::NotSupported);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be EmulateByNameResult::AlreadyJumped?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, I think you are right. This is one of the scenarios I did not test and may have been a merge error.

Copy link
Member

@RalfJung RalfJung Jun 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, NotSupported means "look up exported function symbol".

I wonder if we should look up the exported symbol first, and only then run our own shim? Then we might not even need the NotSupported variant.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should look up the exported symbol first, and only then run our own shim? Then we might not even need the NotSupported variant.

I think the variant is still needed -- the exported symbol will only be run if no shim implementation can be found, and NotSupported is required to represent that.

}
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/shims/windows/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// Better error for attempts to create a thread
"CreateThread" => {
this.check_abi(abi, Abi::System { unwind: false })?;
throw_unsup_format!("Miri does not support concurrency on Windows");

this.handle_unsupported("can't create threads on Windows")?;
return Ok(EmulateByNameResult::AlreadyJumped);
}

// Incomplete shims that we "stub out" just to get pre-main initialization code to work.
Expand Down
2 changes: 1 addition & 1 deletion tests/compile-fail/concurrency/thread-spawn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use std::thread;

// error-pattern: Miri does not support concurrency on Windows
// error-pattern: can't create threads on Windows

fn main() {
thread::spawn(|| {});
Expand Down
11 changes: 11 additions & 0 deletions tests/run-pass/panic/unsupported_foreign_function.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// compile-flags: -Zmiri-panic-on-unsupported

fn main() {
extern "Rust" {
fn foo();
}

unsafe {
foo();
}
}
2 changes: 2 additions & 0 deletions tests/run-pass/panic/unsupported_foreign_function.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
thread 'main' panicked at 'unsupported Miri functionality: can't call foreign function: foo', $DIR/unsupported_foreign_function.rs:9:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace