Skip to content

Commit

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

r? `@ghost`
  • Loading branch information
bors committed Dec 26, 2023
2 parents a815c3b + 4658d38 commit ea7ef7b
Show file tree
Hide file tree
Showing 13 changed files with 155 additions and 150 deletions.
17 changes: 13 additions & 4 deletions src/tools/miri/miri-script/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ impl Command {
println!(
// Open PR with `subtree update` title to silence the `no-merges` triagebot check
// See https://github.com/rust-lang/rust/pull/114157
" https://github.com/rust-lang/rust/compare/{github_user}:{branch}?quick_pull=1&title=Miri+subtree+update"
" https://github.com/rust-lang/rust/compare/{github_user}:{branch}?quick_pull=1&title=Miri+subtree+update&body=r?+@ghost"
);

drop(josh);
Expand Down Expand Up @@ -478,7 +478,11 @@ impl Command {
// Scan for "--target" to overwrite the "MIRI_TEST_TARGET" env var so
// that we set the MIRI_SYSROOT up the right way.
use itertools::Itertools;
let target = flags.iter().tuple_windows().find(|(first, _)| first == &"--target");
let target = flags
.iter()
.take_while(|arg| *arg != "--")
.tuple_windows()
.find(|(first, _)| *first == "--target");
if let Some((_, target)) = target {
// Found it!
e.sh.set_var("MIRI_TEST_TARGET", target);
Expand All @@ -487,6 +491,10 @@ impl Command {
let miriflags = e.sh.var("MIRIFLAGS").unwrap_or_default();
e.sh.set_var("MIRIFLAGS", format!("{miriflags} --target {target}"));
}
// Scan for "--edition" (we'll set one ourselves if that flag is not present).
let have_edition =
flags.iter().take_while(|arg| *arg != "--").any(|arg| *arg == "--edition");

// Prepare a sysroot.
e.build_miri_sysroot(/* quiet */ true)?;

Expand All @@ -496,15 +504,16 @@ impl Command {
let miri_flags = flagsplit(&miri_flags);
let toolchain = &e.toolchain;
let extra_flags = &e.cargo_extra_flags;
let edition_flags = (!have_edition).then_some("--edition=2021"); // keep in sync with `compiletest.rs`.`
if dep {
cmd!(
e.sh,
"cargo +{toolchain} --quiet test --test compiletest {extra_flags...} --manifest-path {miri_manifest} -- --miri-run-dep-mode {miri_flags...} {flags...}"
"cargo +{toolchain} --quiet test --test compiletest {extra_flags...} --manifest-path {miri_manifest} -- --miri-run-dep-mode {miri_flags...} {edition_flags...} {flags...}"
).quiet().run()?;
} else {
cmd!(
e.sh,
"cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags...} {flags...}"
"cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags...} {edition_flags...} {flags...}"
).quiet().run()?;
}
Ok(())
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 @@
604f185fae9a4b0edf7e28f616a0f53880f8f074
2271c26e4a8e062bb00d709d0ccb5846e0c341b9
48 changes: 48 additions & 0 deletions src/tools/miri/src/shims/intrinsics/simd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,54 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
}
}
"masked_load" => {
let [mask, ptr, default] = check_arg_count(args)?;
let (mask, mask_len) = this.operand_to_simd(mask)?;
let ptr = this.read_pointer(ptr)?;
let (default, default_len) = this.operand_to_simd(default)?;
let (dest, dest_len) = this.place_to_simd(dest)?;

assert_eq!(dest_len, mask_len);
assert_eq!(dest_len, default_len);

for i in 0..dest_len {
let mask = this.read_immediate(&this.project_index(&mask, i)?)?;
let default = this.read_immediate(&this.project_index(&default, i)?)?;
let dest = this.project_index(&dest, i)?;

let val = if simd_element_to_bool(mask)? {
// Size * u64 is implemented as always checked
#[allow(clippy::arithmetic_side_effects)]
let ptr = ptr.wrapping_offset(dest.layout.size * i, this);
let place = this.ptr_to_mplace(ptr, dest.layout);
this.read_immediate(&place)?
} else {
default
};
this.write_immediate(*val, &dest)?;
}
}
"masked_store" => {
let [mask, ptr, vals] = check_arg_count(args)?;
let (mask, mask_len) = this.operand_to_simd(mask)?;
let ptr = this.read_pointer(ptr)?;
let (vals, vals_len) = this.operand_to_simd(vals)?;

assert_eq!(mask_len, vals_len);

for i in 0..vals_len {
let mask = this.read_immediate(&this.project_index(&mask, i)?)?;
let val = this.read_immediate(&this.project_index(&vals, i)?)?;

if simd_element_to_bool(mask)? {
// Size * u64 is implemented as always checked
#[allow(clippy::arithmetic_side_effects)]
let ptr = ptr.wrapping_offset(val.layout.size * i, this);
let place = this.ptr_to_mplace(ptr, val.layout);
this.write_immediate(*val, &place)?
};
}
}

name => throw_unsup_format!("unimplemented intrinsic: `simd_{name}`"),
}
Expand Down
7 changes: 2 additions & 5 deletions src/tools/miri/src/shims/unix/linux/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
) -> InterpResult<'tcx, Scalar<Provenance>> {
let this = self.eval_context_mut();

