From b0c7625dd109197cc38710da64e5eeb401f97379 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Sat, 30 Nov 2019 15:09:52 -0500 Subject: [PATCH] add `statx` shim for linux --- src/helpers.rs | 42 ++++++++ src/shims/env.rs | 8 +- src/shims/foreign_items.rs | 15 ++- src/shims/fs.rs | 198 ++++++++++++++++++++++++++++++++++++- src/shims/time.rs | 39 +++----- tests/run-pass/fs.rs | 38 ++++++- 6 files changed, 303 insertions(+), 37 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 05a92164f27d..ac63a3a9b3a4 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -509,3 +509,45 @@ fn bytes_to_os_str<'tcx, 'a>(bytes: &'a [u8]) -> InterpResult<'tcx, &'a OsStr> { .map_err(|_| err_unsup_format!("{:?} is not a valid utf-8 string", bytes))?; Ok(&OsStr::new(s)) } + +// FIXME: change `ImmTy::from_int` so it returns an `InterpResult` instead and remove this +// function. +pub fn immty_from_int_checked<'tcx>( + int: impl Into, + layout: TyLayout<'tcx>, +) -> InterpResult<'tcx, ImmTy<'tcx, Tag>> { + let int = int.into(); + // If `int` does not fit in `size` bits, we error instead of letting + // `ImmTy::from_int` panic. + let size = layout.size; + let truncated = truncate(int as u128, size); + if sign_extend(truncated, size) as i128 != int { + throw_unsup_format!( + "Signed value {:#x} does not fit in {} bits", + int, + size.bits() + ) + } + Ok(ImmTy::from_int(int, layout)) +} + +// FIXME: change `ImmTy::from_uint` so it returns an `InterpResult` instead and remove this +// function. +pub fn immty_from_uint_checked<'tcx>( + int: impl Into, + layout: TyLayout<'tcx>, +) -> InterpResult<'tcx, ImmTy<'tcx, Tag>> { + let int = int.into(); + // If `int` does not fit in `size` bits, we error instead of letting + // `ImmTy::from_int` panic. + let size = layout.size; + if truncate(int, size) != int { + throw_unsup_format!( + "Unsigned value {:#x} does not fit in {} bits", + int, + size.bits() + ) + } + Ok(ImmTy::from_uint(int, layout)) +} + diff --git a/src/shims/env.rs b/src/shims/env.rs index 8c431fd2016d..3994cf78780a 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -20,8 +20,12 @@ impl EnvVars { ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>, mut excluded_env_vars: Vec, ) { - // Exclude `TERM` var to avoid terminfo trying to open the termcap file. - excluded_env_vars.push("TERM".to_owned()); + + // FIXME: this can be removed when we have the `stat64` shim for macos. + if ecx.tcx.sess.target.target.target_os.to_lowercase() != "linux" { + // Exclude `TERM` var to avoid terminfo trying to open the termcap file. + excluded_env_vars.push("TERM".to_owned()); + } if ecx.machine.communicate { for (name, value) in env::vars() { diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 316cc686ad77..3989f7f48a0b 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -303,14 +303,25 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx .expect("Failed to get libc::SYS_getrandom") .to_machine_usize(this)?; - // `libc::syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), GRND_NONBLOCK)` - // is called if a `HashMap` is created the regular way (e.g. HashMap). + let sys_statx = this + .eval_path_scalar(&["libc", "SYS_statx"])? + .expect("Failed to get libc::SYS_statx") + .to_machine_usize(this)?; + match this.read_scalar(args[0])?.to_machine_usize(this)? { + // `libc::syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), GRND_NONBLOCK)` + // is called if a `HashMap` is created the regular way (e.g. HashMap). id if id == sys_getrandom => { // The first argument is the syscall id, // so skip over it. linux_getrandom(this, &args[1..], dest)?; } + id if id == sys_statx => { + // The first argument is the syscall id, + // so skip over it. + let result = this.statx(args[1], args[2], args[3], args[4], args[5])?; + this.write_scalar(Scalar::from_int(result, dest.layout.size), dest)?; + } id => throw_unsup_format!("miri does not support syscall ID {}", id), } } diff --git a/src/shims/fs.rs b/src/shims/fs.rs index 4d10be626460..47f3f50b76af 100644 --- a/src/shims/fs.rs +++ b/src/shims/fs.rs @@ -1,12 +1,16 @@ use std::collections::HashMap; -use std::convert::TryFrom; +use std::convert::{TryInto, TryFrom}; use std::fs::{remove_file, File, OpenOptions}; use std::io::{Read, Write}; +use std::path::PathBuf; +use std::time::SystemTime; -use rustc::ty::layout::{Size, Align}; +use rustc::ty::layout::{Size, Align, LayoutOf}; use crate::stacked_borrows::Tag; use crate::*; +use helpers::immty_from_uint_checked; +use shims::time::system_time_to_duration; #[derive(Debug)] pub struct FileHandle { @@ -98,7 +102,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let path = this.read_os_str_from_c_str(this.read_scalar(path_op)?.not_undef()?)?; - let fd = options.open(path).map(|file| { + let fd = options.open(&path).map(|file| { let mut fh = &mut this.machine.file_handler; fh.low += 1; fh.handles.insert(fh.low, FileHandle { file }).unwrap_none(); @@ -257,6 +261,181 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx this.try_unwrap_io_result(result) } + fn statx( + &mut self, + dirfd_op: OpTy<'tcx, Tag>, // Should be an `int` + pathname_op: OpTy<'tcx, Tag>, // Should be a `const char *` + flags_op: OpTy<'tcx, Tag>, // Should be an `int` + _mask_op: OpTy<'tcx, Tag>, // Should be an `unsigned int` + statxbuf_op: OpTy<'tcx, Tag> // Should be a `struct statx *` + ) -> InterpResult<'tcx, i32> { + let this = self.eval_context_mut(); + + this.check_no_isolation("statx")?; + + let statxbuf_scalar = this.read_scalar(statxbuf_op)?.not_undef()?; + let pathname_scalar = this.read_scalar(pathname_op)?.not_undef()?; + + // If the statxbuf or pathname pointers are null, the function fails with `EFAULT`. + if this.is_null(statxbuf_scalar)? || this.is_null(pathname_scalar)? { + let efault = this.eval_libc("EFAULT")?; + this.set_last_error(efault)?; + return Ok(-1); + } + + // Under normal circumstances, we would use `deref_operand(statxbuf_op)` to produce a + // proper `MemPlace` and then write the results of this function to it. However, the + // `syscall` function is untyped. This means that all the `statx` parameters are provided + // as `isize`s instead of having the proper types. Thus, we have to recover the layout of + // `statxbuf_op` by using the `libc::statx` struct type. + let statxbuf_place = { + // FIXME: This long path is required because `libc::statx` is an struct and also a + // function and `resolve_path` is returning the latter. + let statx_ty = this + .resolve_path(&["libc", "unix", "linux_like", "linux", "gnu", "statx"])? + .ty(*this.tcx); + let statxbuf_ty = this.tcx.mk_mut_ptr(statx_ty); + let statxbuf_layout = this.layout_of(statxbuf_ty)?; + let statxbuf_imm = ImmTy::from_scalar(statxbuf_scalar, statxbuf_layout); + this.ref_to_mplace(statxbuf_imm)? + }; + + let path: PathBuf = this.read_os_str_from_c_str(pathname_scalar)?.into(); + // `flags` should be a `c_int` but the `syscall` function provides an `isize`. + let flags: i32 = this + .read_scalar(flags_op)? + .to_machine_isize(&*this.tcx)? + .try_into() + .map_err(|e| err_unsup_format!( + "Failed to convert pointer sized operand to integer: {}", + e + ))?; + // `dirfd` should be a `c_int` but the `syscall` function provides an `isize`. + let dirfd: i32 = this + .read_scalar(dirfd_op)? + .to_machine_isize(&*this.tcx)? + .try_into() + .map_err(|e| err_unsup_format!( + "Failed to convert pointer sized operand to integer: {}", + e + ))?; + // we only support interpreting `path` as an absolute directory or as a directory relative + // to `dirfd` when the latter is `AT_FDCWD`. The behavior of `statx` with a relative path + // and a directory file descriptor other than `AT_FDCWD` is specified but it cannot be + // tested from `libstd`. If you found this error, please open an issue reporting it. + if !(path.is_absolute() || dirfd == this.eval_libc_i32("AT_FDCWD")?) + { + throw_unsup_format!( + "Using statx with a relative path and a file descriptor different from `AT_FDCWD` is not supported" + ) + } + + // the `_mask_op` paramter specifies the file information that the caller requested. + // However `statx` is allowed to return information that was not requested or to not + // return information that was requested. This `mask` represents the information we can + // actually provide in any host platform. + let mut mask = + this.eval_libc("STATX_TYPE")?.to_u32()? | this.eval_libc("STATX_SIZE")?.to_u32()?; + + // If the `AT_SYMLINK_NOFOLLOW` flag is set, we query the file's metadata without following + // symbolic links. + let metadata = if flags & this.eval_libc("AT_SYMLINK_NOFOLLOW")?.to_i32()? != 0 { + // FIXME: metadata for symlinks need testing. + std::fs::symlink_metadata(path) + } else { + std::fs::metadata(path) + }; + + let metadata = match metadata { + Ok(metadata) => metadata, + Err(e) => { + this.set_last_error_from_io_error(e)?; + return Ok(-1); + } + }; + + let file_type = metadata.file_type(); + + let mode_name = if file_type.is_file() { + "S_IFREG" + } else if file_type.is_dir() { + "S_IFDIR" + } else { + "S_IFLNK" + }; + + // The `mode` field specifies the type of the file and the permissions over the file for + // the owner, its group and other users. Given that we can only provide the file type + // without using platform specific methods, we only set the bits corresponding to the file + // type. This should be an `__u16` but `libc` provides its values as `u32`. + let mode: u16 = this.eval_libc(mode_name)? + .to_u32()? + .try_into() + .unwrap_or_else(|_| bug!("libc contains bad value for `{}` constant", mode_name)); + + let size = metadata.len(); + + let (access_sec, access_nsec) = extract_sec_and_nsec( + metadata.accessed(), + &mut mask, + this.eval_libc("STATX_ATIME")?.to_u32()? + )?; + + let (created_sec, created_nsec) = extract_sec_and_nsec( + metadata.created(), + &mut mask, + this.eval_libc("STATX_BTIME")?.to_u32()? + )?; + + let (modified_sec, modified_nsec) = extract_sec_and_nsec( + metadata.modified(), + &mut mask, + this.eval_libc("STATX_MTIME")?.to_u32()? + )?; + + let __u32_layout = this.libc_ty_layout("__u32")?; + let __u64_layout = this.libc_ty_layout("__u64")?; + let __u16_layout = this.libc_ty_layout("__u16")?; + + // Now we transform all this fields into `ImmTy`s and write them to `statxbuf`. We write a + // zero for the unavailable fields. + // FIXME: Provide more fields using platform specific methods. + let imms = [ + immty_from_uint_checked(mask, __u32_layout)?, // stx_mask + immty_from_uint_checked(0u128, __u32_layout)?, // stx_blksize + immty_from_uint_checked(0u128, __u64_layout)?, // stx_attributes + immty_from_uint_checked(0u128, __u32_layout)?, // stx_nlink + immty_from_uint_checked(0u128, __u32_layout)?, // stx_uid + immty_from_uint_checked(0u128, __u32_layout)?, // stx_gid + immty_from_uint_checked(mode, __u16_layout)?, // stx_mode + immty_from_uint_checked(0u128, __u16_layout)?, // statx padding + immty_from_uint_checked(0u128, __u64_layout)?, // stx_ino + immty_from_uint_checked(size, __u64_layout)?, // stx_size + immty_from_uint_checked(0u128, __u64_layout)?, // stx_blocks + immty_from_uint_checked(0u128, __u64_layout)?, // stx_attributes + immty_from_uint_checked(access_sec, __u64_layout)?, // stx_atime.tv_sec + immty_from_uint_checked(access_nsec, __u32_layout)?, // stx_atime.tv_nsec + immty_from_uint_checked(0u128, __u32_layout)?, // statx_timestamp padding + immty_from_uint_checked(created_sec, __u64_layout)?, // stx_btime.tv_sec + immty_from_uint_checked(created_nsec, __u32_layout)?, // stx_btime.tv_nsec + immty_from_uint_checked(0u128, __u32_layout)?, // statx_timestamp padding + immty_from_uint_checked(0u128, __u64_layout)?, // stx_ctime.tv_sec + immty_from_uint_checked(0u128, __u32_layout)?, // stx_ctime.tv_nsec + immty_from_uint_checked(0u128, __u32_layout)?, // statx_timestamp padding + immty_from_uint_checked(modified_sec, __u64_layout)?, // stx_mtime.tv_sec + immty_from_uint_checked(modified_nsec, __u32_layout)?, // stx_mtime.tv_nsec + immty_from_uint_checked(0u128, __u32_layout)?, // statx_timestamp padding + immty_from_uint_checked(0u128, __u64_layout)?, // stx_rdev_major + immty_from_uint_checked(0u128, __u64_layout)?, // stx_rdev_minor + immty_from_uint_checked(0u128, __u64_layout)?, // stx_dev_major + immty_from_uint_checked(0u128, __u64_layout)?, // stx_dev_minor + ]; + + this.write_packed_immediates(&statxbuf_place, &imms)?; + + Ok(0) + } + /// Function used when a handle is not found inside `FileHandler`. It returns `Ok(-1)`and sets /// the last OS error to `libc::EBADF` (invalid file descriptor). This function uses /// `T: From` instead of `i32` directly because some fs functions return different integer @@ -268,3 +447,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok((-1).into()) } } + +// Extracts the number of seconds and nanoseconds elapsed between `time` and the unix epoch, and +// then sets the `mask` bits determined by `flag` when `time` is Ok. If `time` is an error, it +// returns `(0, 0)` without setting any bits. +fn extract_sec_and_nsec<'tcx>(time: std::io::Result, mask: &mut u32, flag: u32) -> InterpResult<'tcx, (u64, u32)> { + if let Ok(time) = time { + let duration = system_time_to_duration(&time)?; + *mask |= flag; + Ok((duration.as_secs(), duration.subsec_nanos())) + } else { + Ok((0, 0)) + } +} diff --git a/src/shims/time.rs b/src/shims/time.rs index d75cb7bad384..da9ea07c4f25 100644 --- a/src/shims/time.rs +++ b/src/shims/time.rs @@ -1,34 +1,19 @@ use std::time::{Duration, SystemTime}; -use rustc::ty::layout::TyLayout; - use crate::stacked_borrows::Tag; use crate::*; +use helpers::immty_from_int_checked; -// Returns the time elapsed between now and the unix epoch as a `Duration` and the sign of the time -// interval +// Returns the time elapsed between now and the unix epoch as a `Duration`. fn get_time<'tcx>() -> InterpResult<'tcx, Duration> { - SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .map_err(|_| err_unsup_format!("Times before the Unix epoch are not supported").into()) + system_time_to_duration(&SystemTime::now()) } -fn int_to_immty_checked<'tcx>( - int: i128, - layout: TyLayout<'tcx>, -) -> InterpResult<'tcx, ImmTy<'tcx, Tag>> { - // If `int` does not fit in `size` bits, we error instead of letting - // `ImmTy::from_int` panic. - let size = layout.size; - let truncated = truncate(int as u128, size); - if sign_extend(truncated, size) as i128 != int { - throw_unsup_format!( - "Signed value {:#x} does not fit in {} bits", - int, - size.bits() - ) - } - Ok(ImmTy::from_int(int, layout)) +// Returns the time elapsed between the provided time and the unix epoch as a `Duration`. +pub fn system_time_to_duration<'tcx>(time: &SystemTime) -> InterpResult<'tcx, Duration> { + time + .duration_since(SystemTime::UNIX_EPOCH) + .map_err(|_| err_unsup_format!("Times before the Unix epoch are not supported").into()) } impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} @@ -57,8 +42,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let tv_nsec = duration.subsec_nanos() as i128; let imms = [ - int_to_immty_checked(tv_sec, this.libc_ty_layout("time_t")?)?, - int_to_immty_checked(tv_nsec, this.libc_ty_layout("c_long")?)?, + immty_from_int_checked(tv_sec, this.libc_ty_layout("time_t")?)?, + immty_from_int_checked(tv_nsec, this.libc_ty_layout("c_long")?)?, ]; this.write_packed_immediates(&tp, &imms)?; @@ -89,8 +74,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let tv_usec = duration.subsec_micros() as i128; let imms = [ - int_to_immty_checked(tv_sec, this.libc_ty_layout("time_t")?)?, - int_to_immty_checked(tv_usec, this.libc_ty_layout("suseconds_t")?)?, + immty_from_int_checked(tv_sec, this.libc_ty_layout("time_t")?)?, + immty_from_int_checked(tv_usec, this.libc_ty_layout("suseconds_t")?)?, ]; this.write_packed_immediates(&tv, &imms)?; diff --git a/tests/run-pass/fs.rs b/tests/run-pass/fs.rs index 7765e5b368e8..b8f9e3229af6 100644 --- a/tests/run-pass/fs.rs +++ b/tests/run-pass/fs.rs @@ -2,11 +2,32 @@ // compile-flags: -Zmiri-disable-isolation use std::fs::{File, remove_file}; -use std::io::{Read, Write, ErrorKind}; +use std::io::{Read, Write, ErrorKind, Result}; +use std::path::{PathBuf, Path}; + +#[cfg(target_os = "linux")] +fn test_metadata(bytes: &[u8], path: &Path) -> Result<()> { + // Test that the file metadata is correct. + let metadata = path.metadata()?; + // `path` should point to a file. + assert!(metadata.is_file()); + // The size of the file must be equal to the number of written bytes. + assert_eq!(bytes.len() as u64, metadata.len()); + Ok(()) +} + +// FIXME: Implement stat64 for macos. +#[cfg(not(target_os = "linux"))] +fn test_metadata(_bytes: &[u8], _path: &Path) -> Result<()> { + Ok(()) +} fn main() { - let path = std::env::temp_dir().join("miri_test_fs.txt"); + let tmp = std::env::temp_dir(); + let filename = PathBuf::from("miri_test_fs.txt"); + let path = tmp.join(&filename); let bytes = b"Hello, World!\n"; + // Test creating, writing and closing a file (closing is tested when `file` is dropped). let mut file = File::create(&path).unwrap(); // Writing 0 bytes should not change the file contents. @@ -21,7 +42,14 @@ fn main() { // Reading until EOF should get the whole text. file.read_to_end(&mut contents).unwrap(); assert_eq!(bytes, contents.as_slice()); - // Removing file should succeed + + // Test that metadata of an absolute path is correct. + test_metadata(bytes, &path).unwrap(); + // Test that metadata of a relative path is correct. + std::env::set_current_dir(tmp).unwrap(); + test_metadata(bytes, &filename).unwrap(); + + // Removing file should succeed. remove_file(&path).unwrap(); // The two following tests also check that the `__errno_location()` shim is working properly. @@ -29,4 +57,8 @@ fn main() { assert_eq!(ErrorKind::NotFound, File::open(&path).unwrap_err().kind()); // Removing a non-existing file should fail with a "not found" error. assert_eq!(ErrorKind::NotFound, remove_file(&path).unwrap_err().kind()); + // Reading the metadata of a non-existing file should fail with a "not found" error. + if cfg!(target_os = "linux") { // FIXME: Implement stat64 for macos. + assert_eq!(ErrorKind::NotFound, test_metadata(bytes, &path).unwrap_err().kind()); + } }