diff --git a/library/std/src/sys/fs/unix.rs b/library/std/src/sys/fs/unix.rs index 7283e8f710c55..48b43ea77a2c6 100644 --- a/library/std/src/sys/fs/unix.rs +++ b/library/std/src/sys/fs/unix.rs @@ -21,42 +21,6 @@ use libc::dirfd; use libc::fstatat as fstatat64; #[cfg(any(all(target_os = "linux", not(target_env = "musl")), target_os = "hurd"))] use libc::fstatat64; -#[cfg(any( - target_os = "aix", - target_os = "android", - target_os = "freebsd", - target_os = "fuchsia", - target_os = "illumos", - target_os = "nto", - target_os = "qnx", - target_os = "redox", - target_os = "solaris", - target_os = "vita", - target_os = "wasi", - all(target_os = "linux", target_env = "musl"), -))] -use libc::readdir as readdir64; -#[cfg(not(any( - target_os = "aix", - target_os = "android", - target_os = "freebsd", - target_os = "fuchsia", - target_os = "hurd", - target_os = "illumos", - target_os = "l4re", - target_os = "linux", - target_os = "nto", - target_os = "qnx", - target_os = "redox", - target_os = "solaris", - target_os = "vita", - target_os = "wasi", -)))] -use libc::readdir_r as readdir64_r; -#[cfg(any(all(target_os = "linux", not(target_env = "musl")), target_os = "hurd"))] -use libc::readdir64; -#[cfg(target_os = "l4re")] -use libc::readdir64_r; use libc::{c_int, mode_t}; #[cfg(target_os = "android")] use libc::{ @@ -404,21 +368,6 @@ fn get_path_from_fd(fd: c_int) -> Option { get_path(fd) } -#[cfg(any( - target_os = "aix", - target_os = "android", - target_os = "freebsd", - target_os = "fuchsia", - target_os = "hurd", - target_os = "illumos", - target_os = "linux", - target_os = "nto", - target_os = "qnx", - target_os = "redox", - target_os = "solaris", - target_os = "vita", - target_os = "wasi", -))] pub struct DirEntry { dir: Arc, entry: dirent64_min, @@ -431,26 +380,13 @@ pub struct DirEntry { // Define a minimal subset of fields we need from `dirent64`, especially since // we're not using the immediate `d_name` on these targets. Keeping this as an // `entry` field in `DirEntry` helps reduce the `cfg` boilerplate elsewhere. -#[cfg(any( - target_os = "aix", - target_os = "android", - target_os = "freebsd", - target_os = "fuchsia", - target_os = "hurd", - target_os = "illumos", - target_os = "linux", - target_os = "nto", - target_os = "qnx", - target_os = "redox", - target_os = "solaris", - target_os = "vita", - target_os = "wasi", -))] struct dirent64_min { d_ino: u64, #[cfg(not(any( target_os = "solaris", target_os = "illumos", + target_os = "haiku", + target_os = "vxworks", target_os = "aix", target_os = "nto", target_os = "qnx", @@ -459,27 +395,6 @@ struct dirent64_min { d_type: u8, } -#[cfg(not(any( - target_os = "aix", - target_os = "android", - target_os = "freebsd", - target_os = "fuchsia", - target_os = "hurd", - target_os = "illumos", - target_os = "linux", - target_os = "nto", - target_os = "qnx", - target_os = "redox", - target_os = "solaris", - target_os = "vita", - target_os = "wasi", -)))] -pub struct DirEntry { - dir: Arc, - // The full entry includes a fixed-length `d_name`. - entry: dirent64, -} - #[derive(Clone)] pub struct OpenOptions { // generic @@ -858,48 +773,87 @@ impl fmt::Debug for ReadDir { impl Iterator for ReadDir { type Item = io::Result; - #[cfg(any( - target_os = "aix", - target_os = "android", - target_os = "freebsd", - target_os = "fuchsia", - target_os = "hurd", - target_os = "illumos", - target_os = "linux", - target_os = "nto", - target_os = "qnx", - target_os = "redox", - target_os = "solaris", - target_os = "vita", - target_os = "wasi", - ))] fn next(&mut self) -> Option> { - use crate::sys::io::{errno, set_errno}; - if self.end_of_stream { return None; } unsafe { loop { - // As of POSIX.1-2017, readdir() is not required to be thread safe; only - // readdir_r() is. However, readdir_r() cannot correctly handle platforms - // with unlimited or variable NAME_MAX. Many modern platforms guarantee - // thread safety for readdir() as long an individual DIR* is not accessed - // concurrently, which is sufficient for Rust. - set_errno(0); - let entry_ptr: *const dirent64 = readdir64(self.inner.dirp.0); - if entry_ptr.is_null() { - // We either encountered an error, or reached the end. Either way, - // the next call to next() should return None. - self.end_of_stream = true; - - // To distinguish between errors and end-of-directory, we had to clear - // errno beforehand to check for an error now. - return match errno() { - 0 => None, - e => Some(Err(Error::from_raw_os_error(e))), - }; + // POSIX.1-2024 formalized what was already guaranteed by a lot + // of implementations and required readdir() to be thread-safe as + // long as an individual DIR* is not accessed concurrently. Taking + // a mutable reference to the `ReadDir` iterator prevents that. + // Even POSIX.1-1994 specified that the data in the returned + // dirent + // > is not overwritten by another call to readdir() on a + // > different directory stream. + // + // and that guarantee together with the requirement that the + // underlying syscalls need to be thread-safe because of readdir_r + // make it very unlikely for an implementation to be non-conforming. + // Nevertheless, there are still some platforms where we either + // cannot confirm `readdir` to be thread-safe or know that it + // isn't. + cfg_select! { + any( + target_os = "espidf", // readdir truly isn't thread-safe. + target_os = "lynxos178", + target_os = "qurt", + target_os = "rtems", + target_os = "vxworks", + ) => { + use crate::mem::MaybeUninit; + + let mut entry = MaybeUninit::uninit(); + let mut entry_ptr: *mut dirent64 = ptr::null_mut(); + let err = libc::readdir_r(self.inner.dirp.0, entry.as_mut_ptr(), &mut entry_ptr); + if err != 0 { + if entry_ptr.is_null() { + // We encountered an error (which will be returned in this iteration), but + // we also reached the end of the directory stream. The `end_of_stream` + // flag is enabled to make sure that we return `None` in the next iteration + // (instead of looping forever) + self.end_of_stream = true; + } + return Some(Err(Error::from_raw_os_error(err))); + } + if entry_ptr.is_null() { + return None; + } + + let entry_ptr = entry_ptr.cast_const(); + } + _ => { + #[cfg(not(any( + all(target_os = "linux", not(target_env = "musl")), + target_os = "hurd", + target_os = "l4re", + )))] + use libc::readdir as readdir64; + #[cfg(any( + all(target_os = "linux", not(target_env = "musl")), + target_os = "hurd", + target_os = "l4re" + ))] + use libc::readdir64; + use crate::sys::io::{errno, set_errno}; + + set_errno(0); + let entry_ptr: *const dirent64 = readdir64(self.inner.dirp.0); + if entry_ptr.is_null() { + // We either encountered an error, or reached the end. Either way, + // the next call to next() should return None. + self.end_of_stream = true; + + // To distinguish between errors and end-of-directory, we had to clear + // errno beforehand to check for an error now. + return match errno() { + 0 => None, + e => Some(Err(Error::from_raw_os_error(e))), + }; + } + } } // The dirent64 struct is a weird imaginary thing that isn't ever supposed @@ -930,25 +884,38 @@ impl Iterator for ReadDir { // When loading from a field, we can skip the `&raw const`; `(*entry_ptr).d_ino` as // a value expression will do the right thing: `byte_offset` to the field and then // only access those bytes. - #[cfg(not(target_os = "vita"))] let entry = dirent64_min { - #[cfg(target_os = "freebsd")] + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "netbsd", + target_os = "openbsd", + ))] d_ino: (*entry_ptr).d_fileno, - #[cfg(not(target_os = "freebsd"))] + #[cfg(any(target_os = "nuttx", target_os = "vita",))] + d_ino: 0, + #[cfg(not(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "netbsd", + target_os = "nuttx", + target_os = "openbsd", + target_os = "vita", + )))] d_ino: (*entry_ptr).d_ino as u64, #[cfg(not(any( target_os = "solaris", target_os = "illumos", + target_os = "haiku", + target_os = "vxworks", target_os = "aix", target_os = "nto", target_os = "qnx", + target_os = "vita", )))] d_type: (*entry_ptr).d_type as u8, }; - #[cfg(target_os = "vita")] - let entry = dirent64_min { d_ino: 0u64 }; - return Some(Ok(DirEntry { entry, name: name.to_owned(), @@ -957,51 +924,6 @@ impl Iterator for ReadDir { } } } - - #[cfg(not(any( - target_os = "aix", - target_os = "android", - target_os = "freebsd", - target_os = "fuchsia", - target_os = "hurd", - target_os = "illumos", - target_os = "linux", - target_os = "nto", - target_os = "qnx", - target_os = "redox", - target_os = "solaris", - target_os = "vita", - target_os = "wasi", - )))] - fn next(&mut self) -> Option> { - if self.end_of_stream { - return None; - } - - unsafe { - let mut ret = DirEntry { entry: mem::zeroed(), dir: Arc::clone(&self.inner) }; - let mut entry_ptr = ptr::null_mut(); - loop { - let err = readdir64_r(self.inner.dirp.0, &mut ret.entry, &mut entry_ptr); - if err != 0 { - if entry_ptr.is_null() { - // We encountered an error (which will be returned in this iteration), but - // we also reached the end of the directory stream. The `end_of_stream` - // flag is enabled to make sure that we return `None` in the next iteration - // (instead of looping forever) - self.end_of_stream = true; - } - return Some(Err(Error::from_raw_os_error(err))); - } - if entry_ptr.is_null() { - return None; - } - if ret.name_bytes() != b"." && ret.name_bytes() != b".." { - return Some(Ok(ret)); - } - } - } - } } /// Aborts the process if a file desceriptor is not open, if debug asserts are enabled @@ -1080,7 +1002,7 @@ impl DirEntry { ))] pub fn metadata(&self) -> io::Result { let fd = cvt(unsafe { dirfd(self.dir.dirp.0) })?; - let name = self.name_cstr().as_ptr(); + let name = self.name.as_ptr(); cfg_has_statx! { if let Some(ret) = unsafe { try_statx( @@ -1150,110 +1072,12 @@ impl DirEntry { } } - #[cfg(any( - target_os = "aix", - target_os = "android", - target_os = "cygwin", - target_os = "emscripten", - target_os = "espidf", - target_os = "freebsd", - target_os = "fuchsia", - target_os = "haiku", - target_os = "horizon", - target_os = "hurd", - target_os = "illumos", - target_os = "l4re", - target_os = "linux", - target_os = "nto", - target_os = "qnx", - target_os = "redox", - target_os = "rtems", - target_os = "solaris", - target_os = "vita", - target_os = "vxworks", - target_os = "wasi", - target_vendor = "apple", - ))] - pub fn ino(&self) -> u64 { - self.entry.d_ino as u64 - } - - #[cfg(any(target_os = "openbsd", target_os = "netbsd", target_os = "dragonfly"))] - pub fn ino(&self) -> u64 { - self.entry.d_fileno as u64 - } - - #[cfg(target_os = "nuttx")] pub fn ino(&self) -> u64 { - // Leave this 0 for now, as NuttX does not provide an inode number - // in its directory entries. - 0 - } - - #[cfg(any( - target_os = "netbsd", - target_os = "openbsd", - target_os = "dragonfly", - target_vendor = "apple", - ))] - fn name_bytes(&self) -> &[u8] { - use crate::slice; - unsafe { - slice::from_raw_parts( - self.entry.d_name.as_ptr() as *const u8, - self.entry.d_namlen as usize, - ) - } - } - #[cfg(not(any( - target_os = "netbsd", - target_os = "openbsd", - target_os = "dragonfly", - target_vendor = "apple", - )))] - fn name_bytes(&self) -> &[u8] { - self.name_cstr().to_bytes() - } - - #[cfg(not(any( - target_os = "android", - target_os = "freebsd", - target_os = "linux", - target_os = "solaris", - target_os = "illumos", - target_os = "fuchsia", - target_os = "redox", - target_os = "aix", - target_os = "nto", - target_os = "qnx", - target_os = "vita", - target_os = "hurd", - target_os = "wasi", - )))] - fn name_cstr(&self) -> &CStr { - unsafe { CStr::from_ptr(self.entry.d_name.as_ptr()) } - } - #[cfg(any( - target_os = "android", - target_os = "freebsd", - target_os = "linux", - target_os = "solaris", - target_os = "illumos", - target_os = "fuchsia", - target_os = "redox", - target_os = "aix", - target_os = "nto", - target_os = "qnx", - target_os = "vita", - target_os = "hurd", - target_os = "wasi", - ))] - fn name_cstr(&self) -> &CStr { - &self.name + self.entry.d_ino } pub fn file_name_os_str(&self) -> &OsStr { - OsStr::from_bytes(self.name_bytes()) + OsStr::from_bytes(self.name.as_bytes()) } } @@ -2558,24 +2382,23 @@ mod remove_dir_impl { for child in dir { let child = child?; - let child_name = child.name_cstr(); // we need an inner try block, because if one of these // directories has already been deleted, then we need to // continue the loop, not return ok. let result: io::Result<()> = try { match is_dir(&child) { Some(true) => { - remove_dir_all_recursive(Some(fd), child_name)?; + remove_dir_all_recursive(Some(fd), &child.name)?; } Some(false) => { - cvt(unsafe { unlinkat(fd, child_name.as_ptr(), 0) })?; + cvt(unsafe { unlinkat(fd, child.name.as_ptr(), 0) })?; } None => { // POSIX specifies that calling unlink()/unlinkat(..., 0) on a directory can succeed // if the process has the appropriate privileges. This however can causing orphaned // directories requiring an fsck e.g. on Solaris and Illumos. So we try recursing // into it first instead of trying to unlink() it. - remove_dir_all_recursive(Some(fd), child_name)?; + remove_dir_all_recursive(Some(fd), &child.name)?; } } }; diff --git a/library/std/src/sys/io/error/unix.rs b/library/std/src/sys/io/error/unix.rs index 2feeefa2530c8..16091e0e340a3 100644 --- a/library/std/src/sys/io/error/unix.rs +++ b/library/std/src/sys/io/error/unix.rs @@ -60,11 +60,13 @@ pub fn errno() -> i32 { // needed for readdir and syscall! #[cfg(not(any( target_os = "dragonfly", - target_os = "vxworks", + target_os = "espidf", + target_os = "lynxos178", + target_os = "qurt", target_os = "rtems", + target_os = "vxworks", target_os = "wasi", )))] -#[allow(dead_code)] // but not all target cfgs actually end up using it #[inline] pub fn set_errno(e: i32) { unsafe { *errno_location() = e as c_int } @@ -92,14 +94,13 @@ pub fn errno() -> i32 { pub fn errno() -> i32 { unsafe extern "C" { #[thread_local] - static errno: c_int; + static mut errno: c_int; } unsafe { errno as i32 } } #[cfg(target_os = "dragonfly")] -#[allow(dead_code)] #[inline] pub fn set_errno(e: i32) { unsafe extern "C" { @@ -107,9 +108,7 @@ pub fn set_errno(e: i32) { static mut errno: c_int; } - unsafe { - errno = e; - } + unsafe { errno = e }; } #[cfg(target_os = "wasi")] diff --git a/library/std/src/sys/io/mod.rs b/library/std/src/sys/io/mod.rs index 0158137174087..d3b9b8788464e 100644 --- a/library/std/src/sys/io/mod.rs +++ b/library/std/src/sys/io/mod.rs @@ -35,9 +35,17 @@ mod kernel_copy; not(any(target_os = "dragonfly", target_os = "vxworks", target_os = "rtems")) ))] pub use error::errno_location; -#[cfg_attr(not(target_os = "linux"), allow(unused_imports))] #[cfg(any( - all(target_family = "unix", not(any(target_os = "vxworks", target_os = "rtems"))), + all( + target_family = "unix", + not(any( + target_os = "espidf", + target_os = "lynxos178", + target_os = "qurt", + target_os = "rtems", + target_os = "vxworks", + )) + ), target_os = "wasi", ))] pub use error::set_errno; diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 659125fa3388c..06bda203b6d18 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -1083,9 +1083,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if !matches!( &this.tcx.sess.target.os, - Os::Linux | Os::Android | Os::Solaris | Os::Illumos | Os::FreeBsd + Os::Linux | Os::Android | Os::Solaris | Os::Illumos | Os::FreeBsd | Os::MacOs ) { - panic!("`readdir` should not be called on {}", this.tcx.sess.target.os); + throw_unsup_format!("`readdir` is not yet supported on {}", this.tcx.sess.target.os); } let dirp = this.read_target_usize(dirp_op)?; @@ -1136,6 +1136,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // pub d_namlen: uint8_t, // pub d_name: [c_char; 256], // } + // + // On macOS: + // pub struct dirent { + // pub d_ino: u64, + // pub d_seekoff: u64, + // pub d_reclen: u16, + // pub d_namlen: u16, + // pub d_type: u8, + // pub d_name: [c_char; 1024], + // } // 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 @@ -1179,6 +1189,9 @@ 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_seekoff) = this.try_project_field_named(&entry, "d_seekoff")? { + this.write_null(&d_seekoff)?; + } if let Some(d_namlen) = this.try_project_field_named(&entry, "d_namlen")? { this.write_int(name_len.strict_sub(1), &d_namlen)?; } 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 d9d00d7ba239b..8e6ef116ba0c2 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-fs.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-fs.rs @@ -54,6 +54,8 @@ fn main() { test_ioctl(); test_opendir_closedir(); test_readdir(); + #[cfg(target_os = "macos")] + test_readdir_r(); #[cfg(target_os = "linux")] test_statx_on_file_path(); #[cfg(target_os = "linux")] @@ -856,21 +858,53 @@ fn test_readdir() { assert!(!dirp.is_null()); let mut entries = Vec::new(); loop { - cfg_select! { - 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; - } - _ => { - let entry_ptr = libc::readdir(dirp); - } + 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(); +} + +// We only support `readdir_r` on macOS. +// (It is deprecated so we don't want to add more support.) +#[cfg(target_os = "macos")] +fn test_readdir_r() { + use std::fs::{create_dir, remove_dir, write}; + use std::mem::MaybeUninit; + + let dir_path = utils::prepare_dir("miri_test_libc_readdir_r"); + 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 { + let mut entry: MaybeUninit = MaybeUninit::uninit(); + let mut result: *mut libc::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; if entry_ptr.is_null() { break; }