let old_address = this.read_target_usize(old_address)?;
let old_address = this.read_pointer(old_address)?;
let old_size = this.read_target_usize(old_size)?;
let new_size = this.read_target_usize(new_size)?;
let flags = this.read_scalar(flags)?.to_i32()?;

// old_address must be a multiple of the page size
#[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero
if old_address % this.machine.page_size != 0 || new_size == 0 {
if old_address.addr().bytes() % this.machine.page_size != 0 || new_size == 0 {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
return Ok(this.eval_libc("MAP_FAILED"));
}
Expand All @@ -41,7 +41,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
return Ok(this.eval_libc("MAP_FAILED"));
}

let old_address = Machine::ptr_from_addr_cast(this, old_address)?;
let align = this.machine.page_align();
let ptr = this.reallocate_ptr(
old_address,
Expand All @@ -59,8 +58,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
)
.unwrap();
}
// Memory mappings are always exposed
Machine::expose_ptr(this, ptr)?;

Ok(Scalar::from_pointer(ptr, this))
}
Expand Down
45 changes: 14 additions & 31 deletions src/tools/miri/src/shims/unix/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@
//! mmap/munmap behave a lot like alloc/dealloc, and for simple use they are exactly
//! equivalent. That is the only part we support: no MAP_FIXED or MAP_SHARED or anything
//! else that goes beyond a basic allocation API.
//!
//! Note that in addition to only supporting malloc-like calls to mmap, we only support free-like
//! calls to munmap, but for a very different reason. In principle, according to the man pages, it
//! is possible to unmap arbitrary regions of address space. But in a high-level language like Rust
//! this amounts to partial deallocation, which LLVM does not support. So any attempt to call our
//! munmap shim which would partily unmap a region of address space previously mapped by mmap will
//! report UB.

use crate::{helpers::round_to_next_multiple_of, *};
use rustc_target::abi::Size;
Expand Down Expand Up @@ -100,8 +107,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
std::iter::repeat(0u8).take(usize::try_from(map_length).unwrap()),
)
.unwrap();
// Memory mappings don't use provenance, and are always exposed.
Machine::expose_ptr(this, ptr)?;

Ok(Scalar::from_pointer(ptr, this))
}
Expand All @@ -113,43 +118,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
) -> InterpResult<'tcx, Scalar<Provenance>> {
let this = self.eval_context_mut();

let addr = this.read_target_usize(addr)?;
let addr = this.read_pointer(addr)?;
let length = this.read_target_usize(length)?;

