diff --git a/src/shims/unix/linux/mem.rs b/src/shims/unix/linux/mem.rs index 026fd6ae5e..7bbe5618df 100644 --- a/src/shims/unix/linux/mem.rs +++ b/src/shims/unix/linux/mem.rs @@ -15,14 +15,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ) -> InterpResult<'tcx, Scalar> { 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")); } @@ -41,7 +41,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); } - let old_address = Machine::ptr_from_addr_cast(this, old_address)?; let align = this.machine.page_align(); let ptr = this.reallocate_ptr( old_address, @@ -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)) } diff --git a/src/shims/unix/mem.rs b/src/shims/unix/mem.rs index cf36d4b191..88eadb5f46 100644 --- a/src/shims/unix/mem.rs +++ b/src/shims/unix/mem.rs @@ -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; @@ -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)) } @@ -113,43 +118,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ) -> InterpResult<'tcx, Scalar> { 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), )?; diff --git a/tests/fail-dep/shims/mmap_use_after_munmap.stderr b/tests/fail-dep/shims/mmap_use_after_munmap.stderr index 21b4baa500..49f2b84baa 100644 --- a/tests/fail-dep/shims/mmap_use_after_munmap.stderr +++ b/tests/fail-dep/shims/mmap_use_after_munmap.stderr @@ -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 | @@ -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 diff --git a/tests/fail-dep/shims/munmap.rs b/tests/fail-dep/shims/munmap.rs deleted file mode 100644 index 453437a06c..0000000000 --- a/tests/fail-dep/shims/munmap.rs +++ /dev/null @@ -1,22 +0,0 @@ -//@compile-flags: -Zmiri-disable-isolation -//@ignore-target-windows: No libc on Windows - -#![feature(rustc_private)] -#![feature(strict_provenance)] - -use std::ptr; - -fn main() { - // Linux specifies that it is not an error if the specified range does not contain any pages. - // But we simply do not support such calls. This test checks that we report this as - // unsupported, not Undefined Behavior. - let res = unsafe { - libc::munmap( - //~^ ERROR: unsupported operation - // Some high address we surely have not allocated anything at - ptr::invalid_mut(1 << 30), - 4096, - ) - }; - assert_eq!(res, 0); -} diff --git a/tests/fail-dep/shims/munmap.stderr b/tests/fail-dep/shims/munmap.stderr deleted file mode 100644 index f17473677f..0000000000 --- a/tests/fail-dep/shims/munmap.stderr +++ /dev/null @@ -1,39 +0,0 @@ -warning: integer-to-pointer cast - --> $DIR/munmap.rs:LL:CC - | -LL | / libc::munmap( -LL | | -LL | | // Some high address we surely have not allocated anything at -LL | | ptr::invalid_mut(1 << 30), -LL | | 4096, -LL | | ) - | |_________^ 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/munmap.rs:LL:CC - -error: unsupported operation: Miri only supports munmap on memory allocated directly by mmap - --> $DIR/munmap.rs:LL:CC - | -LL | / libc::munmap( -LL | | -LL | | // Some high address we surely have not allocated anything at -LL | | ptr::invalid_mut(1 << 30), -LL | | 4096, -LL | | ) - | |_________^ Miri only supports munmap on memory allocated directly 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 - = note: BACKTRACE: - = note: inside `main` at $DIR/munmap.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 - diff --git a/tests/fail-dep/shims/munmap_partial.rs b/tests/fail-dep/shims/munmap_partial.rs index 938850ee28..04573d7e52 100644 --- a/tests/fail-dep/shims/munmap_partial.rs +++ b/tests/fail-dep/shims/munmap_partial.rs @@ -1,5 +1,6 @@ -//! 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 fn main() { @@ -13,6 +14,6 @@ fn main() { 0, ); libc::munmap(ptr, 1); - //~^ ERROR: unsupported operation + //~^ ERROR: Undefined Behavior } } diff --git a/tests/fail-dep/shims/munmap_partial.stderr b/tests/fail-dep/shims/munmap_partial.stderr index 14eb9d3205..90d8fcfb34 100644 --- a/tests/fail-dep/shims/munmap_partial.stderr +++ b/tests/fail-dep/shims/munmap_partial.stderr @@ -1,29 +1,15 @@ -warning: integer-to-pointer cast +error: Undefined Behavior: incorrect layout on deallocation: ALLOC has size 8192 and alignment ALIGN, but gave size 4096 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 8192 and alignment ALIGN, but gave size 4096 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 diff --git a/tests/pass-dep/shims/mmap.rs b/tests/pass-dep/shims/mmap.rs index 03ffc3b389..72802a7f8c 100644 --- a/tests/pass-dep/shims/mmap.rs +++ b/tests/pass-dep/shims/mmap.rs @@ -28,9 +28,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); }