diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index e90d3732ca577..58ec4e10856df 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -99b7134389e9766462601a2fc4013840b9d31745 +a526d7ce45fd2284e0e7c7556ccba2425b9d25e5 diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 9f3fa075f38f6..25b154a8206c9 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -620,6 +620,14 @@ fn main() { "-Zmiri-unique-is-unique only has an effect when -Zmiri-tree-borrows is also used" ); } + // Tree Borrows + permissive provenance does not work. + if miri_config.provenance_mode == ProvenanceMode::Permissive + && matches!(miri_config.borrow_tracker, Some(BorrowTrackerMethod::TreeBorrows)) + { + show_error!( + "Tree Borrows does not support integer-to-pointer casts, and is hence not compatible with permissive provenance" + ); + } debug!("rustc arguments: {:?}", rustc_args); debug!("crate arguments: {:?}", miri_config.args); diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index c9e7e300593bc..d537a7fbc1798 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -232,6 +232,10 @@ impl GlobalStateInner { pub fn remove_unreachable_allocs(&mut self, allocs: &LiveAllocs<'_, '_>) { self.root_ptr_tags.retain(|id, _| allocs.is_live(*id)); } + + pub fn borrow_tracker_method(&self) -> BorrowTrackerMethod { + self.borrow_tracker_method + } } /// Which borrow tracking method to use diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 603733f9dc000..1d75486a78189 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -5,6 +5,7 @@ pub mod diagnostics; mod item; mod stack; +use std::cell::RefCell; use std::cmp; use std::fmt::Write; use std::mem; @@ -820,7 +821,19 @@ trait EvalContextPrivExt<'tcx, 'ecx>: crate::MiriInterpCxExt<'tcx> { // See https://github.com/rust-lang/unsafe-code-guidelines/issues/276. let size = match size { Some(size) => size, - None => return Ok(place.clone()), + None => { + // The first time this happens, show a warning. + thread_local! { static WARNING_SHOWN: RefCell = const { RefCell::new(false) }; } + WARNING_SHOWN.with_borrow_mut(|shown| { + if *shown { + return; + } + // Not yet shown. Show it! + *shown = true; + this.emit_diagnostic(NonHaltingDiagnostic::ExternTypeReborrow); + }); + return Ok(place.clone()); + } }; // Compute new borrow. diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 86074384084d5..123d4b407fb4c 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -141,8 +141,15 @@ impl<'tcx> NewPermission { ) -> Option { let ty_is_freeze = pointee.is_freeze(*cx.tcx, cx.param_env()); let ty_is_unpin = pointee.is_unpin(*cx.tcx, cx.param_env()); + let is_protected = kind == RetagKind::FnEntry; + // As demonstrated by `tests/fail/tree_borrows/reservedim_spurious_write.rs`, + // interior mutability and protectors interact poorly. + // To eliminate the case of Protected Reserved IM we override interior mutability + // in the case of a protected reference: protected references are always considered + // "freeze". let initial_state = match mutability { - Mutability::Mut if ty_is_unpin => Permission::new_reserved(ty_is_freeze), + Mutability::Mut if ty_is_unpin => + Permission::new_reserved(ty_is_freeze || is_protected), Mutability::Not if ty_is_freeze => Permission::new_frozen(), // Raw pointers never enter this function so they are not handled. // However raw pointers are not the only pointers that take the parent @@ -151,7 +158,7 @@ impl<'tcx> NewPermission { _ => return None, }; - let protector = (kind == RetagKind::FnEntry).then_some(ProtectorKind::StrongProtector); + let protector = is_protected.then_some(ProtectorKind::StrongProtector); Some(Self { zero_size: false, initial_state, protector }) } diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs index 7aa9c3e862bcc..8e23257b6c006 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -22,6 +22,11 @@ enum PermissionPriv { /// - foreign-read then child-write is UB due to `conflicted`, /// - child-write then foreign-read is UB since child-write will activate and then /// foreign-read disables a protected `Active`, which is UB. + /// + /// Note: since the discovery of `tests/fail/tree_borrows/reservedim_spurious_write.rs`, + /// `ty_is_freeze` does not strictly mean that the type has no interior mutability, + /// it could be an interior mutable type that lost its interior mutability privileges + /// when retagged with a protector. Reserved { ty_is_freeze: bool, conflicted: bool }, /// represents: a unique pointer; /// allows: child reads, child writes; @@ -141,6 +146,12 @@ mod transition { /// non-protected interior mutable `Reserved` which stay the same. fn foreign_write(state: PermissionPriv, protected: bool) -> Option { Some(match state { + // FIXME: since the fix related to reservedim_spurious_write, it is now possible + // to express these transitions of the state machine without an explicit dependency + // on `protected`: because `ty_is_freeze: false` implies `!protected` then + // the line handling `Reserved { .. } if protected` could be deleted. + // This will however require optimizations to the exhaustive tests because + // fewer initial conditions are valid. Reserved { .. } if protected => Disabled, res @ Reserved { ty_is_freeze: false, .. } => res, _ => Disabled, diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 647d7d44bb1bb..1bed55743d4af 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -130,6 +130,7 @@ pub enum NonHaltingDiagnostic { WeakMemoryOutdatedLoad { ptr: Pointer, }, + ExternTypeReborrow, } /// Level of Miri specific diagnostics @@ -139,6 +140,15 @@ pub enum DiagLevel { Note, } +/// Generate a note/help text without a span. +macro_rules! note { + ($($tt:tt)*) => { (None, format!($($tt)*)) }; +} +/// Generate a note/help text with a span. +macro_rules! note_span { + ($span:expr, $($tt:tt)*) => { (Some($span), format!($($tt)*)) }; +} + /// Attempts to prune a stacktrace to omit the Rust runtime, and returns a bool indicating if any /// frames were pruned. If the stacktrace does not have any local frames, we conclude that it must /// be pointing to a problem in the Rust runtime itself, and do not prune it at all. @@ -227,38 +237,38 @@ pub fn report_error<'tcx>( let helps = match info { UnsupportedInIsolation(_) => vec![ - (None, format!("set `MIRIFLAGS=-Zmiri-disable-isolation` to disable isolation;")), - (None, format!("or set `MIRIFLAGS=-Zmiri-isolation-error=warn` to make Miri return an error code from isolated operations (if supported for that operation) and continue with a warning")), + note!("set `MIRIFLAGS=-Zmiri-disable-isolation` to disable isolation;"), + note!("or set `MIRIFLAGS=-Zmiri-isolation-error=warn` to make Miri return an error code from isolated operations (if supported for that operation) and continue with a warning"), ], UnsupportedForeignItem(_) => { vec![ - (None, format!("if this is a basic API commonly used on this target, please report an issue with Miri")), - (None, format!("however, note that Miri does not aim to support every FFI function out there; for instance, we will not support APIs for things such as GUIs, scripting languages, or databases")), + note!("if this is a basic API commonly used on this target, please report an issue with Miri"), + note!("however, note that Miri does not aim to support every FFI function out there; for instance, we will not support APIs for things such as GUIs, scripting languages, or databases"), ] } StackedBorrowsUb { help, history, .. } => { msg.extend(help.clone()); let mut helps = vec![ - (None, format!("this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental")), - (None, format!("see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information")), + note!("this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental"), + note!("see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information"), ]; if let Some(TagHistory {created, invalidated, protected}) = history.clone() { helps.push((Some(created.1), created.0)); if let Some((msg, span)) = invalidated { - helps.push((Some(span), msg)); + helps.push(note_span!(span, "{msg}")); } if let Some((protector_msg, protector_span)) = protected { - helps.push((Some(protector_span), protector_msg)); + helps.push(note_span!(protector_span, "{protector_msg}")); } } helps }, TreeBorrowsUb { title: _, details, history } => { let mut helps = vec![ - (None, format!("this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental")) + note!("this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental") ]; for m in details { - helps.push((None, m.clone())); + helps.push(note!("{m}")); } for event in history.events.clone() { helps.push(event); @@ -267,26 +277,26 @@ pub fn report_error<'tcx>( } MultipleSymbolDefinitions { first, first_crate, second, second_crate, .. } => vec![ - (Some(*first), format!("it's first defined here, in crate `{first_crate}`")), - (Some(*second), format!("then it's defined here again, in crate `{second_crate}`")), + note_span!(*first, "it's first defined here, in crate `{first_crate}`"), + note_span!(*second, "then it's defined here again, in crate `{second_crate}`"), ], SymbolShimClashing { link_name, span } => - vec![(Some(*span), format!("the `{link_name}` symbol is defined here"))], + vec![note_span!(*span, "the `{link_name}` symbol is defined here")], Int2PtrWithStrictProvenance => - vec![(None, format!("use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead"))], + vec![note!("use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead")], DataRace { op1, extra, retag_explain, .. } => { - let mut helps = vec![(Some(op1.span), format!("and (1) occurred earlier here"))]; + let mut helps = vec![note_span!(op1.span, "and (1) occurred earlier here")]; if let Some(extra) = extra { - helps.push((None, format!("{extra}"))); - helps.push((None, format!("see https://doc.rust-lang.org/nightly/std/sync/atomic/index.html#memory-model-for-atomic-accesses for more information about the Rust memory model"))); + helps.push(note!("{extra}")); + helps.push(note!("see https://doc.rust-lang.org/nightly/std/sync/atomic/index.html#memory-model-for-atomic-accesses for more information about the Rust memory model")); } if *retag_explain { - helps.push((None, "retags occur on all (re)borrows and as well as when references are copied or moved".to_owned())); - helps.push((None, "retags permit optimizations that insert speculative reads or writes".to_owned())); - helps.push((None, "therefore from the perspective of data races, a retag has the same implications as a read or write".to_owned())); + helps.push(note!("retags occur on all (re)borrows and as well as when references are copied or moved")); + helps.push(note!("retags permit optimizations that insert speculative reads or writes")); + helps.push(note!("therefore from the perspective of data races, a retag has the same implications as a read or write")); } - helps.push((None, format!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior"))); - helps.push((None, format!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information"))); + helps.push(note!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior")); + helps.push(note!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information")); helps } , @@ -331,32 +341,32 @@ pub fn report_error<'tcx>( let helps = match e.kind() { Unsupported(_) => vec![ - (None, format!("this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support")), + note!("this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support"), ], UndefinedBehavior(AlignmentCheckFailed { .. }) if ecx.machine.check_alignment == AlignmentCheck::Symbolic => vec![ - (None, format!("this usually indicates that your program performed an invalid operation and caused Undefined Behavior")), - (None, format!("but due to `-Zmiri-symbolic-alignment-check`, alignment errors can also be false positives")), + note!("this usually indicates that your program performed an invalid operation and caused Undefined Behavior"), + note!("but due to `-Zmiri-symbolic-alignment-check`, alignment errors can also be false positives"), ], UndefinedBehavior(info) => { let mut helps = vec![ - (None, format!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior")), - (None, format!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information")), + note!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior"), + note!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information"), ]; match info { PointerUseAfterFree(alloc_id, _) | PointerOutOfBounds { alloc_id, .. } => { if let Some(span) = ecx.machine.allocated_span(*alloc_id) { - helps.push((Some(span), format!("{:?} was allocated here:", alloc_id))); + helps.push(note_span!(span, "{:?} was allocated here:", alloc_id)); } if let Some(span) = ecx.machine.deallocated_span(*alloc_id) { - helps.push((Some(span), format!("{:?} was deallocated here:", alloc_id))); + helps.push(note_span!(span, "{:?} was deallocated here:", alloc_id)); } } AbiMismatchArgument { .. } | AbiMismatchReturn { .. } => { - helps.push((None, format!("this means these two types are not *guaranteed* to be ABI-compatible across all targets"))); - helps.push((None, format!("if you think this code should be accepted anyway, please report an issue with Miri"))); + helps.push(note!("this means these two types are not *guaranteed* to be ABI-compatible across all targets")); + helps.push(note!("if you think this code should be accepted anyway, please report an issue with Miri")); } _ => {}, } @@ -593,6 +603,8 @@ impl<'tcx> MiriMachine<'tcx> { RejectedIsolatedOp(_) => ("operation rejected by isolation".to_string(), DiagLevel::Warning), Int2Ptr { .. } => ("integer-to-pointer cast".to_string(), DiagLevel::Warning), + ExternTypeReborrow => + ("reborrow of reference to `extern type`".to_string(), DiagLevel::Warning), CreatedPointerTag(..) | PoppedPointerTag(..) | CreatedCallId(..) @@ -630,51 +642,56 @@ impl<'tcx> MiriMachine<'tcx> { Int2Ptr { .. } => format!("integer-to-pointer cast"), WeakMemoryOutdatedLoad { ptr } => format!("weak memory emulation: outdated value returned from load at {ptr}"), + ExternTypeReborrow => + format!("reborrow of a reference to `extern type` is not properly supported"), }; let notes = match &e { ProgressReport { block_count } => { - // It is important that each progress report is slightly different, since - // identical diagnostics are being deduplicated. - vec![(None, format!("so far, {block_count} basic blocks have been executed"))] + vec![note!("so far, {block_count} basic blocks have been executed")] } _ => vec![], }; let helps = match &e { - Int2Ptr { details: true } => - vec![ - ( - None, - format!( - "This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program." - ), + Int2Ptr { details: true } => { + let mut v = vec![ + note!( + "this program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program" + ), + note!( + "see https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.html for more details on that operation" ), - ( - None, - format!( - "See https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.html for more details on that operation." - ), + note!( + "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" ), - ( - None, - format!( - "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." - ), + note!( + "you can then set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics" ), - ( - None, - format!( - "You can then set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics." - ), + ]; + if self.borrow_tracker.as_ref().is_some_and(|b| { + matches!(b.borrow().borrow_tracker_method(), BorrowTrackerMethod::TreeBorrows) + }) { + v.push( + note!("Tree Borrows does not support integer-to-pointer casts, so the program is likely to go wrong when this pointer gets used") + ); + } else { + v.push( + note!("alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning") + ); + } + v + } + ExternTypeReborrow => { + vec![ + note!( + "`extern type` are not compatible with the Stacked Borrows aliasing model implemented by Miri; Miri may miss bugs in this code" ), - ( - None, - format!( - "Alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning." - ), + note!( + "try running with `MIRIFLAGS=-Zmiri-tree-borrows` to use the more permissive but also even more experimental Tree Borrows aliasing checks instead" ), - ], + ] + } _ => vec![], }; diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index adb8459356165..e492793a65135 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -660,7 +660,7 @@ impl<'tcx> MiriMachine<'tcx> { tls: TlsData::default(), isolated_op: config.isolated_op, validate: config.validate, - fds: shims::FdTable::new(config.mute_stdout_stderr), + fds: shims::FdTable::init(config.mute_stdout_stderr), dirs: Default::default(), layouts, threads, diff --git a/src/tools/miri/src/shims/env.rs b/src/tools/miri/src/shims/env.rs index 7ad395cccb799..6586ea8e48cff 100644 --- a/src/tools/miri/src/shims/env.rs +++ b/src/tools/miri/src/shims/env.rs @@ -108,4 +108,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { EnvVars::Windows(vars) => vars.get(name), } } + + fn get_pid(&self) -> u32 { + let this = self.eval_context_ref(); + if this.machine.communicate() { std::process::id() } else { 1000 } + } } diff --git a/src/tools/miri/src/shims/unix/env.rs b/src/tools/miri/src/shims/unix/env.rs index 405431f4327db..3b8ad65195b8a 100644 --- a/src/tools/miri/src/shims/unix/env.rs +++ b/src/tools/miri/src/shims/unix/env.rs @@ -274,12 +274,23 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); this.assert_target_os_is_unix("getpid"); - this.check_no_isolation("`getpid`")?; - // The reason we need to do this wacky of a conversion is because // `libc::getpid` returns an i32, however, `std::process::id()` return an u32. // So we un-do the conversion that stdlib does and turn it back into an i32. #[allow(clippy::cast_possible_wrap)] - Ok(std::process::id() as i32) + Ok(this.get_pid() as i32) + } + + fn linux_gettid(&mut self) -> InterpResult<'tcx, i32> { + let this = self.eval_context_ref(); + this.assert_target_os("linux", "gettid"); + + let index = this.machine.threads.active_thread().to_u32(); + + // Compute a TID for this thread, ensuring that the main thread has PID == TID. + let tid = this.get_pid().strict_add(index); + + #[allow(clippy::cast_possible_wrap)] + Ok(tid as i32) } } diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index 8fb046b5e64cf..0fffecd99d5cb 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -36,6 +36,30 @@ pub trait FileDescription: std::fmt::Debug + Any { throw_unsup_format!("cannot write to {}", self.name()); } + /// Reads as much as possible into the given buffer from a given offset, + /// and returns the number of bytes read. + fn pread<'tcx>( + &mut self, + _communicate_allowed: bool, + _bytes: &mut [u8], + _offset: u64, + _ecx: &mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + throw_unsup_format!("cannot pread from {}", self.name()); + } + + /// Writes as much as possible from the given buffer starting at a given offset, + /// and returns the number of bytes written. + fn pwrite<'tcx>( + &mut self, + _communicate_allowed: bool, + _bytes: &[u8], + _offset: u64, + _ecx: &mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + throw_unsup_format!("cannot pwrite to {}", self.name()); + } + /// Seeks to the given offset (which can be relative to the beginning, end, or current position). /// Returns the new position from the start of the stream. fn seek<'tcx>( @@ -168,10 +192,6 @@ impl FileDescription for NullOutput { pub struct FileDescriptor(Rc>>); impl FileDescriptor { - pub fn new(fd: T) -> Self { - FileDescriptor(Rc::new(RefCell::new(Box::new(fd)))) - } - pub fn borrow(&self) -> Ref<'_, dyn FileDescription> { Ref::map(self.0.borrow(), |fd| fd.as_ref()) } @@ -203,20 +223,25 @@ impl VisitProvenance for FdTable { } impl FdTable { - pub(crate) fn new(mute_stdout_stderr: bool) -> FdTable { - let mut fds: BTreeMap<_, FileDescriptor> = BTreeMap::new(); - fds.insert(0i32, FileDescriptor::new(io::stdin())); + fn new() -> Self { + FdTable { fds: BTreeMap::new() } + } + pub(crate) fn init(mute_stdout_stderr: bool) -> FdTable { + let mut fds = FdTable::new(); + fds.insert_fd(io::stdin()); if mute_stdout_stderr { - fds.insert(1i32, FileDescriptor::new(NullOutput)); - fds.insert(2i32, FileDescriptor::new(NullOutput)); + assert_eq!(fds.insert_fd(NullOutput), 1); + assert_eq!(fds.insert_fd(NullOutput), 2); } else { - fds.insert(1i32, FileDescriptor::new(io::stdout())); - fds.insert(2i32, FileDescriptor::new(io::stderr())); + assert_eq!(fds.insert_fd(io::stdout()), 1); + assert_eq!(fds.insert_fd(io::stderr()), 2); } - FdTable { fds } + fds } - pub fn insert_fd(&mut self, file_handle: FileDescriptor) -> i32 { + /// Insert a file descriptor to the FdTable. + pub fn insert_fd(&mut self, fd: T) -> i32 { + let file_handle = FileDescriptor(Rc::new(RefCell::new(Box::new(fd)))); self.insert_fd_with_min_fd(file_handle, 0) } @@ -380,7 +405,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { Ok((-1).into()) } - fn read(&mut self, fd: i32, buf: Pointer, count: u64) -> InterpResult<'tcx, i64> { + /// Read data from `fd` into buffer specified by `buf` and `count`. + /// + /// If `offset` is `None`, reads data from current cursor position associated with `fd` + /// and updates cursor position on completion. Otherwise, reads from the specified offset + /// and keeps the cursor unchanged. + fn read( + &mut self, + fd: i32, + buf: Pointer, + count: u64, + offset: Option, + ) -> InterpResult<'tcx, i64> { let this = self.eval_context_mut(); // Isolation check is done via `FileDescriptor` trait. @@ -398,25 +434,31 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let communicate = this.machine.communicate(); // We temporarily dup the FD to be able to retain mutable access to `this`. - let Some(file_descriptor) = this.machine.fds.dup(fd) else { + let Some(fd) = this.machine.fds.dup(fd) else { trace!("read: FD not found"); return this.fd_not_found(); }; - trace!("read: FD mapped to {:?}", file_descriptor); + trace!("read: FD mapped to {fd:?}"); // We want to read at most `count` bytes. We are sure that `count` is not negative // because it was a target's `usize`. Also we are sure that its smaller than // `usize::MAX` because it is bounded by the host's `isize`. let mut bytes = vec![0; usize::try_from(count).unwrap()]; - // `File::read` never returns a value larger than `count`, - // so this cannot fail. - let result = file_descriptor - .borrow_mut() - .read(communicate, &mut bytes, this)? - .map(|c| i64::try_from(c).unwrap()); - drop(file_descriptor); - - match result { + let result = match offset { + None => fd.borrow_mut().read(communicate, &mut bytes, this), + Some(offset) => { + let Ok(offset) = u64::try_from(offset) else { + let einval = this.eval_libc("EINVAL"); + this.set_last_error(einval)?; + return Ok(-1); + }; + fd.borrow_mut().pread(communicate, &mut bytes, offset, this) + } + }; + drop(fd); + + // `File::read` never returns a value larger than `count`, so this cannot fail. + match result?.map(|c| i64::try_from(c).unwrap()) { Ok(read_bytes) => { // If reading to `bytes` did not fail, we write those bytes to the buffer. // Crucially, if fewer than `bytes.len()` bytes were read, only write @@ -434,7 +476,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } } - fn write(&mut self, fd: i32, buf: Pointer, count: u64) -> InterpResult<'tcx, i64> { + fn write( + &mut self, + fd: i32, + buf: Pointer, + count: u64, + offset: Option, + ) -> InterpResult<'tcx, i64> { let this = self.eval_context_mut(); // Isolation check is done via `FileDescriptor` trait. @@ -451,16 +499,24 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?.to_owned(); // We temporarily dup the FD to be able to retain mutable access to `this`. - let Some(file_descriptor) = this.machine.fds.dup(fd) else { + let Some(fd) = this.machine.fds.dup(fd) else { return this.fd_not_found(); }; - let result = file_descriptor - .borrow_mut() - .write(communicate, &bytes, this)? - .map(|c| i64::try_from(c).unwrap()); - drop(file_descriptor); + let result = match offset { + None => fd.borrow_mut().write(communicate, &bytes, this), + Some(offset) => { + let Ok(offset) = u64::try_from(offset) else { + let einval = this.eval_libc("EINVAL"); + this.set_last_error(einval)?; + return Ok(-1); + }; + fd.borrow_mut().pwrite(communicate, &bytes, offset, this) + } + }; + drop(fd); + let result = result?.map(|c| i64::try_from(c).unwrap()); this.try_unwrap_io_result(result) } } diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 3a18d62203333..966e590fcc41e 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -92,7 +92,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let fd = this.read_scalar(fd)?.to_i32()?; let buf = this.read_pointer(buf)?; let count = this.read_target_usize(count)?; - let result = this.read(fd, buf, count)?; + let result = this.read(fd, buf, count, None)?; this.write_scalar(Scalar::from_target_isize(result, this), dest)?; } "write" => { @@ -101,7 +101,47 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let buf = this.read_pointer(buf)?; let count = this.read_target_usize(n)?; trace!("Called write({:?}, {:?}, {:?})", fd, buf, count); - let result = this.write(fd, buf, count)?; + let result = this.write(fd, buf, count, None)?; + // Now, `result` is the value we return back to the program. + this.write_scalar(Scalar::from_target_isize(result, this), dest)?; + } + "pread" => { + let [fd, buf, count, offset] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let fd = this.read_scalar(fd)?.to_i32()?; + let buf = this.read_pointer(buf)?; + let count = this.read_target_usize(count)?; + let offset = this.read_scalar(offset)?.to_int(this.libc_ty_layout("off_t").size)?; + let result = this.read(fd, buf, count, Some(offset))?; + this.write_scalar(Scalar::from_target_isize(result, this), dest)?; + } + "pwrite" => { + let [fd, buf, n, offset] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let fd = this.read_scalar(fd)?.to_i32()?; + let buf = this.read_pointer(buf)?; + let count = this.read_target_usize(n)?; + let offset = this.read_scalar(offset)?.to_int(this.libc_ty_layout("off_t").size)?; + trace!("Called pwrite({:?}, {:?}, {:?}, {:?})", fd, buf, count, offset); + let result = this.write(fd, buf, count, Some(offset))?; + // Now, `result` is the value we return back to the program. + this.write_scalar(Scalar::from_target_isize(result, this), dest)?; + } + "pread64" => { + let [fd, buf, count, offset] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let fd = this.read_scalar(fd)?.to_i32()?; + let buf = this.read_pointer(buf)?; + let count = this.read_target_usize(count)?; + let offset = this.read_scalar(offset)?.to_int(this.libc_ty_layout("off64_t").size)?; + let result = this.read(fd, buf, count, Some(offset))?; + this.write_scalar(Scalar::from_target_isize(result, this), dest)?; + } + "pwrite64" => { + let [fd, buf, n, offset] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let fd = this.read_scalar(fd)?.to_i32()?; + let buf = this.read_pointer(buf)?; + let count = this.read_target_usize(n)?; + let offset = this.read_scalar(offset)?.to_int(this.libc_ty_layout("off64_t").size)?; + trace!("Called pwrite64({:?}, {:?}, {:?}, {:?})", fd, buf, count, offset); + let result = this.write(fd, buf, count, Some(offset))?; // Now, `result` is the value we return back to the program. this.write_scalar(Scalar::from_target_isize(result, this), dest)?; } diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index e34aa5c09dfe1..6923b39733f0b 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -16,8 +16,6 @@ use crate::shims::unix::*; use crate::*; use shims::time::system_time_to_duration; -use self::fd::FileDescriptor; - #[derive(Debug)] struct FileHandle { file: File, @@ -49,6 +47,54 @@ impl FileDescription for FileHandle { Ok(self.file.write(bytes)) } + fn pread<'tcx>( + &mut self, + communicate_allowed: bool, + bytes: &mut [u8], + offset: u64, + _ecx: &mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + assert!(communicate_allowed, "isolation should have prevented even opening a file"); + // Emulates pread using seek + read + seek to restore cursor position. + // Correctness of this emulation relies on sequential nature of Miri execution. + // The closure is used to emulate `try` block, since we "bubble" `io::Error` using `?`. + let mut f = || { + let cursor_pos = self.file.stream_position()?; + self.file.seek(SeekFrom::Start(offset))?; + let res = self.file.read(bytes); + // Attempt to restore cursor position even if the read has failed + self.file + .seek(SeekFrom::Start(cursor_pos)) + .expect("failed to restore file position, this shouldn't be possible"); + res + }; + Ok(f()) + } + + fn pwrite<'tcx>( + &mut self, + communicate_allowed: bool, + bytes: &[u8], + offset: u64, + _ecx: &mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + assert!(communicate_allowed, "isolation should have prevented even opening a file"); + // Emulates pwrite using seek + write + seek to restore cursor position. + // Correctness of this emulation relies on sequential nature of Miri execution. + // The closure is used to emulate `try` block, since we "bubble" `io::Error` using `?`. + let mut f = || { + let cursor_pos = self.file.stream_position()?; + self.file.seek(SeekFrom::Start(offset))?; + let res = self.file.write(bytes); + // Attempt to restore cursor position even if the write has failed + self.file + .seek(SeekFrom::Start(cursor_pos)) + .expect("failed to restore file position, this shouldn't be possible"); + res + }; + Ok(f()) + } + fn seek<'tcx>( &mut self, communicate_allowed: bool, @@ -266,7 +312,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); - let path = this.read_pointer(&args[0])?; + let path_raw = this.read_pointer(&args[0])?; + let path = this.read_path_from_c_str(path_raw)?; let flag = this.read_scalar(&args[1])?.to_i32()?; let mut options = OpenOptions::new(); @@ -366,14 +413,36 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(-1); } } + + let o_nofollow = this.eval_libc_i32("O_NOFOLLOW"); + if flag & o_nofollow == o_nofollow { + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.custom_flags(libc::O_NOFOLLOW); + } + // Strictly speaking, this emulation is not equivalent to the O_NOFOLLOW flag behavior: + // the path could change between us checking it here and the later call to `open`. + // But it's good enough for Miri purposes. + #[cfg(not(unix))] + { + // O_NOFOLLOW only fails when the trailing component is a symlink; + // the entire rest of the path can still contain symlinks. + if path.is_symlink() { + let eloop = this.eval_libc("ELOOP"); + this.set_last_error(eloop)?; + return Ok(-1); + } + } + mirror |= o_nofollow; + } + // If `flag` is not equal to `mirror`, there is an unsupported option enabled in `flag`, // then we throw an error. if flag != mirror { throw_unsup_format!("unsupported flags {:#x}", flag & !mirror); } - let path = this.read_path_from_c_str(path)?; - // Reject if isolation is enabled. if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`open`", reject_with)?; @@ -381,10 +450,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(-1); } - let fd = options.open(path).map(|file| { - let fh = &mut this.machine.fds; - fh.insert_fd(FileDescriptor::new(FileHandle { file, writable })) - }); + let fd = options + .open(path) + .map(|file| this.machine.fds.insert_fd(FileHandle { file, writable })); this.try_unwrap_io_result(fd) } @@ -1476,9 +1544,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { match file { Ok(f) => { - let fh = &mut this.machine.fds; - let fd = - fh.insert_fd(FileDescriptor::new(FileHandle { file: f, writable: true })); + let fd = this.machine.fds.insert_fd(FileHandle { file: f, writable: true }); return Ok(fd); } Err(e) => diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index a5661460e95cf..ad35d67876ccb 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -5,8 +5,6 @@ use rustc_data_structures::fx::FxHashMap; use crate::shims::unix::*; use crate::*; -use self::shims::unix::fd::FileDescriptor; - /// An `Epoll` file descriptor connects file handles and epoll events #[derive(Clone, Debug, Default)] struct Epoll { @@ -66,7 +64,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ); } - let fd = this.machine.fds.insert_fd(FileDescriptor::new(Epoll::default())); + let fd = this.machine.fds.insert_fd(Epoll::default()); Ok(Scalar::from_i32(fd)) } diff --git a/src/tools/miri/src/shims/unix/linux/eventfd.rs b/src/tools/miri/src/shims/unix/linux/eventfd.rs index cae5abca3bd08..0fc28c72bb6fb 100644 --- a/src/tools/miri/src/shims/unix/linux/eventfd.rs +++ b/src/tools/miri/src/shims/unix/linux/eventfd.rs @@ -8,8 +8,6 @@ use rustc_target::abi::Endian; use crate::shims::unix::*; use crate::{concurrency::VClock, *}; -use self::shims::unix::fd::FileDescriptor; - // We'll only do reads and writes in chunks of size u64. const U64_ARRAY_SIZE: usize = mem::size_of::(); @@ -180,11 +178,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_unsup_format!("eventfd: encountered unknown unsupported flags {:#x}", flags); } - let fd = this.machine.fds.insert_fd(FileDescriptor::new(Event { + let fd = this.machine.fds.insert_fd(Event { counter: val.into(), is_nonblock, clock: VClock::default(), - })); + }); Ok(Scalar::from_i32(fd)) } } diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index 95bee38cd7835..20c6a23479421 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -94,6 +94,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { )?; this.write_scalar(res, dest)?; } + "gettid" => { + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let result = this.linux_gettid()?; + this.write_scalar(Scalar::from_i32(result), dest)?; + } // Dynamically invoked syscalls "syscall" => { diff --git a/src/tools/miri/src/shims/unix/socket.rs b/src/tools/miri/src/shims/unix/socket.rs index 6d3d63b4efa02..cc0f932e03829 100644 --- a/src/tools/miri/src/shims/unix/socket.rs +++ b/src/tools/miri/src/shims/unix/socket.rs @@ -7,8 +7,6 @@ use std::rc::{Rc, Weak}; use crate::shims::unix::*; use crate::{concurrency::VClock, *}; -use self::fd::FileDescriptor; - /// The maximum capacity of the socketpair buffer in bytes. /// This number is arbitrary as the value can always /// be configured in the real system. @@ -221,9 +219,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; let fds = &mut this.machine.fds; - let sv0 = fds.insert_fd(FileDescriptor::new(socketpair_0)); + let sv0 = fds.insert_fd(socketpair_0); + let sv1 = fds.insert_fd(socketpair_1); let sv0 = Scalar::from_int(sv0, sv.layout.size); - let sv1 = fds.insert_fd(FileDescriptor::new(socketpair_1)); let sv1 = Scalar::from_int(sv1, sv.layout.size); this.write_scalar(sv0, &sv)?; diff --git a/src/tools/miri/src/shims/windows/env.rs b/src/tools/miri/src/shims/windows/env.rs index ed3eb69798637..77ae06bd5c2d8 100644 --- a/src/tools/miri/src/shims/windows/env.rs +++ b/src/tools/miri/src/shims/windows/env.rs @@ -200,9 +200,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn GetCurrentProcessId(&mut self) -> InterpResult<'tcx, u32> { let this = self.eval_context_mut(); this.assert_target_os("windows", "GetCurrentProcessId"); - this.check_no_isolation("`GetCurrentProcessId`")?; - Ok(std::process::id()) + Ok(this.get_pid()) } #[allow(non_snake_case)] diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 0bbf2a8e13e9a..1bd32fce8bd9e 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/src/shims/x86/mod.rs @@ -1178,7 +1178,7 @@ fn pclmulqdq<'tcx>( // if the i-th bit in right is set if (right & (1 << i)) != 0 { // xor result with `left` shifted to the left by i positions - result ^= (left as u128) << i; + result ^= u128::from(left) << i; } } diff --git a/src/tools/miri/tests/fail/extern-type-field-offset.stderr b/src/tools/miri/tests/fail/extern-type-field-offset.stderr index 3ed5732b4eb06..c07b63e0c0384 100644 --- a/src/tools/miri/tests/fail/extern-type-field-offset.stderr +++ b/src/tools/miri/tests/fail/extern-type-field-offset.stderr @@ -1,3 +1,14 @@ +warning: reborrow of reference to `extern type` + --> $DIR/extern-type-field-offset.rs:LL:CC + | +LL | let x: &Newtype = unsafe { &*(&buf as *const _ as *const Newtype) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reborrow of a reference to `extern type` is not properly supported + | + = help: `extern type` are not compatible with the Stacked Borrows aliasing model implemented by Miri; Miri may miss bugs in this code + = help: try running with `MIRIFLAGS=-Zmiri-tree-borrows` to use the more permissive but also even more experimental Tree Borrows aliasing checks instead + = note: BACKTRACE: + = note: inside `main` at $DIR/extern-type-field-offset.rs:LL:CC + error: unsupported operation: `extern type` field does not have a known offset --> $DIR/extern-type-field-offset.rs:LL:CC | @@ -10,5 +21,5 @@ LL | let _field = &x.a; note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -error: aborting due to 1 previous error +error: aborting due to 1 previous error; 1 warning emitted diff --git a/src/tools/miri/tests/fail/intrinsics/ptr_metadata_uninit_slice_len.stderr b/src/tools/miri/tests/fail/intrinsics/ptr_metadata_uninit_slice_len.stderr index 217bc82010df9..84023cf793713 100644 --- a/src/tools/miri/tests/fail/intrinsics/ptr_metadata_uninit_slice_len.stderr +++ b/src/tools/miri/tests/fail/intrinsics/ptr_metadata_uninit_slice_len.stderr @@ -4,11 +4,11 @@ warning: integer-to-pointer cast LL | (*p.as_mut_ptr().cast::<[*const i32; 2]>())[0] = 4 as *const i32; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast | - = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program. - = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.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 set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics. - = help: Alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning. + = help: this program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program + = help: see https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.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 set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics + = help: alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning = note: BACKTRACE: = note: inside `main` at $DIR/ptr_metadata_uninit_slice_len.rs:LL:CC diff --git a/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.stderr b/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.stderr index 7d000ba55e60b..ce9a5b7f15865 100644 --- a/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.stderr @@ -5,7 +5,7 @@ Warning: this tree is indicative only. Some tags may have been hidden. | RsM | └─┬── | RsM | ├─┬── | RsM | │ └─┬── -| RsM | │ └──── Strongly protected +| Rs | │ └──── Strongly protected | RsM | └──── ────────────────────────────────────────────────── error: Undefined Behavior: write access through (y, callee:y, caller:y) at ALLOC[0x0] is forbidden @@ -16,14 +16,14 @@ LL | *y = 1; | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental = help: the accessed tag (y, callee:y, caller:y) is foreign to the protected tag (callee:x) (i.e., it is not a child) - = help: this foreign write access would cause the protected tag (callee:x) (currently Reserved (interior mutable)) to become Disabled + = help: this foreign write access would cause the protected tag (callee:x) (currently Reserved) to become Disabled = help: protected tags must never be Disabled help: the accessed tag was created here --> $DIR/cell-protected-write.rs:LL:CC | LL | let y = (&mut *n).get(); | ^^^^^^^^^ -help: the protected tag was created here, in the initial state Reserved (interior mutable) +help: the protected tag was created here, in the initial state Reserved --> $DIR/cell-protected-write.rs:LL:CC | LL | unsafe fn write_second(x: &mut UnsafeCell, y: *mut u8) { diff --git a/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.rs b/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.rs new file mode 100644 index 0000000000000..73f227fee2fcb --- /dev/null +++ b/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.rs @@ -0,0 +1,107 @@ +// Illustrating a problematic interaction between Reserved, interior mutability, +// and protectors, that makes spurious writes fail in the previous model of Tree Borrows. +// As for all similar tests, we disable preemption so that the error message is deterministic. +//@compile-flags: -Zmiri-tree-borrows -Zmiri-preemption-rate=0 +// +// One revision without spurious read (default source code) and one with spurious read. +// Both are expected to be UB. Both revisions are expected to have the *same* error +// because we are aligning the behavior of `without` to that of `with` so that the +// spurious write is effectively a noop in the long term. +//@revisions: without with + +use std::cell::Cell; +use std::sync::{Arc, Barrier}; +use std::thread; + +// Here is the problematic interleaving: +// - thread 1: retag and activate `x` (protected) +// - thread 2: retag but do not initialize (lazy) `y` as Reserved with interior mutability +// - thread 1: spurious write through `x` would go here +// - thread 2: function exit (noop due to lazyness) +// - thread 1: function exit (no permanent effect on `y` because it is now Reserved IM unprotected) +// - thread 2: write through `y` +// In the source code nothing happens to `y` + +// `Send`able raw pointer wrapper. +#[derive(Copy, Clone)] +struct SendPtr(*mut u8); +unsafe impl Send for SendPtr {} + +type IdxBarrier = (usize, Arc); + +// Barriers to enforce the interleaving. +// This macro expects `synchronized!(thread, msg)` where `thread` is a `IdxBarrier`, +// and `msg` is the message to be displayed when the thread reaches this point in the execution. +macro_rules! synchronized { + ($thread:expr, $msg:expr) => {{ + let (thread_id, barrier) = &$thread; + eprintln!("Thread {} executing: {}", thread_id, $msg); + barrier.wait(); + }}; +} + +fn main() { + // The conflict occurs on one single location but the example involves + // lazily initialized permissions. We will use `&mut Cell<()>` references + // to `data` to achieve this. + let mut data = 0u8; + let ptr = SendPtr(std::ptr::addr_of_mut!(data)); + let barrier = Arc::new(Barrier::new(2)); + let bx = Arc::clone(&barrier); + let by = Arc::clone(&barrier); + + // Retag and activate `x`, wait until the other thread creates a lazy permission. + // Then do a spurious write. Finally exit the function after the other thread. + let thread_1 = thread::spawn(move || { + let b = (1, bx); + synchronized!(b, "start"); + let ptr = ptr; + synchronized!(b, "retag x (&mut, protect)"); + fn inner(x: &mut u8, b: IdxBarrier) { + *x = 42; // activate immediately + synchronized!(b, "[lazy] retag y (&mut, protect, IM)"); + // A spurious write should be valid here because `x` is + // `Active` and protected. + if cfg!(with) { + synchronized!(b, "spurious write x (executed)"); + *x = 64; + } else { + synchronized!(b, "spurious write x (skipped)"); + } + synchronized!(b, "ret y"); + synchronized!(b, "ret x"); + } + inner(unsafe { &mut *ptr.0 }, b.clone()); + synchronized!(b, "write y"); + synchronized!(b, "end"); + }); + + // Create a lazy Reserved with interior mutability. + // Wait for the other thread's spurious write then observe the side effects + // of that write. + let thread_2 = thread::spawn(move || { + let b = (2, by); + synchronized!(b, "start"); + let ptr = ptr; + synchronized!(b, "retag x (&mut, protect)"); + synchronized!(b, "[lazy] retag y (&mut, protect, IM)"); + fn inner(y: &mut Cell<()>, b: IdxBarrier) -> *mut u8 { + synchronized!(b, "spurious write x"); + synchronized!(b, "ret y"); + // `y` is not retagged for any bytes, so the pointer we return + // has its permission lazily initialized. + y as *mut Cell<()> as *mut u8 + } + // Currently `ptr` points to `data`. + // We do a zero-sized retag so that its permission is lazy. + let y_zst = unsafe { &mut *(ptr.0 as *mut Cell<()>) }; + let y = inner(y_zst, b.clone()); + synchronized!(b, "ret x"); + synchronized!(b, "write y"); + unsafe { *y = 13 } //~ERROR: /write access through .* is forbidden/ + synchronized!(b, "end"); + }); + + thread_1.join().unwrap(); + thread_2.join().unwrap(); +} diff --git a/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.with.stderr b/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.with.stderr new file mode 100644 index 0000000000000..0e4517e90105c --- /dev/null +++ b/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.with.stderr @@ -0,0 +1,40 @@ +Thread 1 executing: start +Thread 2 executing: start +Thread 2 executing: retag x (&mut, protect) +Thread 1 executing: retag x (&mut, protect) +Thread 1 executing: [lazy] retag y (&mut, protect, IM) +Thread 2 executing: [lazy] retag y (&mut, protect, IM) +Thread 2 executing: spurious write x +Thread 1 executing: spurious write x (executed) +Thread 1 executing: ret y +Thread 2 executing: ret y +Thread 2 executing: ret x +Thread 1 executing: ret x +Thread 1 executing: write y +Thread 2 executing: write y +error: Undefined Behavior: write access through at ALLOC[0x0] is forbidden + --> $DIR/reservedim_spurious_write.rs:LL:CC + | +LL | unsafe { *y = 13 } + | ^^^^^^^ write access through at ALLOC[0x0] is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag has state Disabled which forbids this child write access +help: the accessed tag was created here, in the initial state Reserved + --> $DIR/reservedim_spurious_write.rs:LL:CC + | +LL | fn inner(y: &mut Cell<()>, b: IdxBarrier) -> *mut u8 { + | ^ +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x1] + --> $DIR/reservedim_spurious_write.rs:LL:CC + | +LL | *x = 64; + | ^^^^^^^ + = help: this transition corresponds to a loss of read and write permissions + = note: BACKTRACE (of the first span) on thread `unnamed-ID`: + = note: inside closure at $DIR/reservedim_spurious_write.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 + diff --git a/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.without.stderr b/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.without.stderr new file mode 100644 index 0000000000000..cbeef90243bfb --- /dev/null +++ b/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.without.stderr @@ -0,0 +1,40 @@ +Thread 1 executing: start +Thread 2 executing: start +Thread 2 executing: retag x (&mut, protect) +Thread 1 executing: retag x (&mut, protect) +Thread 1 executing: [lazy] retag y (&mut, protect, IM) +Thread 2 executing: [lazy] retag y (&mut, protect, IM) +Thread 2 executing: spurious write x +Thread 1 executing: spurious write x (skipped) +Thread 1 executing: ret y +Thread 2 executing: ret y +Thread 2 executing: ret x +Thread 1 executing: ret x +Thread 1 executing: write y +Thread 2 executing: write y +error: Undefined Behavior: write access through at ALLOC[0x0] is forbidden + --> $DIR/reservedim_spurious_write.rs:LL:CC + | +LL | unsafe { *y = 13 } + | ^^^^^^^ write access through at ALLOC[0x0] is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag has state Disabled which forbids this child write access +help: the accessed tag was created here, in the initial state Reserved + --> $DIR/reservedim_spurious_write.rs:LL:CC + | +LL | fn inner(y: &mut Cell<()>, b: IdxBarrier) -> *mut u8 { + | ^ +help: the accessed tag later transitioned to Disabled due to a protector release (acting as a foreign write access) on every location previously accessed by this tag + --> $DIR/reservedim_spurious_write.rs:LL:CC + | +LL | } + | ^ + = help: this transition corresponds to a loss of read and write permissions + = note: BACKTRACE (of the first span) on thread `unnamed-ID`: + = note: inside closure at $DIR/reservedim_spurious_write.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 + diff --git a/src/tools/miri/tests/pass-dep/libc/gettid.rs b/src/tools/miri/tests/pass-dep/libc/gettid.rs new file mode 100644 index 0000000000000..87405b02ac35d --- /dev/null +++ b/src/tools/miri/tests/pass-dep/libc/gettid.rs @@ -0,0 +1,22 @@ +//@only-target-linux +//@revisions: with_isolation without_isolation +//@[without_isolation] compile-flags: -Zmiri-disable-isolation + +use libc::{getpid, gettid}; +use std::thread; + +fn main() { + thread::spawn(|| { + // Test that in isolation mode a deterministic value will be returned. + // The value 1001 is not important, we only care that whatever the value + // is, won't change from execution to execution. + #[cfg(with_isolation)] + assert_eq!(unsafe { gettid() }, 1001); + + assert_ne!(unsafe { gettid() }, unsafe { getpid() }); + }); + + // Test that the thread ID of the main thread is the same as the process + // ID. + assert_eq!(unsafe { gettid() }, unsafe { getpid() }); +} diff --git a/src/tools/miri/tests/pass-dep/libc/libc-fs-readlink.rs b/src/tools/miri/tests/pass-dep/libc/libc-fs-symlink.rs similarity index 76% rename from src/tools/miri/tests/pass-dep/libc/libc-fs-readlink.rs rename to src/tools/miri/tests/pass-dep/libc/libc-fs-symlink.rs index d72edd7d9e3f2..619c6db3a29de 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-fs-readlink.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-fs-symlink.rs @@ -11,6 +11,11 @@ use std::os::unix::ffi::OsStrExt; mod utils; fn main() { + test_readlink(); + test_nofollow_symlink(); +} + +fn test_readlink() { let bytes = b"Hello, World!\n"; let path = utils::prepare_with_content("miri_test_fs_link_target.txt", bytes); let expected_path = path.as_os_str().as_bytes(); @@ -49,3 +54,18 @@ fn main() { assert_eq!(res, -1); assert_eq!(Error::last_os_error().kind(), ErrorKind::NotFound); } + +fn test_nofollow_symlink() { + let bytes = b"Hello, World!\n"; + let path = utils::prepare_with_content("test_nofollow_symlink_target.txt", bytes); + + let symlink_path = utils::prepare("test_nofollow_symlink.txt"); + std::os::unix::fs::symlink(&path, &symlink_path).unwrap(); + + let symlink_cpath = CString::new(symlink_path.as_os_str().as_bytes()).unwrap(); + + let ret = unsafe { libc::open(symlink_cpath.as_ptr(), libc::O_NOFOLLOW | libc::O_CLOEXEC) }; + assert_eq!(ret, -1); + let err = Error::last_os_error().raw_os_error().unwrap(); + assert_eq!(err, libc::ELOOP); +} diff --git a/src/tools/miri/tests/pass-dep/libc/libc-fs.rs b/src/tools/miri/tests/pass-dep/libc/libc-fs.rs index eddea92353e3d..5b2bbfbb27d08 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-fs.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-fs.rs @@ -37,6 +37,7 @@ fn main() { test_sync_file_range(); test_isatty(); test_read_and_uninit(); + test_nofollow_not_symlink(); } fn test_file_open_unix_allow_two_args() { @@ -423,3 +424,11 @@ fn test_read_and_uninit() { remove_file(&path).unwrap(); } } + +fn test_nofollow_not_symlink() { + let bytes = b"Hello, World!\n"; + let path = utils::prepare_with_content("test_nofollow_not_symlink.txt", bytes); + let cpath = CString::new(path.as_os_str().as_bytes()).unwrap(); + let ret = unsafe { libc::open(cpath.as_ptr(), libc::O_NOFOLLOW | libc::O_CLOEXEC) }; + assert!(ret >= 0); +} diff --git a/src/tools/miri/tests/pass/adjacent-allocs.rs b/src/tools/miri/tests/pass/adjacent-allocs.rs index 8be4bdac7e1bb..711c54fd68ad5 100644 --- a/src/tools/miri/tests/pass/adjacent-allocs.rs +++ b/src/tools/miri/tests/pass/adjacent-allocs.rs @@ -1,5 +1,3 @@ -//@revisions: stack tree -//@[tree]compile-flags: -Zmiri-tree-borrows //@compile-flags: -Zmiri-permissive-provenance fn ensure_allocs_can_be_adjacent() { diff --git a/src/tools/miri/tests/pass/box.rs b/src/tools/miri/tests/pass/box.rs index 174bf8be30b22..693209c04568f 100644 --- a/src/tools/miri/tests/pass/box.rs +++ b/src/tools/miri/tests/pass/box.rs @@ -1,5 +1,4 @@ -//@revisions: stack tree -//@[tree]compile-flags: -Zmiri-tree-borrows -Zmiri-permissive-provenance +//@compile-flags: -Zmiri-permissive-provenance #![feature(ptr_internals)] fn main() { diff --git a/src/tools/miri/tests/pass/box.stack.stderr b/src/tools/miri/tests/pass/box.stack.stderr deleted file mode 100644 index 341f84c8992df..0000000000000 --- a/src/tools/miri/tests/pass/box.stack.stderr +++ /dev/null @@ -1,33 +0,0 @@ -warning: integer-to-pointer cast - --> $DIR/box.rs:LL:CC - | -LL | let r2 = ((r as usize) + 0) as *mut i32; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast - | - = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program. - = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.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 set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics. - = help: Alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning. - = note: BACKTRACE: - = note: inside `into_raw` at $DIR/box.rs:LL:CC -note: inside `main` - --> $DIR/box.rs:LL:CC - | -LL | into_raw(); - | ^^^^^^^^^^ - -warning: integer-to-pointer cast - --> $DIR/box.rs:LL:CC - | -LL | let r = ((u.as_ptr() as usize) + 0) as *mut i32; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast - | - = note: BACKTRACE: - = note: inside `into_unique` at $DIR/box.rs:LL:CC -note: inside `main` - --> $DIR/box.rs:LL:CC - | -LL | into_unique(); - | ^^^^^^^^^^^^^ - diff --git a/src/tools/miri/tests/pass/box.stack.stdout b/src/tools/miri/tests/pass/box.stdout similarity index 100% rename from src/tools/miri/tests/pass/box.stack.stdout rename to src/tools/miri/tests/pass/box.stdout diff --git a/src/tools/miri/tests/pass/box.tree.stdout b/src/tools/miri/tests/pass/box.tree.stdout deleted file mode 100644 index 230ef368da644..0000000000000 --- a/src/tools/miri/tests/pass/box.tree.stdout +++ /dev/null @@ -1,3 +0,0 @@ -pair_foo = PairFoo { fst: Foo(42), snd: Foo(1337) } -foo #0 = Foo(42) -foo #1 = Foo(1337) diff --git a/src/tools/miri/tests/pass/extern_types.rs b/src/tools/miri/tests/pass/extern_types.rs index 7ac93577f0cd5..eade5c6ac0874 100644 --- a/src/tools/miri/tests/pass/extern_types.rs +++ b/src/tools/miri/tests/pass/extern_types.rs @@ -1,12 +1,14 @@ //@revisions: stack tree -//@[tree]compile-flags: -Zmiri-tree-borrows -Zmiri-permissive-provenance -#![feature(extern_types)] +//@[tree]compile-flags: -Zmiri-tree-borrows +#![feature(extern_types, strict_provenance)] + +use std::ptr; extern "C" { type Foo; } fn main() { - let x: &Foo = unsafe { &*(16 as *const Foo) }; + let x: &Foo = unsafe { &*(ptr::without_provenance::<()>(16) as *const Foo) }; let _y: &Foo = &*x; } diff --git a/src/tools/miri/tests/pass/extern_types.stack.stderr b/src/tools/miri/tests/pass/extern_types.stack.stderr index 03a9167abbc7c..2c9fc0192af83 100644 --- a/src/tools/miri/tests/pass/extern_types.stack.stderr +++ b/src/tools/miri/tests/pass/extern_types.stack.stderr @@ -1,14 +1,11 @@ -warning: integer-to-pointer cast +warning: reborrow of reference to `extern type` --> $DIR/extern_types.rs:LL:CC | -LL | let x: &Foo = unsafe { &*(16 as *const Foo) }; - | ^^^^^^^^^^^^^^^^^^ integer-to-pointer cast +LL | let x: &Foo = unsafe { &*(ptr::without_provenance::<()>(16) as *const Foo) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reborrow of a reference to `extern type` is not properly supported | - = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program. - = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.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 set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics. - = help: Alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning. + = help: `extern type` are not compatible with the Stacked Borrows aliasing model implemented by Miri; Miri may miss bugs in this code + = help: try running with `MIRIFLAGS=-Zmiri-tree-borrows` to use the more permissive but also even more experimental Tree Borrows aliasing checks instead = note: BACKTRACE: = note: inside `main` at $DIR/extern_types.rs:LL:CC diff --git a/src/tools/miri/tests/pass/getpid.rs b/src/tools/miri/tests/pass/getpid.rs index 733545462ebc0..f350fafff4a26 100644 --- a/src/tools/miri/tests/pass/getpid.rs +++ b/src/tools/miri/tests/pass/getpid.rs @@ -1,9 +1,20 @@ -//@compile-flags: -Zmiri-disable-isolation +//@revisions: with_isolation without_isolation +//@[without_isolation] compile-flags: -Zmiri-disable-isolation fn getpid() -> u32 { std::process::id() } fn main() { - getpid(); + let pid = getpid(); + + std::thread::spawn(move || { + assert_eq!(getpid(), pid); + }); + + // Test that in isolation mode a deterministic value will be returned. + // The value 1000 is not important, we only care that whatever the value + // is, won't change from execution to execution. + #[cfg(with_isolation)] + assert_eq!(pid, 1000); } diff --git a/src/tools/miri/tests/pass/intptrcast.rs b/src/tools/miri/tests/pass/intptrcast.rs index fb1a1dfae5d14..a304f2751bdb6 100644 --- a/src/tools/miri/tests/pass/intptrcast.rs +++ b/src/tools/miri/tests/pass/intptrcast.rs @@ -1,5 +1,3 @@ -//@revisions: stack tree -//@[tree]compile-flags: -Zmiri-tree-borrows //@compile-flags: -Zmiri-permissive-provenance use std::mem; diff --git a/src/tools/miri/tests/pass/pointers.rs b/src/tools/miri/tests/pass/pointers.rs index c7b720dafa20b..280a815abc430 100644 --- a/src/tools/miri/tests/pass/pointers.rs +++ b/src/tools/miri/tests/pass/pointers.rs @@ -1,5 +1,3 @@ -//@revisions: stack tree -//@[tree]compile-flags: -Zmiri-tree-borrows //@compile-flags: -Zmiri-permissive-provenance #![feature(ptr_metadata, const_raw_ptr_comparison)] #![allow(ambiguous_wide_pointer_comparisons)] diff --git a/src/tools/miri/tests/pass/ptr_int_casts.rs b/src/tools/miri/tests/pass/ptr_int_casts.rs index a2fcd098107a6..684e8f6ec7bdd 100644 --- a/src/tools/miri/tests/pass/ptr_int_casts.rs +++ b/src/tools/miri/tests/pass/ptr_int_casts.rs @@ -1,6 +1,7 @@ //@revisions: stack tree +// Tree Borrows doesn't support int2ptr casts, but let's make sure we don't immediately crash either. //@[tree]compile-flags: -Zmiri-tree-borrows -//@compile-flags: -Zmiri-permissive-provenance +//@[stack]compile-flags: -Zmiri-permissive-provenance use std::mem; use std::ptr; diff --git a/src/tools/miri/tests/pass/ptr_int_casts.tree.stderr b/src/tools/miri/tests/pass/ptr_int_casts.tree.stderr new file mode 100644 index 0000000000000..a34474ee0d627 --- /dev/null +++ b/src/tools/miri/tests/pass/ptr_int_casts.tree.stderr @@ -0,0 +1,89 @@ +warning: integer-to-pointer cast + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | assert_eq!(1 as *const i32 as usize, 1); + | ^^^^^^^^^^^^^^^ integer-to-pointer cast + | + = help: this program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program + = help: see https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.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 set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics + = help: Tree Borrows does not support integer-to-pointer casts, so the program is likely to go wrong when this pointer gets used + = note: BACKTRACE: + = note: inside `ptr_int_casts` at $DIR/ptr_int_casts.rs:LL:CC +note: inside `main` + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | ptr_int_casts(); + | ^^^^^^^^^^^^^^^ + +warning: integer-to-pointer cast + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | assert_eq!((1 as *const i32).wrapping_offset(4) as usize, 1 + 4 * 4); + | ^^^^^^^^^^^^^^^^^ integer-to-pointer cast + | + = note: BACKTRACE: + = note: inside `ptr_int_casts` at $DIR/ptr_int_casts.rs:LL:CC +note: inside `main` + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | ptr_int_casts(); + | ^^^^^^^^^^^^^^^ + +warning: integer-to-pointer cast + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | *val = (1 as *const u8).wrapping_offset(-4); + | ^^^^^^^^^^^^^^^^ integer-to-pointer cast + | + = note: BACKTRACE: + = note: inside `ptr_int_casts` at $DIR/ptr_int_casts.rs:LL:CC +note: inside `main` + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | ptr_int_casts(); + | ^^^^^^^^^^^^^^^ + +warning: integer-to-pointer cast + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | let y = y as *const _; + | ^^^^^^^^^^^^^ integer-to-pointer cast + | + = note: BACKTRACE: + = note: inside `ptr_int_casts` at $DIR/ptr_int_casts.rs:LL:CC +note: inside `main` + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | ptr_int_casts(); + | ^^^^^^^^^^^^^^^ + +warning: integer-to-pointer cast + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | let x: fn() -> i32 = unsafe { mem::transmute(y as *mut u8) }; + | ^^^^^^^^^^^^ integer-to-pointer cast + | + = note: BACKTRACE: + = note: inside `ptr_int_casts` at $DIR/ptr_int_casts.rs:LL:CC +note: inside `main` + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | ptr_int_casts(); + | ^^^^^^^^^^^^^^^ + +warning: integer-to-pointer cast + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | assert_eq!((-1i32) as usize as *const i32 as usize, (-1i32) as usize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast + | + = note: BACKTRACE: + = note: inside `ptr_int_casts` at $DIR/ptr_int_casts.rs:LL:CC +note: inside `main` + --> $DIR/ptr_int_casts.rs:LL:CC + | +LL | ptr_int_casts(); + | ^^^^^^^^^^^^^^^ + diff --git a/src/tools/miri/tests/pass/ptr_int_from_exposed.rs b/src/tools/miri/tests/pass/ptr_int_from_exposed.rs index 589ef781a91a9..ade5f537785b7 100644 --- a/src/tools/miri/tests/pass/ptr_int_from_exposed.rs +++ b/src/tools/miri/tests/pass/ptr_int_from_exposed.rs @@ -1,6 +1,7 @@ //@revisions: stack tree +// Tree Borrows doesn't support int2ptr casts, but let's make sure we don't immediately crash either. //@[tree]compile-flags: -Zmiri-tree-borrows -//@compile-flags: -Zmiri-permissive-provenance +//@[stack]compile-flags: -Zmiri-permissive-provenance #![feature(strict_provenance, exposed_provenance)] use std::ptr; diff --git a/src/tools/miri/tests/pass/ptr_int_from_exposed.tree.stderr b/src/tools/miri/tests/pass/ptr_int_from_exposed.tree.stderr new file mode 100644 index 0000000000000..614b0d26a6304 --- /dev/null +++ b/src/tools/miri/tests/pass/ptr_int_from_exposed.tree.stderr @@ -0,0 +1,19 @@ +warning: integer-to-pointer cast + --> $DIR/ptr_int_from_exposed.rs:LL:CC + | +LL | let ptr = ptr::with_exposed_provenance::(x_usize).wrapping_offset(-128); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast + | + = help: this program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program + = help: see https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.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 set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics + = help: Tree Borrows does not support integer-to-pointer casts, so the program is likely to go wrong when this pointer gets used + = note: BACKTRACE: + = note: inside `ptr_roundtrip_out_of_bounds` at $DIR/ptr_int_from_exposed.rs:LL:CC +note: inside `main` + --> $DIR/ptr_int_from_exposed.rs:LL:CC + | +LL | ptr_roundtrip_out_of_bounds(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + diff --git a/src/tools/miri/tests/pass/shims/fs.rs b/src/tools/miri/tests/pass/shims/fs.rs index 16d3e8cab30ab..70e375b098128 100644 --- a/src/tools/miri/tests/pass/shims/fs.rs +++ b/src/tools/miri/tests/pass/shims/fs.rs @@ -30,6 +30,8 @@ fn main() { test_directory(); test_canonicalize(); test_from_raw_os_error(); + #[cfg(unix)] + test_pread_pwrite(); } fn test_path_conversion() { @@ -303,3 +305,42 @@ fn test_from_raw_os_error() { // Make sure we can also format this. let _ = format!("{error:?}"); } + +#[cfg(unix)] +fn test_pread_pwrite() { + use std::os::unix::fs::FileExt; + + let bytes = b"hello world"; + let path = utils::prepare_with_content("miri_test_fs_pread_pwrite.txt", bytes); + let mut f = OpenOptions::new().read(true).write(true).open(path).unwrap(); + + let mut buf1 = [0u8; 3]; + f.seek(SeekFrom::Start(5)).unwrap(); + + // Check that we get expected result after seek + f.read_exact(&mut buf1).unwrap(); + assert_eq!(&buf1, b" wo"); + f.seek(SeekFrom::Start(5)).unwrap(); + + // Check pread + f.read_exact_at(&mut buf1, 2).unwrap(); + assert_eq!(&buf1, b"llo"); + f.read_exact_at(&mut buf1, 6).unwrap(); + assert_eq!(&buf1, b"wor"); + + // Ensure that cursor position is not changed + f.read_exact(&mut buf1).unwrap(); + assert_eq!(&buf1, b" wo"); + f.seek(SeekFrom::Start(5)).unwrap(); + + // Check pwrite + f.write_all_at(b" mo", 6).unwrap(); + + let mut buf2 = [0u8; 11]; + f.read_exact_at(&mut buf2, 0).unwrap(); + assert_eq!(&buf2, b"hello mold"); + + // Ensure that cursor position is not changed + f.read_exact(&mut buf1).unwrap(); + assert_eq!(&buf1, b" m"); +} diff --git a/src/tools/miri/tests/pass/stacked-borrows/issue-miri-2389.stderr b/src/tools/miri/tests/pass/stacked-borrows/issue-miri-2389.stderr index b0e1adf27d183..216bb6c76bc9a 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/issue-miri-2389.stderr +++ b/src/tools/miri/tests/pass/stacked-borrows/issue-miri-2389.stderr @@ -4,11 +4,11 @@ warning: integer-to-pointer cast LL | let wildcard = &root0 as *const Cell as usize as *const Cell; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast | - = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program. - = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.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 set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics. - = help: Alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning. + = help: this program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program + = help: see https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.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 set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics + = help: alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning = note: BACKTRACE: = note: inside `main` at $DIR/issue-miri-2389.rs:LL:CC diff --git a/src/tools/miri/tests/pass/tree_borrows/reserved.stderr b/src/tools/miri/tests/pass/tree_borrows/reserved.stderr index 0d0d52c717fed..d149a4065f92b 100644 --- a/src/tools/miri/tests/pass/tree_borrows/reserved.stderr +++ b/src/tools/miri/tests/pass/tree_borrows/reserved.stderr @@ -6,7 +6,7 @@ Warning: this tree is indicative only. Some tags may have been hidden. | RsM | └─┬── | RsM | ├─┬── | RsM | │ └─┬── -| RsCM| │ └──── +| RsC | │ └──── | RsM | └──── ────────────────────────────────────────────────── [interior mut] Foreign Read: Re* -> Re*