Skip to content
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

update Miri #111867

Merged
merged 17 commits into from
May 23, 2023
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
23 changes: 23 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,16 @@ dependencies = [
"quote",
]

[[package]]
name = "ctrlc"
version = "3.3.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7394a21d012ce5c850497fb774b167d81b99f060025fbf06ee92b9848bd97eb2"
dependencies = [
"nix",
"windows-sys 0.48.0",
]

[[package]]
name = "curl"
version = "0.4.44"
Expand Down Expand Up @@ -2295,6 +2305,7 @@ name = "miri"
version = "0.1.0"
dependencies = [
"colored",
"ctrlc",
"env_logger 0.9.0",
"getrandom",
"lazy_static",
Expand Down Expand Up @@ -2324,6 +2335,18 @@ version = "1.0.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e4a24736216ec316047a1fc4252e27dabb04218aa4a3f37c6e7ddbf1f9782b54"

[[package]]
name = "nix"
version = "0.26.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bfdda3d196821d6af13126e40375cdf7da646a96114af134d5f417a9a1dc8e1a"
dependencies = [
"bitflags",
"cfg-if",
"libc",
"static_assertions",
]

[[package]]
name = "nom"
version = "7.1.0"
Expand Down
29 changes: 29 additions & 0 deletions src/tools/miri/Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,16 @@ dependencies = [
"cfg-if",
]

[[package]]
name = "ctrlc"
version = "3.2.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bbcf33c2a618cbe41ee43ae6e9f2e48368cd9f9db2896f10167d8d762679f639"
dependencies = [
"nix",
"windows-sys 0.45.0",
]

[[package]]
name = "diff"
version = "0.1.13"
Expand Down Expand Up @@ -421,6 +431,7 @@ name = "miri"
version = "0.1.0"
dependencies = [
"colored",
"ctrlc",
"env_logger",
"getrandom",
"lazy_static",
Expand All @@ -437,6 +448,18 @@ dependencies = [
"ui_test",
]

[[package]]
name = "nix"
version = "0.26.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bfdda3d196821d6af13126e40375cdf7da646a96114af134d5f417a9a1dc8e1a"
dependencies = [
"bitflags",
"cfg-if",
"libc",
"static_assertions",
]

[[package]]
name = "object"
version = "0.30.3"
Expand Down Expand Up @@ -713,6 +736,12 @@ version = "1.10.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a507befe795404456341dfab10cef66ead4c041f62b8b11bbb92bffe5d0953e0"

[[package]]
name = "static_assertions"
version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f"

[[package]]
name = "syn"
version = "2.0.15"
Expand Down
1 change: 1 addition & 0 deletions src/tools/miri/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ smallvec = "1.7"
# for more information.
rustc-workspace-hack = "1.0.0"
measureme = "10.0.0"
ctrlc = "3.2.5"

[target.'cfg(unix)'.dependencies]
libc = "0.2"
Expand Down
8 changes: 4 additions & 4 deletions src/tools/miri/cargo-miri/src/arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ impl<'s, I: Iterator<Item = Cow<'s, str>>> Iterator for ArgSplitFlagValue<'_, I>
if arg == "--" {
// Stop searching at `--`.
self.args = None;
return None;
// But yield the `--` so that it does not get lost!
return Some(Err(Cow::Borrowed("--")));
}
// These branches cannot be merged if we want to avoid the allocation in the `Borrowed` branch.
match &arg {
Expand Down Expand Up @@ -79,9 +80,8 @@ impl<'a, I: Iterator<Item = String> + 'a> ArgSplitFlagValue<'a, I> {
) -> impl Iterator<Item = Result<String, String>> + 'a {
ArgSplitFlagValue::new(args.map(Cow::Owned), name).map(|x| {
match x {
Ok(Cow::Owned(s)) => Ok(s),
Err(Cow::Owned(s)) => Err(s),
_ => panic!("iterator converted owned to borrowed"),
Ok(s) => Ok(s.into_owned()),
Err(s) => Err(s.into_owned()),
}
})
}
Expand Down
52 changes: 27 additions & 25 deletions src/tools/miri/cargo-miri/src/phases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,30 +113,17 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
};
let metadata = get_cargo_metadata();
let mut cmd = cargo();
cmd.arg(cargo_cmd);

