diff --git a/src/tools/miri/.gitignore b/src/tools/miri/.gitignore index 4a238dc031342..23f2513addb74 100644 --- a/src/tools/miri/.gitignore +++ b/src/tools/miri/.gitignore @@ -5,6 +5,7 @@ tex/*/out *.rs.bk .vscode .helix +.zed *.mm_profdata perf.data perf.data.old diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index a6ccd9bab3930..90ba52120ee87 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -b6fdaf2a15736cbccf248b532f48e33179614d40 +d10ac47c20152feb5e99b1c35a2e6830f77c66dc diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 48f8e146a190c..b28d70f058827 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -3,7 +3,7 @@ clippy::manual_range_contains, clippy::useless_format, clippy::field_reassign_with_default, - clippy::needless_lifetimes, + clippy::needless_lifetimes )] // The rustc crates we need diff --git a/src/tools/miri/src/shims/sig.rs b/src/tools/miri/src/shims/sig.rs index 4e4b1bc2b9fec..43b913edbebf5 100644 --- a/src/tools/miri/src/shims/sig.rs +++ b/src/tools/miri/src/shims/sig.rs @@ -28,45 +28,118 @@ pub struct ShimSig<'tcx, const ARGS: usize> { /// - `winapi::$ty` for a type from `std::sys::pal::windows::c` #[macro_export] macro_rules! shim_sig { - (extern $abi:literal fn($($arg:ty),* $(,)?) -> $ret:ty) => { + (extern $abi:literal fn($($args:tt)*) -> $($ret:tt)*) => { |this| $crate::shims::sig::ShimSig { abi: std::str::FromStr::from_str($abi).expect("incorrect abi specified"), - args: [$(shim_sig_arg!(this, $arg)),*], - ret: shim_sig_arg!(this, $ret), + args: shim_sig_args_sep!(this, [$($args)*]), + ret: shim_sig_arg!(this, $($ret)*), } }; } /// Helper for `shim_sig!`. +/// +/// Groups tokens into comma-separated chunks and calls the provided macro on them. +/// +/// # Examples +/// +/// ```ignore +/// shim_sig_args_sep!(this, [*const _, i32, libc::off64_t]); +/// // expands to: +/// [shim_sig_arg!(*const _), shim_sig_arg!(i32), shim_sig_arg!(libc::off64_t)]; +/// ``` +#[macro_export] +macro_rules! shim_sig_args_sep { + ($this:ident, [$($tt:tt)*]) => { + shim_sig_args_sep!(@ $this [] [] $($tt)*) + }; + + // All below matchers form a fairly simple iterator over the input. + // - Non-comma token - append to collector + // - Comma token - call the provided macro on the collector and reset the collector + // - End of input - empty collector one last time. emit output as an array + + // Handles `,` token - take collected type and call shim_sig_arg on it. + // Append the result to the final output. + (@ $this:ident [$($final:tt)*] [$($collected:tt)*] , $($tt:tt)*) => { + shim_sig_args_sep!(@ $this [$($final)* shim_sig_arg!($this, $($collected)*), ] [] $($tt)*) + }; + // Handle non-comma token - append to collected type. + (@ $this:ident [$($final:tt)*] [$($collected:tt)*] $first:tt $($tt:tt)*) => { + shim_sig_args_sep!(@ $this [$($final)*] [$($collected)* $first] $($tt)*) + }; + // No more tokens - emit final output, including final non-comma type. + (@ $this:ident [$($final:tt)*] [$($collected:tt)+] ) => { + [$($final)* shim_sig_arg!($this, $($collected)*)] + }; + // No more tokens - emit final output. + (@ $this:ident [$($final:tt)*] [] ) => { + [$($final)*] + }; +} + +/// Helper for `shim_sig!`. +/// +/// Converts a type #[macro_export] macro_rules! shim_sig_arg { - // Unfortuantely we cannot take apart a `ty`-typed token at compile time, - // so we have to stringify it and match at runtime. - ($this:ident, $x:ty) => {{ - match stringify!($x) { - "i8" => $this.tcx.types.i8, - "i16" => $this.tcx.types.i16, - "i32" => $this.tcx.types.i32, - "i64" => $this.tcx.types.i64, - "i128" => $this.tcx.types.i128, - "isize" => $this.tcx.types.isize, - "u8" => $this.tcx.types.u8, - "u16" => $this.tcx.types.u16, - "u32" => $this.tcx.types.u32, - "u64" => $this.tcx.types.u64, - "u128" => $this.tcx.types.u128, - "usize" => $this.tcx.types.usize, - "()" => $this.tcx.types.unit, - "bool" => $this.tcx.types.bool, - "*const _" => $this.machine.layouts.const_raw_ptr.ty, - "*mut _" => $this.machine.layouts.mut_raw_ptr.ty, - ty if let Some(win_ty) = ty.strip_prefix("winapi::") => - $this.windows_ty_layout(win_ty).ty, - ty if ty.contains("::") => - helpers::path_ty_layout($this, &ty.split("::").collect::>()).ty, - ty => panic!("unsupported signature type {ty:?}"), - } - }}; + ($this:ident, i8) => { + $this.tcx.types.i8 + }; + ($this:ident, i16) => { + $this.tcx.types.i16 + }; + ($this:ident, i32) => { + $this.tcx.types.i32 + }; + ($this:ident, i64) => { + $this.tcx.types.i64 + }; + ($this:ident, i128) => { + $this.tcx.types.i128 + }; + ($this:ident, isize) => { + $this.tcx.types.isize + }; + ($this:ident, u8) => { + $this.tcx.types.u8 + }; + ($this:ident, u16) => { + $this.tcx.types.u16 + }; + ($this:ident, u32) => { + $this.tcx.types.u32 + }; + ($this:ident, u64) => { + $this.tcx.types.u64 + }; + ($this:ident, u128) => { + $this.tcx.types.u128 + }; + ($this:ident, usize) => { + $this.tcx.types.usize + }; + ($this:ident, ()) => { + $this.tcx.types.unit + }; + ($this:ident, bool) => { + $this.tcx.types.bool + }; + ($this:ident, *const _) => { + $this.machine.layouts.const_raw_ptr.ty + }; + ($this:ident, *mut _) => { + $this.machine.layouts.mut_raw_ptr.ty + }; + ($this:ident, winapi::$ty:ident) => { + $this.windows_ty_layout(stringify!($ty)).ty + }; + ($this:ident, $krate:ident :: $($path:ident)::+) => { + helpers::path_ty_layout($this, &[stringify!($krate), $(stringify!($path)),*]).ty + }; + ($this:ident, $($other:tt)*) => { + compile_error!(concat!("unsupported signature type: ", stringify!($($other)*))) + } } /// Helper function to compare two ABIs. diff --git a/src/tools/miri/src/shims/time.rs b/src/tools/miri/src/shims/time.rs index 5352b08d8d25e..c49aa9f20f6c0 100644 --- a/src/tools/miri/src/shims/time.rs +++ b/src/tools/miri/src/shims/time.rs @@ -347,7 +347,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { assert_eq!(unblock, UnblockKind::TimedOut); interp_ok(()) } - ) + ), ); interp_ok(Scalar::from_i32(0)) // KERN_SUCCESS diff --git a/src/tools/miri/src/shims/unix/android/foreign_items.rs b/src/tools/miri/src/shims/unix/android/foreign_items.rs index f00bfb0a20787..5259ca2294884 100644 --- a/src/tools/miri/src/shims/unix/android/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/android/foreign_items.rs @@ -39,12 +39,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let result = this.lstat(path, buf)?; this.write_scalar(result, dest)?; } - "readdir" => { - // FIXME: This does not have a direct test (#3179). - let [dirp] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - let result = this.readdir64("dirent", dirp)?; - this.write_scalar(result, dest)?; - } "pread64" => { // FIXME: This does not have a direct test (#3179). let [fd, buf, count, offset] = this.check_shim_sig( @@ -85,7 +79,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let fd = this.read_scalar(fd)?.to_i32()?; let offset = this.read_scalar(offset)?.to_int(offset.layout.size)?; let whence = this.read_scalar(whence)?.to_i32()?; - this.lseek64(fd, offset, whence, dest)?; + this.lseek(fd, offset, whence, dest)?; } "ftruncate64" => { let [fd, length] = this.check_shim_sig( diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 04ec260eccd00..87a307c989484 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -363,7 +363,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(result, dest)?; } "opendir" => { - // FIXME: This does not have a direct test (#3179). let [name] = this.check_shim_sig( shim_sig!(extern "C" fn(*const _) -> *mut _), link_name, @@ -374,7 +373,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(result, dest)?; } "closedir" => { - // FIXME: This does not have a direct test (#3179). let [dirp] = this.check_shim_sig( shim_sig!(extern "C" fn(*mut _) -> i32), link_name, @@ -384,6 +382,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let result = this.closedir(dirp)?; this.write_scalar(result, dest)?; } + "readdir" => { + let [dirp] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; + this.readdir(dirp, dest)?; + } "lseek" => { // FIXME: This does not have a direct test (#3179). let [fd, offset, whence] = this.check_shim_sig( @@ -395,7 +397,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let fd = this.read_scalar(fd)?.to_i32()?; let offset = this.read_scalar(offset)?.to_int(offset.layout.size)?; let whence = this.read_scalar(whence)?.to_i32()?; - this.lseek64(fd, offset, whence, dest)?; + this.lseek(fd, offset, whence, dest)?; } "ftruncate" => { let [fd, length] = this.check_shim_sig( diff --git a/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs b/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs index b94ee27c46a00..f64ee3b162207 100644 --- a/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs @@ -155,11 +155,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let result = this.fstat(fd, buf)?; this.write_scalar(result, dest)?; } - "readdir" | "readdir@FBSD_1.0" => { - // FIXME: This does not have a direct test (#3179). + "readdir@FBSD_1.0" => { let [dirp] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - let result = this.readdir64("dirent", dirp)?; - this.write_scalar(result, dest)?; + this.readdir(dirp, dest)?; } // Miscellaneous "__error" => { diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index f43fd3fe2d188..a01755ef95ae7 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -1,15 +1,17 @@ //! File and file system access use std::borrow::Cow; +use std::ffi::OsString; use std::fs::{ - DirBuilder, File, FileType, OpenOptions, ReadDir, TryLockError, read_dir, remove_dir, - remove_file, rename, + self, DirBuilder, File, FileType, OpenOptions, TryLockError, read_dir, remove_dir, remove_file, + rename, }; use std::io::{self, ErrorKind, Read, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; use std::time::SystemTime; use rustc_abi::Size; +use rustc_data_structures::either::Either; use rustc_data_structures::fx::FxHashMap; use rustc_target::spec::Os; @@ -20,6 +22,40 @@ use crate::shims::sig::check_min_vararg_count; use crate::shims::unix::fd::{FlockOp, UnixFileDescription}; use crate::*; +/// An open directory, tracked by DirHandler. +#[derive(Debug)] +struct OpenDir { + /// The "special" entries that must still be yielded by the iterator. + /// Used for `.` and `..`. + special_entries: Vec<&'static str>, + /// The directory reader on the host. + read_dir: fs::ReadDir, + /// The most recent entry returned by readdir(). + /// Will be freed by the next call. + entry: Option, +} + +impl OpenDir { + fn new(read_dir: fs::ReadDir) -> Self { + Self { special_entries: vec!["..", "."], read_dir, entry: None } + } + + fn next_host_entry(&mut self) -> Option>> { + if let Some(special) = self.special_entries.pop() { + return Some(Ok(Either::Right(special))); + } + let entry = self.read_dir.next()?; + Some(entry.map(Either::Left)) + } +} + +#[derive(Debug)] +struct DirEntry { + name: OsString, + ino: u64, + d_type: i32, +} + impl UnixFileDescription for FileHandle { fn pread<'tcx>( &self, @@ -116,6 +152,71 @@ impl UnixFileDescription for FileHandle { } } +/// The table of open directories. +/// Curiously, Unix/POSIX does not unify this into the "file descriptor" concept... everything +/// is a file, except a directory is not? +#[derive(Debug)] +pub struct DirTable { + /// Directory iterators used to emulate libc "directory streams", as used in opendir, readdir, + /// and closedir. + /// + /// When opendir is called, a directory iterator is created on the host for the target + /// directory, and an entry is stored in this hash map, indexed by an ID which represents + /// the directory stream. When readdir is called, the directory stream ID is used to look up + /// the corresponding ReadDir iterator from this map, and information from the next + /// directory entry is returned. When closedir is called, the ReadDir iterator is removed from + /// the map. + streams: FxHashMap, + /// ID number to be used by the next call to opendir + next_id: u64, +} + +impl DirTable { + #[expect(clippy::arithmetic_side_effects)] + fn insert_new(&mut self, read_dir: fs::ReadDir) -> u64 { + let id = self.next_id; + self.next_id += 1; + self.streams.try_insert(id, OpenDir::new(read_dir)).unwrap(); + id + } +} + +impl Default for DirTable { + fn default() -> DirTable { + DirTable { + streams: FxHashMap::default(), + // Skip 0 as an ID, because it looks like a null pointer to libc + next_id: 1, + } + } +} + +impl VisitProvenance for DirTable { + fn visit_provenance(&self, visit: &mut VisitWith<'_>) { + let DirTable { streams, next_id: _ } = self; + + for dir in streams.values() { + dir.entry.visit_provenance(visit); + } + } +} + +fn maybe_sync_file( + file: &File, + writable: bool, + operation: fn(&File) -> std::io::Result<()>, +) -> std::io::Result { + if !writable && cfg!(windows) { + // sync_all() and sync_data() will return an error on Windows hosts if the file is not opened + // for writing. (FlushFileBuffers requires that the file handle have the + // GENERIC_WRITE right) + Ok(0i32) + } else { + let result = operation(file); + result.map(|_| 0i32) + } +} + impl<'tcx> EvalContextExtPrivate<'tcx> for crate::MiriInterpCx<'tcx> {} trait EvalContextExtPrivate<'tcx>: crate::MiriInterpCxExt<'tcx> { fn write_stat_buf( @@ -178,14 +279,11 @@ trait EvalContextExtPrivate<'tcx>: crate::MiriInterpCxExt<'tcx> { interp_ok(0) } - fn file_type_to_d_type( - &mut self, - file_type: std::io::Result, - ) -> InterpResult<'tcx, i32> { + fn file_type_to_d_type(&self, file_type: std::io::Result) -> InterpResult<'tcx, i32> { #[cfg(unix)] use std::os::unix::fs::FileTypeExt; - let this = self.eval_context_mut(); + let this = self.eval_context_ref(); match file_type { Ok(file_type) => { match () { @@ -216,86 +314,32 @@ trait EvalContextExtPrivate<'tcx>: crate::MiriInterpCxExt<'tcx> { } } } -} -/// An open directory, tracked by DirHandler. -#[derive(Debug)] -struct OpenDir { - /// The directory reader on the host. - read_dir: ReadDir, - /// The most recent entry returned by readdir(). - /// Will be freed by the next call. - entry: Option, -} - -impl OpenDir { - fn new(read_dir: ReadDir) -> Self { - Self { read_dir, entry: None } - } -} - -/// The table of open directories. -/// Curiously, Unix/POSIX does not unify this into the "file descriptor" concept... everything -/// is a file, except a directory is not? -#[derive(Debug)] -pub struct DirTable { - /// Directory iterators used to emulate libc "directory streams", as used in opendir, readdir, - /// and closedir. - /// - /// When opendir is called, a directory iterator is created on the host for the target - /// directory, and an entry is stored in this hash map, indexed by an ID which represents - /// the directory stream. When readdir is called, the directory stream ID is used to look up - /// the corresponding ReadDir iterator from this map, and information from the next - /// directory entry is returned. When closedir is called, the ReadDir iterator is removed from - /// the map. - streams: FxHashMap, - /// ID number to be used by the next call to opendir - next_id: u64, -} - -impl DirTable { - #[expect(clippy::arithmetic_side_effects)] - fn insert_new(&mut self, read_dir: ReadDir) -> u64 { - let id = self.next_id; - self.next_id += 1; - self.streams.try_insert(id, OpenDir::new(read_dir)).unwrap(); - id - } -} - -impl Default for DirTable { - fn default() -> DirTable { - DirTable { - streams: FxHashMap::default(), - // Skip 0 as an ID, because it looks like a null pointer to libc - next_id: 1, - } - } -} - -impl VisitProvenance for DirTable { - fn visit_provenance(&self, visit: &mut VisitWith<'_>) { - let DirTable { streams, next_id: _ } = self; - - for dir in streams.values() { - dir.entry.visit_provenance(visit); - } - } -} - -fn maybe_sync_file( - file: &File, - writable: bool, - operation: fn(&File) -> std::io::Result<()>, -) -> std::io::Result { - if !writable && cfg!(windows) { - // sync_all() and sync_data() will return an error on Windows hosts if the file is not opened - // for writing. (FlushFileBuffers requires that the file handle have the - // GENERIC_WRITE right) - Ok(0i32) - } else { - let result = operation(file); - result.map(|_| 0i32) + fn dir_entry_fields( + &self, + entry: Either, + ) -> InterpResult<'tcx, DirEntry> { + let this = self.eval_context_ref(); + interp_ok(match entry { + Either::Left(dir_entry) => { + DirEntry { + name: dir_entry.file_name(), + d_type: this.file_type_to_d_type(dir_entry.file_type())?, + // If the host is a Unix system, fill in the inode number with its real value. + // If not, use 0 as a fallback value. + #[cfg(unix)] + ino: std::os::unix::fs::DirEntryExt::ino(&dir_entry), + #[cfg(not(unix))] + ino: 0u64, + } + } + Either::Right(special) => + DirEntry { + name: special.into(), + d_type: this.eval_libc("DT_DIR").to_u8()?.into(), + ino: 0, + }, + }) } } @@ -443,7 +487,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { interp_ok(Scalar::from_i32(this.try_unwrap_io_result(fd)?)) } - fn lseek64( + fn lseek( &mut self, fd_num: i32, offset: i128, @@ -899,14 +943,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } } - fn readdir64(&mut self, dirent_type: &str, dirp_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { + fn readdir(&mut self, dirp_op: &OpTy<'tcx>, dest: &MPlaceTy<'tcx>) -> InterpResult<'tcx> { let this = self.eval_context_mut(); if !matches!( &this.tcx.sess.target.os, Os::Linux | Os::Android | Os::Solaris | Os::Illumos | Os::FreeBsd ) { - panic!("`readdir64` should not be called on {}", this.tcx.sess.target.os); + panic!("`readdir` should not be called on {}", this.tcx.sess.target.os); } let dirp = this.read_target_usize(dirp_op)?; @@ -915,21 +959,17 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`readdir`", reject_with)?; this.set_last_error(LibcError("EBADF"))?; - return interp_ok(Scalar::null_ptr(this)); + this.write_null(dest)?; + return interp_ok(()); } let open_dir = this.machine.dirs.streams.get_mut(&dirp).ok_or_else(|| { - err_unsup_format!("the DIR pointer passed to readdir64 did not come from opendir") + err_ub_format!("the DIR pointer passed to `readdir` did not come from opendir") })?; - let entry = match open_dir.read_dir.next() { + let entry = match open_dir.next_host_entry() { Some(Ok(dir_entry)) => { - // If the host is a Unix system, fill in the inode number with its real value. - // If not, use 0 as a fallback value. - #[cfg(unix)] - let ino = std::os::unix::fs::DirEntryExt::ino(&dir_entry); - #[cfg(not(unix))] - let ino = 0u64; + let dir_entry = this.dir_entry_fields(dir_entry)?; // Write the directory entry into a newly allocated buffer. // The name is written with write_bytes, while the rest of the @@ -954,23 +994,29 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // } // // On FreeBSD: - // pub struct dirent{ + // pub struct dirent { // pub d_fileno: uint32_t, // pub d_reclen: uint16_t, // pub d_type: uint8_t, // pub d_namlen: uint8_t, - // pub d_name: [c_char; 256] + // pub d_name: [c_char; 256], // } - let mut name = dir_entry.file_name(); // not a Path as there are no separators! + // We just use the pointee type here since determining the right pointee type + // independently is highly non-trivial: it depends on which exact alias of the + // function was invoked (e.g. `fstat` vs `fstat64`), and then on FreeBSD it also + // depends on the ABI level which can be different between the libc used by std and + // the libc used by everyone else. + let dirent_ty = dest.layout.ty.builtin_deref(true).unwrap(); + let dirent_layout = this.layout_of(dirent_ty)?; + let fields = &dirent_layout.fields; + let d_name_offset = fields.offset(fields.count().strict_sub(1)).bytes(); + + // Determine the size of the buffer we have to allocate. + let mut name = dir_entry.name; // not a Path as there are no separators! name.push("\0"); // Add a NUL terminator let name_bytes = name.as_encoded_bytes(); let name_len = u64::try_from(name_bytes.len()).unwrap(); - - let dirent_layout = this.libc_ty_layout(dirent_type); - let fields = &dirent_layout.fields; - let last_field = fields.count().strict_sub(1); - let d_name_offset = fields.offset(last_field).bytes(); let size = d_name_offset.strict_add(name_len); let entry = this.allocate_ptr( @@ -981,11 +1027,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { )?; let entry = this.ptr_to_mplace(entry.into(), dirent_layout); - // Write common fields + // Write the name. + // The name is not a normal field, we already computed the offset above. + let name_ptr = entry.ptr().wrapping_offset(Size::from_bytes(d_name_offset), this); + this.write_bytes_ptr(name_ptr, name_bytes.iter().copied())?; + + // Write common fields. let ino_name = if this.tcx.sess.target.os == Os::FreeBsd { "d_fileno" } else { "d_ino" }; this.write_int_fields_named( - &[(ino_name, ino.into()), ("d_reclen", size.into())], + &[(ino_name, dir_entry.ino.into()), ("d_reclen", size.into())], &entry, )?; @@ -993,20 +1044,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if let Some(d_off) = this.try_project_field_named(&entry, "d_off")? { this.write_null(&d_off)?; } - if let Some(d_namlen) = this.try_project_field_named(&entry, "d_namlen")? { this.write_int(name_len.strict_sub(1), &d_namlen)?; } - - let file_type = this.file_type_to_d_type(dir_entry.file_type())?; if let Some(d_type) = this.try_project_field_named(&entry, "d_type")? { - this.write_int(file_type, &d_type)?; + this.write_int(dir_entry.d_type, &d_type)?; } - // The name is not a normal field, we already computed the offset above. - let name_ptr = entry.ptr().wrapping_offset(Size::from_bytes(d_name_offset), this); - this.write_bytes_ptr(name_ptr, name_bytes.iter().copied())?; - Some(entry.ptr()) } None => { @@ -1025,7 +1069,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.deallocate_ptr(old_entry, None, MiriMemoryKind::Runtime.into())?; } - interp_ok(Scalar::from_maybe_pointer(entry.unwrap_or_else(Pointer::null), this)) + this.write_pointer(entry.unwrap_or_else(Pointer::null), dest)?; + interp_ok(()) } fn macos_readdir_r( @@ -1051,8 +1096,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let open_dir = this.machine.dirs.streams.get_mut(&dirp).ok_or_else(|| { err_unsup_format!("the DIR pointer passed to readdir_r did not come from opendir") })?; - interp_ok(match open_dir.read_dir.next() { + interp_ok(match open_dir.next_host_entry() { Some(Ok(dir_entry)) => { + let dir_entry = this.dir_entry_fields(dir_entry)?; // Write into entry, write pointer to result, return 0 on success. // The name is written with write_os_str_to_c_str, while the rest of the // dirent struct is written using write_int_fields. @@ -1068,36 +1114,27 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // } let entry_place = this.deref_pointer_as(entry_op, this.libc_ty_layout("dirent"))?; - let name_place = this.project_field_named(&entry_place, "d_name")?; - let file_name = dir_entry.file_name(); // not a Path as there are no separators! + // Write the name. + let name_place = this.project_field_named(&entry_place, "d_name")?; let (name_fits, file_name_buf_len) = this.write_os_str_to_c_str( - &file_name, + &dir_entry.name, name_place.ptr(), name_place.layout.size.bytes(), )?; - let file_name_len = file_name_buf_len.strict_sub(1); if !name_fits { throw_unsup_format!( "a directory entry had a name too large to fit in libc::dirent" ); } - // If the host is a Unix system, fill in the inode number with its real value. - // If not, use 0 as a fallback value. - #[cfg(unix)] - let ino = std::os::unix::fs::DirEntryExt::ino(&dir_entry); - #[cfg(not(unix))] - let ino = 0u64; - - let file_type = this.file_type_to_d_type(dir_entry.file_type())?; - + // Write the other fields. this.write_int_fields_named( &[ - ("d_reclen", 0), - ("d_namlen", file_name_len.into()), - ("d_type", file_type.into()), - ("d_ino", ino.into()), + ("d_reclen", entry_place.layout.size.bytes().into()), + ("d_namlen", file_name_buf_len.strict_sub(1).into()), + ("d_type", dir_entry.d_type.into()), + ("d_ino", dir_entry.ino.into()), ("d_seekoff", 0), ], &entry_place, 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 426bc28ce8878..12cee0d162a0c 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -84,7 +84,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let fd = this.read_scalar(fd)?.to_i32()?; let offset = this.read_scalar(offset)?.to_int(offset.layout.size)?; let whence = this.read_scalar(whence)?.to_i32()?; - this.lseek64(fd, offset, whence, dest)?; + this.lseek(fd, offset, whence, dest)?; } "ftruncate64" => { let [fd, length] = this.check_shim_sig( @@ -114,10 +114,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(result, dest)?; } "readdir64" => { - // FIXME: This does not have a direct test (#3179). let [dirp] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - let result = this.readdir64("dirent64", dirp)?; - this.write_scalar(result, dest)?; + this.readdir(dirp, dest)?; } "sync_file_range" => { let [fd, offset, nbytes, flags] = diff --git a/src/tools/miri/src/shims/unix/macos/foreign_items.rs b/src/tools/miri/src/shims/unix/macos/foreign_items.rs index be4f2e66bab10..c9e9c30ac2c7d 100644 --- a/src/tools/miri/src/shims/unix/macos/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/macos/foreign_items.rs @@ -64,13 +64,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(result, dest)?; } "opendir$INODE64" => { - // FIXME: This does not have a direct test (#3179). let [name] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; let result = this.opendir(name)?; this.write_scalar(result, dest)?; } "readdir_r" | "readdir_r$INODE64" => { - // FIXME: This does not have a direct test (#3179). let [dirp, entry, result] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; let result = this.macos_readdir_r(dirp, entry, result)?; @@ -122,8 +120,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(result, dest)?; } - // FIXME: add a test that directly calls this function. "mach_wait_until" => { + // FIXME: This does not have a direct test (#3179). let [deadline] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; let result = this.mach_wait_until(deadline)?; this.write_scalar(result, dest)?; diff --git a/src/tools/miri/src/shims/unix/solarish/foreign_items.rs b/src/tools/miri/src/shims/unix/solarish/foreign_items.rs index f3918fdccf128..723e7bae4cdf8 100644 --- a/src/tools/miri/src/shims/unix/solarish/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/solarish/foreign_items.rs @@ -102,12 +102,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let result = this.lstat(path, buf)?; this.write_scalar(result, dest)?; } - "readdir" => { - // FIXME: This does not have a direct test (#3179). - let [dirp] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - let result = this.readdir64("dirent", dirp)?; - this.write_scalar(result, dest)?; - } // Sockets and pipes "__xnet_socketpair" => { diff --git a/src/tools/miri/src/shims/windows/handle.rs b/src/tools/miri/src/shims/windows/handle.rs index 92d6321bed1fd..b0d29bdd6f271 100644 --- a/src/tools/miri/src/shims/windows/handle.rs +++ b/src/tools/miri/src/shims/windows/handle.rs @@ -314,7 +314,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { "`DuplicateHandle` called on a thread handle, which is unsupported" ); } - Handle::Pseudo(pseudo) => Handle::Pseudo(pseudo), + Handle::Pseudo(_) => { + throw_unsup_format!( + "`DuplicateHandle` called on a pseudo handle, which is unsupported" + ); + } Handle::Null | Handle::Invalid => this.invalid_handle("DuplicateHandle")?, }; diff --git a/src/tools/miri/tests/fail-dep/concurrency/windows_join_main.rs b/src/tools/miri/tests/fail-dep/concurrency/windows_join_main.rs index a71778b1d0d49..8c085384a688d 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/windows_join_main.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/windows_join_main.rs @@ -12,7 +12,7 @@ use windows_sys::Win32::System::Threading::{INFINITE, WaitForSingleObject}; // XXX HACK: This is how miri represents the handle for thread 0. // This value can be "legitimately" obtained by using `GetCurrentThread` with `DuplicateHandle` -// but miri does not implement `DuplicateHandle` yet. (FIXME: it does now.) +// but miri does not implement `DuplicateHandle` for pseudo handles yet. const MAIN_THREAD: HANDLE = (2i32 << 29) as HANDLE; fn main() { diff --git a/src/tools/miri/tests/fail-dep/win/duplicate-pseudo-handle.rs b/src/tools/miri/tests/fail-dep/win/duplicate-pseudo-handle.rs new file mode 100644 index 0000000000000..c69ea419f7d2a --- /dev/null +++ b/src/tools/miri/tests/fail-dep/win/duplicate-pseudo-handle.rs @@ -0,0 +1,22 @@ +//@only-target: windows # Uses win32 api functions +use windows_sys::Win32::Foundation::{CloseHandle, DUPLICATE_SAME_ACCESS, DuplicateHandle}; +use windows_sys::Win32::System::Threading::{GetCurrentProcess, GetCurrentThread}; + +fn main() { + unsafe { + let cur_proc = GetCurrentProcess(); + + let pseudo = GetCurrentThread(); + let mut out = std::mem::zeroed(); + let res = + DuplicateHandle(cur_proc, pseudo, cur_proc, &mut out, 0, 0, DUPLICATE_SAME_ACCESS); + //~^ERROR: pseudo handle + assert!(res != 0); + assert!(out.addr() != 0); + // Since the original handle was a pseudo handle, we must return something different. + assert!(out != pseudo); + // And closing it should work (which it does not for a pseudo handle). + let res = CloseHandle(out); + assert!(res != 0); + } +} diff --git a/src/tools/miri/tests/fail-dep/win/duplicate-pseudo-handle.stderr b/src/tools/miri/tests/fail-dep/win/duplicate-pseudo-handle.stderr new file mode 100644 index 0000000000000..bbab2af677810 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/win/duplicate-pseudo-handle.stderr @@ -0,0 +1,12 @@ +error: unsupported operation: `DuplicateHandle` called on a pseudo handle, which is unsupported + --> tests/fail-dep/win/duplicate-pseudo-handle.rs:LL:CC + | +LL | DuplicateHandle(cur_proc, pseudo, cur_proc, &mut out, 0, 0, DUPLICATE_SAME_ACCESS); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unsupported operation occurred here + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support + +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/libc-fs.rs b/src/tools/miri/tests/pass-dep/libc/libc-fs.rs index 00d5f7d97e281..f5e9a56d7d039 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-fs.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-fs.rs @@ -48,6 +48,8 @@ fn main() { test_nofollow_not_symlink(); #[cfg(target_os = "macos")] test_ioctl(); + test_opendir_closedir(); + test_readdir(); } fn test_file_open_unix_allow_two_args() { @@ -579,3 +581,83 @@ fn test_ioctl() { assert_eq!(libc::ioctl(fd, libc::FIOCLEX), 0); } } + +fn test_opendir_closedir() { + // dir should exist + use std::fs::{create_dir, remove_dir}; + let path = utils::prepare_dir("miri_test_libc_opendir_closedir"); + create_dir(&path).expect("create_dir failed"); + let cpath = CString::new(path.as_os_str().as_bytes()).expect("CString::new failed"); + let dir: *mut libc::DIR = unsafe { libc::opendir(cpath.as_ptr()) }; + assert!(!dir.is_null()); + assert_eq!(unsafe { libc::closedir(dir) }, 0); + + // dir should not exist + remove_dir(&path).unwrap(); + let dir: *mut libc::DIR = unsafe { libc::opendir(cpath.as_ptr()) }; + assert!(dir.is_null()); + let e = std::io::Error::last_os_error(); + assert_eq!(e.raw_os_error(), Some(libc::ENOENT)); + assert_eq!(e.kind(), ErrorKind::NotFound); + + // open normal file as dir should fail + let file_path = utils::prepare_with_content("test_not_a_dir.txt", b"hello"); + let cfile = CString::new(file_path.as_os_str().as_bytes()).expect("CString::new failed"); + let dir: *mut libc::DIR = unsafe { libc::opendir(cfile.as_ptr()) }; + assert!(dir.is_null()); + let e = std::io::Error::last_os_error(); + assert_eq!(e.raw_os_error(), Some(libc::ENOTDIR)); + assert_eq!(e.kind(), ErrorKind::NotADirectory); + remove_file(&file_path).unwrap(); +} + +fn test_readdir() { + use std::fs::{create_dir, remove_dir, write}; + + let dir_path = utils::prepare_dir("miri_test_libc_readdir"); + create_dir(&dir_path).ok(); + + // Create test files + let file1 = dir_path.join("file1.txt"); + let file2 = dir_path.join("file2.txt"); + write(&file1, b"content1").unwrap(); + write(&file2, b"content2").unwrap(); + + let c_path = CString::new(dir_path.as_os_str().as_bytes()).unwrap(); + + unsafe { + let dirp = libc::opendir(c_path.as_ptr()); + assert!(!dirp.is_null()); + let mut entries = Vec::new(); + loop { + cfg_if::cfg_if! { + if #[cfg(target_os = "macos")] { + // On macos we only support readdir_r as that's what std uses there. + use std::mem::MaybeUninit; + use libc::dirent; + let mut entry: MaybeUninit = MaybeUninit::uninit(); + let mut result: *mut dirent = std::ptr::null_mut(); + let ret = libc::readdir_r(dirp, entry.as_mut_ptr(), &mut result); + assert_eq!(ret, 0); + let entry_ptr = result; + } else { + let entry_ptr = libc::readdir(dirp); + } + } + if entry_ptr.is_null() { + break; + } + let name_ptr = std::ptr::addr_of!((*entry_ptr).d_name) as *const libc::c_char; + let name = CStr::from_ptr(name_ptr); + let name_str = name.to_string_lossy(); + entries.push(name_str.into_owned()); + } + assert_eq!(libc::closedir(dirp), 0); + entries.sort(); + assert_eq!(&entries, &[".", "..", "file1.txt", "file2.txt"]); + } + + remove_file(&file1).unwrap(); + remove_file(&file2).unwrap(); + remove_dir(&dir_path).unwrap(); +}