// addr must be a multiple of the page size
// addr must be a multiple of the page size, but apart from that munmap is just implemented
// as a dealloc.
#[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero
if addr % this.machine.page_size != 0 {
if addr.addr().bytes() % this.machine.page_size != 0 {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
return Ok(Scalar::from_i32(-1));
}

let length = round_to_next_multiple_of(length, this.machine.page_size);

let ptr = Machine::ptr_from_addr_cast(this, addr)?;

let Ok(ptr) = ptr.into_pointer_or_addr() else {
throw_unsup_format!("Miri only supports munmap on memory allocated directly by mmap");
};
let Some((alloc_id, offset, _prov)) = Machine::ptr_get_alloc(this, ptr) else {
throw_unsup_format!("Miri only supports munmap on memory allocated directly by mmap");
};

// Elsewhere in this function we are careful to check what we can and throw an unsupported
// error instead of Undefined Behavior when use of this function falls outside of the
// narrow scope we support. We deliberately do not check the MemoryKind of this allocation,
// because we want to report UB on attempting to unmap memory that Rust "understands", such
// the stack, heap, or statics.
let (_kind, alloc) = this.memory.alloc_map().get(alloc_id).unwrap();
if offset != Size::ZERO || alloc.len() as u64 != length {
throw_unsup_format!(
"Miri only supports munmap calls that exactly unmap a region previously returned by mmap"
);
}

let len = Size::from_bytes(alloc.len() as u64);
let length = Size::from_bytes(round_to_next_multiple_of(length, this.machine.page_size));
this.deallocate_ptr(
ptr.into(),
Some((len, this.machine.page_align())),
addr,
Some((length, this.machine.page_align())),
MemoryKind::Machine(MiriMemoryKind::Mmap),
)?;

Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/compiletest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ fn test_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) ->
mode,
program,
out_dir: PathBuf::from(std::env::var_os("CARGO_TARGET_DIR").unwrap()).join("ui"),
edition: Some("2021".into()),
edition: Some("2021".into()), // keep in sync with `./miri run`
threads: std::env::var("MIRI_TEST_THREADS")
.ok()
.map(|threads| NonZeroUsize::new(threads.parse().unwrap()).unwrap()),
Expand Down
17 changes: 1 addition & 16 deletions src/tools/miri/tests/fail-dep/shims/mmap_use_after_munmap.stderr
Original file line number Diff line number Diff line change
@@ -1,18 +1,3 @@
warning: integer-to-pointer cast
--> $DIR/mmap_use_after_munmap.rs:LL:CC
|
LL | libc::munmap(ptr, 4096);
| ^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast
|
= help: This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`,
= help: which means that Miri might miss pointer bugs in this program.
= help: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation.
= help: To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead.
= help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics.
= help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning.
= note: BACKTRACE:
= note: inside `main` at $DIR/mmap_use_after_munmap.rs:LL:CC

error: Undefined Behavior: memory access failed: ALLOC has been freed, so this pointer is dangling
--> $DIR/mmap_use_after_munmap.rs:LL:CC
|
Expand Down Expand Up @@ -43,5 +28,5 @@ LL | libc::munmap(ptr, 4096);

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error; 1 warning emitted
error: aborting due to 1 previous error

22 changes: 0 additions & 22 deletions src/tools/miri/tests/fail-dep/shims/munmap.rs

This file was deleted.

39 changes: 0 additions & 39 deletions src/tools/miri/tests/fail-dep/shims/munmap.stderr

This file was deleted.

8 changes: 5 additions & 3 deletions src/tools/miri/tests/fail-dep/shims/munmap_partial.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//! Our mmap/munmap support is a thin wrapper over Interpcx::allocate_ptr. Since the underlying
//! layer has much more UB than munmap does, we need to be sure we throw an unsupported error here.
//! The man pages for mmap/munmap suggest that it is possible to partly unmap a previously-mapped
//! region of addres space, but to LLVM that would be partial deallocation, which LLVM does not
//! support. So even though the man pages say this sort of use is possible, we must report UB.
//@ignore-target-windows: No libc on Windows
//@normalize-stderr-test: "size [0-9]+ and alignment" -> "size SIZE and alignment"

fn main() {
unsafe {
Expand All @@ -13,6 +15,6 @@ fn main() {
0,
);
libc::munmap(ptr, 1);
//~^ ERROR: unsupported operation
//~^ ERROR: Undefined Behavior
}
}
24 changes: 5 additions & 19 deletions src/tools/miri/tests/fail-dep/shims/munmap_partial.stderr
Original file line number Diff line number Diff line change
@@ -1,29 +1,15 @@
warning: integer-to-pointer cast
error: Undefined Behavior: incorrect layout on deallocation: ALLOC has size SIZE and alignment ALIGN, but gave size SIZE and alignment ALIGN
--> $DIR/munmap_partial.rs:LL:CC
|
LL | libc::munmap(ptr, 1);
| ^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast
| ^^^^^^^^^^^^^^^^^^^^ incorrect layout on deallocation: ALLOC has size SIZE and alignment ALIGN, but gave size SIZE and alignment ALIGN
|
= help: This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`,
= help: which means that Miri might miss pointer bugs in this program.
= help: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation.
= help: To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead.
= help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics.
= help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning.
= note: BACKTRACE:
= note: inside `main` at $DIR/munmap_partial.rs:LL:CC

error: unsupported operation: Miri only supports munmap calls that exactly unmap a region previously returned by mmap
--> $DIR/munmap_partial.rs:LL:CC
|
LL | libc::munmap(ptr, 1);
| ^^^^^^^^^^^^^^^^^^^^ Miri only supports munmap calls that exactly unmap a region previously returned by mmap
|
= help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside `main` at $DIR/munmap_partial.rs:LL:CC

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error; 1 warning emitted
error: aborting due to 1 previous error

5 changes: 2 additions & 3 deletions src/tools/miri/tests/pass-dep/shims/mmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ fn test_mmap() {
}
assert!(slice.iter().all(|b| *b == 1));

// Ensure that we can munmap with just an integer
let just_an_address = ptr::invalid_mut(ptr.addr());
let res = unsafe { libc::munmap(just_an_address, page_size) };
// Ensure that we can munmap
let res = unsafe { libc::munmap(ptr, page_size) };
assert_eq!(res, 0i32);

// Test all of our error conditions
Expand Down
Loading

0 comments on commit ea7ef7b

Please sign in to comment.