From 4cd367f029653af104123aad057a261069a28d67 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 23 Jan 2026 17:15:07 +0100 Subject: [PATCH] fix readdir on freebsd, and minor tweaks --- .../src/shims/unix/android/foreign_items.rs | 2 +- .../miri/src/shims/unix/foreign_items.rs | 5 ++--- .../src/shims/unix/freebsd/foreign_items.rs | 3 +-- src/tools/miri/src/shims/unix/fs.rs | 22 +++++++++++++------ .../src/shims/unix/linux/foreign_items.rs | 5 ++--- src/tools/miri/tests/pass-dep/libc/libc-fs.rs | 12 ++++------ 6 files changed, 25 insertions(+), 24 deletions(-) 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 8619f1afbdf2..5259ca229488 100644 --- a/src/tools/miri/src/shims/unix/android/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/android/foreign_items.rs @@ -79,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 4f8adb1b3148..87a307c98948 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -384,8 +384,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } "readdir" => { 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)?; } "lseek" => { // FIXME: This does not have a direct test (#3179). @@ -398,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 97f8128a145d..f64ee3b16220 100644 --- a/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs @@ -157,8 +157,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } "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 f43fd3fe2d18..28f3bad4e194 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -443,7 +443,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 +899,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,11 +915,12 @@ 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() { @@ -967,7 +968,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { 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); + // 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 last_field = fields.count().strict_sub(1); let d_name_offset = fields.offset(last_field).bytes(); @@ -1025,7 +1032,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( 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 f743c32e1cae..12cee0d162a0 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( @@ -115,8 +115,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } "readdir64" => { 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/tests/pass-dep/libc/libc-fs.rs b/src/tools/miri/tests/pass-dep/libc/libc-fs.rs index b6064afadce8..ecf5a23e7268 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-fs.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-fs.rs @@ -614,7 +614,6 @@ fn test_opendir_closedir() { fn test_readdir() { use std::fs::{create_dir, remove_dir, write}; - use libc::opendir; let dir_path = utils::prepare_dir("miri_test_libc_readdir"); create_dir(&dir_path).ok(); @@ -627,12 +626,13 @@ fn test_readdir() { let c_path = CString::new(dir_path.as_os_str().as_bytes()).unwrap(); unsafe { - let dirp = opendir(c_path.as_ptr()); + 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(); @@ -641,8 +641,6 @@ fn test_readdir() { assert_eq!(ret, 0); let entry_ptr = result; } else { - // TODO : Fix the bug with FreeBSD (reading files with empty names) - // run : ./miri test --target x86_64-unknown-freebsd tests/pass-dep/libc/libc-fs.rs let entry_ptr = libc::readdir(dirp); } } @@ -652,13 +650,11 @@ fn test_readdir() { 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(); - if name_str != "." && name_str != ".." { - entries.push(name_str.into_owned()); - } + entries.push(name_str.into_owned()); } assert_eq!(libc::closedir(dirp), 0); entries.sort(); - assert_eq!(entries, vec!["file1.txt", "file2.txt"]); + assert_eq!(&entries, &["file1.txt", "file2.txt"]); } remove_file(&file1).unwrap();