Skip to content

Miri subtree update #119315

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 21 commits into from
Dec 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
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