// Forward all arguments before `--` other than `--target-dir` and its value to Cargo.
// (We want to *change* the target-dir value, so we must not forward it.)
let mut target_dir = None;
for arg in ArgSplitFlagValue::from_string_iter(&mut args, "--target-dir") {
match arg {
Ok(value) => {
if target_dir.is_some() {
show_error!("`--target-dir` is provided more than once");
}
target_dir = Some(value.into());
}
Err(arg) => {
cmd.arg(arg);
}
}
cmd.arg(&cargo_cmd);
// In nextest we have to also forward the main `verb`.
if cargo_cmd == "nextest" {
cmd.arg(
args.next()
.unwrap_or_else(|| show_error!("`cargo miri nextest` expects a verb (e.g. `run`)")),
);
}
// Detect the target directory if it's not specified via `--target-dir`.
// (`cargo metadata` does not support `--target-dir`, that's why we have to handle this ourselves.)
let target_dir = target_dir.get_or_insert_with(|| metadata.target_directory.clone());
// Set `--target-dir` to `miri` inside the original target directory.
target_dir.push("miri");
cmd.arg("--target-dir").arg(target_dir);
// We set the following flags *before* forwarding more arguments.
// This is needed to fix <https://github.com/rust-lang/miri/issues/2829>: cargo will stop
// interpreting things as flags when it sees the first positional argument.

// Make sure the build target is explicitly set.
// This is needed to make the `target.runner` settings do something,
Expand All @@ -154,8 +141,23 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
cmd.arg("--config")
.arg(format!("target.'cfg(all())'.runner=[{cargo_miri_path_for_toml}, 'runner']"));

// Forward all further arguments after `--` to cargo.
cmd.arg("--").args(args);
// Set `--target-dir` to `miri` inside the original target directory.
let mut target_dir = match get_arg_flag_value("--target-dir") {
Some(dir) => PathBuf::from(dir),
None => metadata.target_directory.clone().into_std_path_buf(),
};
target_dir.push("miri");
cmd.arg("--target-dir").arg(target_dir);

// *After* we set all the flags that need setting, forward everything else. Make sure to skip
// `--target-dir` (which would otherwise be set twice).
for arg in
ArgSplitFlagValue::from_string_iter(&mut args, "--target-dir").filter_map(Result::err)
{
cmd.arg(arg);
}
// Forward all further arguments (not consumed by `ArgSplitFlagValue`) to cargo.
cmd.args(args);

// Set `RUSTC_WRAPPER` to ourselves. Cargo will prepend that binary to its usual invocation,
// i.e., the first argument is `rustc` -- which is what we use in `main` to distinguish
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
69fef92ab2f287f072b66fb7b4f62c8bb4acba43
8b4b20836b832e91aa605a2faf5e2a55190202c8
17 changes: 17 additions & 0 deletions src/tools/miri/src/concurrency/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::cell::RefCell;
use std::collections::hash_map::Entry;
use std::num::TryFromIntError;
use std::sync::atomic::{AtomicBool, Ordering::Relaxed};
use std::task::Poll;
use std::time::{Duration, SystemTime};

Expand Down Expand Up @@ -1012,8 +1013,24 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
/// Run the core interpreter loop. Returns only when an interrupt occurs (an error or program
/// termination).
fn run_threads(&mut self) -> InterpResult<'tcx, !> {
static SIGNALED: AtomicBool = AtomicBool::new(false);
ctrlc::set_handler(move || {
// Indicate that we have ben signaled to stop. If we were already signaled, exit
// immediately. In our interpreter loop we try to consult this value often, but if for
// whatever reason we don't get to that check or the cleanup we do upon finding that
// this bool has become true takes a long time, the exit here will promptly exit the
// process on the second Ctrl-C.
if SIGNALED.swap(true, Relaxed) {
std::process::exit(1);
}
})
.unwrap();
let this = self.eval_context_mut();
loop {
if SIGNALED.load(Relaxed) {
this.machine.handle_abnormal_termination();
std::process::exit(1);
}
match this.machine.threads.schedule(&this.machine.clock)? {
SchedulingAction::ExecuteStep => {
if !this.step()? {
Expand Down
9 changes: 9 additions & 0 deletions src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,15 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
let def_id = frame.instance.def_id();
def_id.is_local() || self.local_crates.contains(&def_id.krate)
}

/// Called when the interpreter is going to shut down abnormally, such as due to a Ctrl-C.
pub(crate) fn handle_abnormal_termination(&mut self) {
// All strings in the profile data are stored in a single string table which is not
// written to disk until the profiler is dropped. If the interpreter exits without dropping
// the profiler, it is not possible to interpret the profile data and all measureme tools
// will panic when given the file.
drop(self.profiler.take());
}
}

impl VisitTags for MiriMachine<'_, '_> {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let ptr_layout = this.layout_of(ptr_ty)?;

for (i, ptr) in ptrs.into_iter().enumerate() {
let offset = ptr_layout.size * i.try_into().unwrap();
let offset = ptr_layout.size.checked_mul(i.try_into().unwrap(), this).unwrap();

let op_place = buf_place.offset(offset, ptr_layout, this)?;

Expand Down
8 changes: 4 additions & 4 deletions src/tools/miri/src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
dependency_format.1.iter().enumerate().filter_map(|(num, &linkage)| {
// We add 1 to the number because that's what rustc also does everywhere it
// calls `CrateNum::new`...
#[allow(clippy::integer_arithmetic)]
#[allow(clippy::arithmetic_side_effects)]
(linkage != Linkage::NotLinked).then_some(CrateNum::new(num + 1))
}),
) {
Expand Down Expand Up @@ -707,7 +707,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
.position(|&c| c == val)
{
let idx = u64::try_from(idx).unwrap();
#[allow(clippy::integer_arithmetic)] // idx < num, so this never wraps
#[allow(clippy::arithmetic_side_effects)] // idx < num, so this never wraps
let new_ptr = ptr.offset(Size::from_bytes(num - idx - 1), this)?;
this.write_pointer(new_ptr, dest)?;
} else {
Expand Down Expand Up @@ -916,10 +916,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let a = this.read_scalar(a)?.to_u64()?;
let b = this.read_scalar(b)?.to_u64()?;

#[allow(clippy::integer_arithmetic)]
#[allow(clippy::arithmetic_side_effects)]
// adding two u64 and a u8 cannot wrap in a u128
let wide_sum = u128::from(c_in) + u128::from(a) + u128::from(b);
#[allow(clippy::integer_arithmetic)] // it's a u128, we can shift by 64
#[allow(clippy::arithmetic_side_effects)] // it's a u128, we can shift by 64
let (c_out, sum) = ((wide_sum >> 64).truncate::<u8>(), wide_sum.truncate::<u64>());

let c_out_field = this.place_field(dest, 0)?;
Expand Down
13 changes: 10 additions & 3 deletions src/tools/miri/src/shims/intrinsics/simd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let (op, op_len) = this.operand_to_simd(op)?;
let bitmask_len = op_len.max(8);

assert!(dest.layout.ty.is_integral());
// Returns either an unsigned integer or array of `u8`.
assert!(
dest.layout.ty.is_integral()
|| matches!(dest.layout.ty.kind(), ty::Array(elemty, _) if elemty == &this.tcx.types.u8)
);
assert!(bitmask_len <= 64);
assert_eq!(bitmask_len, dest.layout.size.bits());
let op_len = u32::try_from(op_len).unwrap();
Expand All @@ -577,7 +581,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
.unwrap();
}
}
this.write_int(res, dest)?;
// We have to force the place type to be an int so that we can write `res` into it.
let mut dest = this.force_allocation(dest)?;
dest.layout = this.machine.layouts.uint(dest.layout.size).unwrap();
this.write_int(res, &dest.into())?;
}

name => throw_unsup_format!("unimplemented intrinsic: `simd_{name}`"),
Expand Down Expand Up @@ -605,7 +612,7 @@ fn simd_bitmask_index(idx: u32, vec_len: u32, endianness: Endian) -> u32 {
assert!(idx < vec_len);
match endianness {
Endian::Little => idx,
#[allow(clippy::integer_arithmetic)] // idx < vec_len
#[allow(clippy::arithmetic_side_effects)] // idx < vec_len
Endian::Big => vec_len - 1 - idx, // reverse order of bits
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![warn(clippy::integer_arithmetic)]
#![warn(clippy::arithmetic_side_effects)]

mod backtrace;
#[cfg(target_os = "linux")]
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
Ok(0)
}

#[allow(non_snake_case, clippy::integer_arithmetic)]
#[allow(non_snake_case, clippy::arithmetic_side_effects)]
fn GetSystemTimeAsFileTime(
&mut self,
LPFILETIME_op: &OpTy<'tcx, Provenance>,
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl<'tcx> Default for TlsData<'tcx> {
impl<'tcx> TlsData<'tcx> {
/// Generate a new TLS key with the given destructor.
/// `max_size` determines the integer size the key has to fit in.
#[allow(clippy::integer_arithmetic)]
#[allow(clippy::arithmetic_side_effects)]
pub fn create_tls_key(
&mut self,
dtor: Option<ty::Instance<'tcx>>,
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ pub struct DirHandler {
}

impl DirHandler {
#[allow(clippy::integer_arithmetic)]
#[allow(clippy::arithmetic_side_effects)]
fn insert_new(&mut self, read_dir: ReadDir) -> u64 {
let id = self.next_id;
self.next_id += 1;
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/unix/linux/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ pub fn futex<'tcx>(
// before doing the syscall.
this.atomic_fence(AtomicFenceOrd::SeqCst)?;
let mut n = 0;
#[allow(clippy::integer_arithmetic)]
#[allow(clippy::arithmetic_side_effects)]
for _ in 0..val {
if let Some(thread) = this.futex_wake(addr_usize, bitset) {
this.unblock_thread(thread);
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/windows/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
.copied()
.scan(Size::ZERO, |a, x| {
let res = Some(*a);
*a += x;
*a = a.checked_add(x, this).unwrap();
res
})
.collect();
Expand Down
Loading