From 7e2a9f21bcd8437a2b54853f047665937558e219 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 2 Dec 2024 07:31:44 +0000 Subject: [PATCH] solarish stat following-up, supports for readdir. --- src/helpers.rs | 25 +++++++++----- src/shims/unix/fs.rs | 44 +++++++++++++++++------- src/shims/unix/linux/foreign_items.rs | 2 +- src/shims/unix/solarish/foreign_items.rs | 6 ++++ tests/pass/shims/fs.rs | 14 ++++---- 5 files changed, 64 insertions(+), 27 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index b57ce4e070..2e21bca737 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -309,18 +309,30 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } /// Project to the given *named* field (which must be a struct or union type). - fn project_field_named>( + fn try_project_field_named>( &self, base: &P, name: &str, - ) -> InterpResult<'tcx, P> { + ) -> InterpResult<'tcx, Option

> { let this = self.eval_context_ref(); let adt = base.layout().ty.ty_adt_def().unwrap(); for (idx, field) in adt.non_enum_variant().fields.iter().enumerate() { if field.name.as_str() == name { - return this.project_field(base, idx); + return interp_ok(Some(this.project_field(base, idx)?)); } } + interp_ok(None) + } + + /// Project to the given *named* field (which must be a struct or union type). + fn project_field_named>( + &self, + base: &P, + name: &str, + ) -> InterpResult<'tcx, P> { + if let Some(field) = self.try_project_field_named(base, name)? { + return interp_ok(field); + } bug!("No field named {} in type {}", name, base.layout().ty); } @@ -330,11 +342,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { base: &P, name: &str, ) -> bool { - let adt = base.layout().ty.ty_adt_def().unwrap(); - for field in adt.non_enum_variant().fields.iter() { - if field.name.as_str() == name { - return true; - } + if let Some(_f) = self.try_project_field_named(base, name).unwrap() { + return true; } false } diff --git a/src/shims/unix/fs.rs b/src/shims/unix/fs.rs index b41a4d2246..1116f24c34 100644 --- a/src/shims/unix/fs.rs +++ b/src/shims/unix/fs.rs @@ -1048,10 +1048,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } } - fn linux_readdir64(&mut self, dirp_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { + fn linux_solarish_readdir64(&mut self, dirp_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - this.assert_target_os("linux", "readdir64"); + if !matches!(&*this.tcx.sess.target.os, "linux" | "solaris" | "illumos") { + panic!("`linux_solaris_readdir64` should not be called on {}", this.tcx.sess.target.os); + } let dirp = this.read_target_usize(dirp_op)?; @@ -1070,9 +1072,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { Some(Ok(dir_entry)) => { // Write the directory entry into a newly allocated buffer. // The name is written with write_bytes, while the rest of the - // dirent64 struct is written using write_int_fields. + // dirent64 (or dirent) struct is written using write_int_fields. // For reference: + // On Linux: // pub struct dirent64 { // pub d_ino: ino64_t, // pub d_off: off64_t, @@ -1080,14 +1083,30 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // pub d_type: c_uchar, // pub d_name: [c_char; 256], // } + // + // On Solaris: + // pub struct dirent { + // pub d_ino: ino64_t, + // pub d_off: off64_t, + // pub d_reclen: c_ushort, + // pub d_name: [c_char; 3], + // } let mut name = dir_entry.file_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 dirent64_layout = this.libc_ty_layout("dirent64"); - let d_name_offset = dirent64_layout.fields.offset(4 /* d_name */).bytes(); + let is_linux = matches!(&*this.tcx.sess.target.os, "linux"); + + let dirent64_layout = if is_linux { + this.libc_ty_layout("dirent64") + } else { + this.libc_ty_layout("dirent") + }; + let fields = &dirent64_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( @@ -1105,17 +1124,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let ino = 0u64; let file_type = this.file_type_to_d_type(dir_entry.file_type())?; - this.write_int_fields_named( - &[ - ("d_ino", ino.into()), - ("d_off", 0), - ("d_reclen", size.into()), - ("d_type", file_type.into()), - ], + &[("d_ino", ino.into()), ("d_off", 0), ("d_reclen", size.into())], &this.ptr_to_mplace(entry, dirent64_layout), )?; + if let Some(d_type) = this.try_project_field_named( + &this.ptr_to_mplace(entry, dirent64_layout), + "d_type", + )? { + this.write_int(file_type, &d_type)?; + } + let name_ptr = entry.wrapping_offset(Size::from_bytes(d_name_offset), this); this.write_bytes_ptr(name_ptr, name_bytes.iter().copied())?; diff --git a/src/shims/unix/linux/foreign_items.rs b/src/shims/unix/linux/foreign_items.rs index bc3619090c..6697fa49e4 100644 --- a/src/shims/unix/linux/foreign_items.rs +++ b/src/shims/unix/linux/foreign_items.rs @@ -37,7 +37,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { "readdir64" => { let [dirp] = this.check_shim(abi, ExternAbi::C { unwind: false }, link_name, args)?; - let result = this.linux_readdir64(dirp)?; + let result = this.linux_solarish_readdir64(dirp)?; this.write_scalar(result, dest)?; } "sync_file_range" => { diff --git a/src/shims/unix/solarish/foreign_items.rs b/src/shims/unix/solarish/foreign_items.rs index e452917036..cf0b432667 100644 --- a/src/shims/unix/solarish/foreign_items.rs +++ b/src/shims/unix/solarish/foreign_items.rs @@ -76,6 +76,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let result = this.macos_fbsd_solaris_fstat(fd, buf)?; this.write_scalar(result, dest)?; } + "readdir" => { + let [dirp] = + this.check_shim(abi, ExternAbi::C { unwind: false }, link_name, args)?; + let result = this.linux_solarish_readdir64(dirp)?; + this.write_scalar(result, dest)?; + } // Miscellaneous "___errno" => { diff --git a/tests/pass/shims/fs.rs b/tests/pass/shims/fs.rs index 3e514d95ee..289c6aa2fc 100644 --- a/tests/pass/shims/fs.rs +++ b/tests/pass/shims/fs.rs @@ -27,11 +27,8 @@ fn main() { test_file_sync(); test_errors(); test_rename(); - // solarish needs to support readdir/readdir64 for these tests. - if cfg!(not(any(target_os = "solaris", target_os = "illumos"))) { - test_directory(); - test_canonicalize(); - } + test_directory(); + test_canonicalize(); test_from_raw_os_error(); #[cfg(unix)] test_pread_pwrite(); @@ -279,7 +276,12 @@ fn test_directory() { .collect::>() ); // Deleting the directory should fail, since it is not empty. - assert_eq!(ErrorKind::DirectoryNotEmpty, remove_dir(&dir_path).unwrap_err().kind()); + + // Solaris/Illumos `rmdir` call set errno to EEXIST if directory contains + // other entries than `.` and `..`. + // https://docs.oracle.com/cd/E86824_01/html/E54765/rmdir-2.html + let err = remove_dir(&dir_path).unwrap_err().kind(); + assert!(matches!(err, ErrorKind::AlreadyExists | ErrorKind::DirectoryNotEmpty)); // Clean up the files in the directory remove_file(&path_1).unwrap(); remove_file(&path_2).unwrap();