Skip to content

Commit

Permalink
Auto merge of #103571 - RalfJung:miri, r=RalfJung
Browse files Browse the repository at this point in the history
update Miri

r? `@ghost`
  • Loading branch information
bors committed Oct 26, 2022
2 parents 36285c5 + 6ce5226 commit 1898c34
Show file tree
Hide file tree
Showing 25 changed files with 234 additions and 92 deletions.
18 changes: 13 additions & 5 deletions src/tools/miri/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -538,15 +538,23 @@ extern "Rust" {
fn miri_start_panic(payload: *mut u8) -> !;

/// Miri-provided extern function to get the internal unique identifier for the allocation that a pointer
/// points to. This is only useful as an input to `miri_print_stacks`, and it is a separate call because
/// points to. If this pointer is invalid (not pointing to an allocation), interpretation will abort.
///
/// This is only useful as an input to `miri_print_borrow_stacks`, and it is a separate call because
/// getting a pointer to an allocation at runtime can change the borrow stacks in the allocation.
/// This function should be considered unstable. It exists only to support `miri_print_borrow_stacks` and so
/// inherits all of its instability.
fn miri_get_alloc_id(ptr: *const ()) -> u64;

/// Miri-provided extern function to print (from the interpreter, not the program) the contents of all
/// borrow stacks in an allocation. The format of what this emits is unstable and may change at any time.
/// In particular, users should be aware that Miri will periodically attempt to garbage collect the
/// contents of all stacks. Callers of this function may wish to pass `-Zmiri-tag-gc=0` to disable the GC.
fn miri_print_stacks(alloc_id: u64);
/// borrow stacks in an allocation. The leftmost tag is the bottom of the stack.
/// The format of what this emits is unstable and may change at any time. In particular, users should be
/// aware that Miri will periodically attempt to garbage collect the contents of all stacks. Callers of
/// this function may wish to pass `-Zmiri-tag-gc=0` to disable the GC.
///
/// This function is extremely unstable. At any time the format of its output may change, its signature may
/// change, or it may be removed entirely.
fn miri_print_borrow_stacks(alloc_id: u64);

/// Miri-provided extern function to print (from the interpreter, not the
/// program) the contents of a section of program memory, as bytes. Bytes
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ fn main() {
// Re-export the TARGET environment variable so it can
// be accessed by miri.
let target = std::env::var("TARGET").unwrap();
println!("cargo:rustc-env=TARGET={}", target);
println!("cargo:rustc-env=TARGET={target}");
}
10 changes: 5 additions & 5 deletions src/tools/miri/cargo-miri/src/phases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Examples:
"#;

fn show_help() {
println!("{}", CARGO_MIRI_HELP);
println!("{CARGO_MIRI_HELP}");
}

fn show_version() {
Expand All @@ -52,7 +52,7 @@ fn forward_patched_extern_arg(args: &mut impl Iterator<Item = String>, cmd: &mut
let path = args.next().expect("`--extern` should be followed by a filename");
if let Some(lib) = path.strip_suffix(".rlib") {
// If this is an rlib, make it an rmeta.
cmd.arg(format!("{}.rmeta", lib));
cmd.arg(format!("{lib}.rmeta"));
} else {
// Some other extern file (e.g. a `.so`). Forward unchanged.
cmd.arg(path);
Expand Down Expand Up @@ -336,7 +336,7 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
"[cargo-miri rustc inside rustdoc] captured input:\n{}",
std::str::from_utf8(&env.stdin).unwrap()
);
eprintln!("[cargo-miri rustc inside rustdoc] going to run:\n{:?}", cmd);
eprintln!("[cargo-miri rustc inside rustdoc] going to run:\n{cmd:?}");
}

exec_with_pipe(cmd, &env.stdin, format!("{}.stdin", out_filename("", "").display()));
Expand Down Expand Up @@ -374,7 +374,7 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
val.push("metadata");
}
}
cmd.arg(format!("{}={}", emit_flag, val.join(",")));
cmd.arg(format!("{emit_flag}={}", val.join(",")));
} else if arg == "--extern" {
// Patch `--extern` filenames, since Cargo sometimes passes stub `.rlib` files:
// https://github.com/rust-lang/miri/issues/1705
Expand Down Expand Up @@ -535,7 +535,7 @@ pub fn phase_runner(mut binary_args: impl Iterator<Item = String>, phase: Runner
// Run it.
debug_cmd("[cargo-miri runner]", verbose, &cmd);
match phase {
RunnerPhase::Rustdoc => exec_with_pipe(cmd, &info.stdin, format!("{}.stdin", binary)),
RunnerPhase::Rustdoc => exec_with_pipe(cmd, &info.stdin, format!("{binary}.stdin")),
RunnerPhase::Cargo => exec(cmd),
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/tools/miri/cargo-miri/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub fn escape_for_toml(s: &str) -> String {
// We want to surround this string in quotes `"`. So we first escape all quotes,
// and also all backslashes (that are used to escape quotes).
let s = s.replace('\\', r#"\\"#).replace('"', r#"\""#);
format!("\"{}\"", s)
format!("\"{s}\"")
}

/// Returns the path to the `miri` binary
Expand Down Expand Up @@ -175,7 +175,7 @@ pub fn ask_to_run(mut cmd: Command, ask: bool, text: &str) {
let is_ci = env::var_os("CI").is_some() || env::var_os("TF_BUILD").is_some();
if ask && !is_ci {
let mut buf = String::new();
print!("I will run `{:?}` to {}. Proceed? [Y/n] ", cmd, text);
print!("I will run `{cmd:?}` to {text}. Proceed? [Y/n] ");
io::stdout().flush().unwrap();
io::stdin().read_line(&mut buf).unwrap();
match buf.trim().to_lowercase().as_ref() {
Expand All @@ -185,10 +185,10 @@ pub fn ask_to_run(mut cmd: Command, ask: bool, text: &str) {
a => show_error!("invalid answer `{}`", a),
};
} else {
eprintln!("Running `{:?}` to {}.", cmd, text);
eprintln!("Running `{cmd:?}` to {text}.");
}

if cmd.status().unwrap_or_else(|_| panic!("failed to execute {:?}", cmd)).success().not() {
if cmd.status().unwrap_or_else(|_| panic!("failed to execute {cmd:?}")).success().not() {
show_error!("failed to {}", text);
}
}
Expand Down Expand Up @@ -276,12 +276,12 @@ pub fn debug_cmd(prefix: &str, verbose: usize, cmd: &Command) {
// Print only what has been changed for this `cmd`.
for (var, val) in cmd.get_envs() {
if let Some(val) = val {
writeln!(out, "{}={:?} \\", var.to_string_lossy(), val).unwrap();
writeln!(out, "{}={val:?} \\", var.to_string_lossy()).unwrap();
} else {
writeln!(out, "--unset={}", var.to_string_lossy()).unwrap();
}
}
}
write!(out, "{cmd:?}").unwrap();
eprintln!("{}", out);
eprintln!("{out}");
}
2 changes: 1 addition & 1 deletion src/tools/miri/rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
b1ab3b738ac718da74cd4aa0bb7f362d0adbdf84
85d089b41e2a0c0f07ab34f6c5a7c451389f25e6
6 changes: 3 additions & 3 deletions src/tools/miri/src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ fn init_late_loggers(tcx: TyCtxt<'_>) {
if log::Level::from_str(&var).is_ok() {
env::set_var(
"RUSTC_LOG",
&format!(
format!(
"rustc_middle::mir::interpret={0},rustc_const_eval::interpret={0}",
var
),
Expand Down Expand Up @@ -243,7 +243,7 @@ fn host_sysroot() -> Option<String> {
)
}
}
format!("{}/toolchains/{}", home, toolchain)
format!("{home}/toolchains/{toolchain}")
}
_ => option_env!("RUST_SYSROOT")
.unwrap_or_else(|| {
Expand Down Expand Up @@ -330,7 +330,7 @@ fn main() {
} else if crate_kind == "host" {
false
} else {
panic!("invalid `MIRI_BE_RUSTC` value: {:?}", crate_kind)
panic!("invalid `MIRI_BE_RUSTC` value: {crate_kind:?}")
};

// We cannot use `rustc_driver::main` as we need to adjust the CLI arguments.
Expand Down
1 change: 1 addition & 0 deletions src/tools/miri/src/concurrency/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.machine.threads.active_thread_stack_mut()
}

/// Set the name of the current thread. The buffer must not include the null terminator.
#[inline]
fn set_thread_name(&mut self, thread: ThreadId, new_thread_name: Vec<u8>) {
let this = self.eval_context_mut();
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/concurrency/vector_clock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ mod tests {

//Test partial_cmp
let compare = l.partial_cmp(&r);
assert_eq!(compare, o, "Invalid comparison\n l: {:?}\n r: {:?}", l, r);
assert_eq!(compare, o, "Invalid comparison\n l: {l:?}\n r: {r:?}");
let alt_compare = r.partial_cmp(&l);
assert_eq!(
alt_compare,
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ pub fn report_error<'tcx, 'mir>(
msg.insert(0, e.to_string());
report_msg(
DiagLevel::Error,
&if let Some(title) = title { format!("{}: {}", title, msg[0]) } else { msg[0].clone() },
&if let Some(title) = title { format!("{title}: {}", msg[0]) } else { msg[0].clone() },
msg,
vec![],
helps,
Expand Down
63 changes: 61 additions & 2 deletions src/tools/miri/src/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pub mod convert;

use std::cmp;
use std::iter;
use std::mem;
use std::num::NonZeroUsize;
use std::time::Duration;
Expand Down Expand Up @@ -107,7 +108,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
/// Gets an instance for a path.
fn resolve_path(&self, path: &[&str]) -> ty::Instance<'tcx> {
self.try_resolve_path(path)
.unwrap_or_else(|| panic!("failed to find required Rust item: {:?}", path))
.unwrap_or_else(|| panic!("failed to find required Rust item: {path:?}"))
}

/// Evaluates the scalar at the specified path. Returns Some(val)
Expand Down Expand Up @@ -505,7 +506,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
RejectOpWith::WarningWithoutBacktrace => {
this.tcx
.sess
.warn(&format!("{} was made to return an error due to isolation", op_name));
.warn(format!("{op_name} was made to return an error due to isolation"));
Ok(())
}
RejectOpWith::Warning => {
Expand Down Expand Up @@ -735,6 +736,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
})
}

/// Read a sequence of bytes until the first null terminator.
fn read_c_str<'a>(&'a self, ptr: Pointer<Option<Provenance>>) -> InterpResult<'tcx, &'a [u8]>
where
'tcx: 'a,
Expand All @@ -761,6 +763,30 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.read_bytes_ptr_strip_provenance(ptr, len)
}

/// Helper function to write a sequence of bytes with an added null-terminator, which is what
/// the Unix APIs usually handle. This function returns `Ok((false, length))` without trying
/// to write if `size` is not large enough to fit the contents of `c_str` plus a null
/// terminator. It returns `Ok((true, length))` if the writing process was successful. The
/// string length returned does include the null terminator.
fn write_c_str(
&mut self,
c_str: &[u8],
ptr: Pointer<Option<Provenance>>,
size: u64,
) -> InterpResult<'tcx, (bool, u64)> {
// If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required null
// terminator to memory using the `ptr` pointer would cause an out-of-bounds access.
let string_length = u64::try_from(c_str.len()).unwrap();
let string_length = string_length.checked_add(1).unwrap();
if size < string_length {
return Ok((false, string_length));
}
self.eval_context_mut()
.write_bytes_ptr(ptr, c_str.iter().copied().chain(iter::once(0u8)))?;
Ok((true, string_length))
}

/// Read a sequence of u16 until the first null terminator.
fn read_wide_str(&self, mut ptr: Pointer<Option<Provenance>>) -> InterpResult<'tcx, Vec<u16>> {
let this = self.eval_context_ref();
let size2 = Size::from_bytes(2);
Expand All @@ -783,6 +809,39 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
Ok(wchars)
}

/// Helper function to write a sequence of u16 with an added 0x0000-terminator, which is what
/// the Windows APIs usually handle. This function returns `Ok((false, length))` without trying
/// to write if `size` is not large enough to fit the contents of `os_string` plus a null
/// terminator. It returns `Ok((true, length))` if the writing process was successful. The
/// string length returned does include the null terminator. Length is measured in units of
/// `u16.`
fn write_wide_str(
&mut self,
wide_str: &[u16],
ptr: Pointer<Option<Provenance>>,
size: u64,
) -> InterpResult<'tcx, (bool, u64)> {
// If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required
// 0x0000 terminator to memory would cause an out-of-bounds access.
let string_length = u64::try_from(wide_str.len()).unwrap();
let string_length = string_length.checked_add(1).unwrap();
if size < string_length {
return Ok((false, string_length));
}

// Store the UTF-16 string.
let size2 = Size::from_bytes(2);
let this = self.eval_context_mut();
let mut alloc = this
.get_ptr_alloc_mut(ptr, size2 * string_length, Align::from_bytes(2).unwrap())?
.unwrap(); // not a ZST, so we will get a result
for (offset, wchar) in wide_str.iter().copied().chain(iter::once(0x0000)).enumerate() {
let offset = u64::try_from(offset).unwrap();
alloc.write_scalar(alloc_range(size2 * offset, size2), Scalar::from_u16(wchar))?;
}
Ok((true, string_length))
}

/// Check that the ABI is what we expect.
fn check_abi<'a>(&self, abi: Abi, exp_abi: Abi) -> InterpResult<'a, ()> {
if self.eval_context_ref().machine.enforce_abi && abi != exp_abi {
Expand Down
6 changes: 3 additions & 3 deletions src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,12 @@ impl interpret::Provenance for Provenance {
Provenance::Concrete { alloc_id, sb } => {
// Forward `alternate` flag to `alloc_id` printing.
if f.alternate() {
write!(f, "[{:#?}]", alloc_id)?;
write!(f, "[{alloc_id:#?}]")?;
} else {
write!(f, "[{:?}]", alloc_id)?;
write!(f, "[{alloc_id:?}]")?;
}
// Print Stacked Borrows tag.
write!(f, "{:?}", sb)?;
write!(f, "{sb:?}")?;
}
Provenance::Wildcard => {
write!(f, "[wildcard]")?;
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/range_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl<T> RangeMap<T> {
let mut left = 0usize; // inclusive
let mut right = self.v.len(); // exclusive
loop {
debug_assert!(left < right, "find_offset: offset {} is out-of-bounds", offset);
debug_assert!(left < right, "find_offset: offset {offset} is out-of-bounds");
let candidate = left.checked_add(right).unwrap() / 2;
let elem = &self.v[candidate];
if offset < elem.range.start {
Expand Down
10 changes: 7 additions & 3 deletions src/tools/miri/src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
return Ok(Some(body));
}

this.handle_unsupported(format!("can't call foreign function: {}", link_name))?;
this.handle_unsupported(format!("can't call foreign function: {link_name}"))?;
return Ok(None);
}
}
Expand Down Expand Up @@ -420,10 +420,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
"miri_get_alloc_id" => {
let [ptr] = this.check_shim(abi, Abi::Rust, link_name, args)?;
let ptr = this.read_pointer(ptr)?;
let (alloc_id, _, _) = this.ptr_get_alloc_id(ptr)?;
let (alloc_id, _, _) = this.ptr_get_alloc_id(ptr).map_err(|_e| {
err_machine_stop!(TerminationInfo::Abort(
format!("pointer passed to miri_get_alloc_id must not be dangling, got {ptr:?}")
))
})?;
this.write_scalar(Scalar::from_u64(alloc_id.0.get()), dest)?;
}
"miri_print_stacks" => {
"miri_print_borrow_stacks" => {
let [id] = this.check_shim(abi, Abi::Rust, link_name, args)?;
let id = this.read_scalar(id)?.to_u64()?;
if let Some(id) = std::num::NonZeroU64::new(id) {
Expand Down
33 changes: 2 additions & 31 deletions src/tools/miri/src/shims/os_str.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::borrow::Cow;
use std::ffi::{OsStr, OsString};
use std::iter;
use std::path::{Path, PathBuf};

#[cfg(unix)]
Expand All @@ -9,7 +8,6 @@ use std::os::unix::ffi::{OsStrExt, OsStringExt};
use std::os::windows::ffi::{OsStrExt, OsStringExt};

use rustc_middle::ty::layout::LayoutOf;
use rustc_target::abi::{Align, Size};

use crate::*;

Expand Down Expand Up @@ -100,16 +98,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
size: u64,
) -> InterpResult<'tcx, (bool, u64)> {
let bytes = os_str_to_bytes(os_str)?;
// If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required null
// terminator to memory using the `ptr` pointer would cause an out-of-bounds access.
let string_length = u64::try_from(bytes.len()).unwrap();
let string_length = string_length.checked_add(1).unwrap();
if size < string_length {
return Ok((false, string_length));
}
self.eval_context_mut()
.write_bytes_ptr(ptr, bytes.iter().copied().chain(iter::once(0u8)))?;
Ok((true, string_length))
self.eval_context_mut().write_c_str(bytes, ptr, size)
}

/// Helper function to write an OsStr as a 0x0000-terminated u16-sequence, which is what
Expand Down Expand Up @@ -140,25 +129,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}

let u16_vec = os_str_to_u16vec(os_str)?;
// If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required
// 0x0000 terminator to memory would cause an out-of-bounds access.
let string_length = u64::try_from(u16_vec.len()).unwrap();
let string_length = string_length.checked_add(1).unwrap();
if size < string_length {
return Ok((false, string_length));
}

// Store the UTF-16 string.
let size2 = Size::from_bytes(2);
let this = self.eval_context_mut();
let mut alloc = this
.get_ptr_alloc_mut(ptr, size2 * string_length, Align::from_bytes(2).unwrap())?
.unwrap(); // not a ZST, so we will get a result
for (offset, wchar) in u16_vec.into_iter().chain(iter::once(0x0000)).enumerate() {
let offset = u64::try_from(offset).unwrap();
alloc.write_scalar(alloc_range(size2 * offset, size2), Scalar::from_u16(wchar))?;
}
Ok((true, string_length))
self.eval_context_mut().write_wide_str(&u16_vec, ptr, size)
}

/// Allocate enough memory to store the given `OsStr` as a null-terminated sequence of bytes.
Expand Down
Loading

0 comments on commit 1898c34

Please sign in to comment.