Skip to content

Commit

Permalink
changes from review.
Browse files Browse the repository at this point in the history
  • Loading branch information
devnexen committed Nov 21, 2023
1 parent 28adab5 commit 97446f8
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 118 deletions.
154 changes: 71 additions & 83 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ case $HOST_TARGET in
MIRI_TEST_TARGET=aarch64-unknown-linux-gnu run_tests
MIRI_TEST_TARGET=aarch64-apple-darwin run_tests
MIRI_TEST_TARGET=i686-pc-windows-gnu run_tests
MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple pthread-threadname libc-getentropy libc-getrandom libc-misc libc-fs atomic env align
MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple pthread-threadname libc-getentropy libc-getrandom libc-misc libc-fs fs atomic env align
MIRI_TEST_TARGET=aarch64-linux-android run_tests_minimal hello integer vec panic/panic
MIRI_TEST_TARGET=wasm32-wasi run_tests_minimal no_std integer strings wasm
MIRI_TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std integer strings wasm
Expand Down
11 changes: 11 additions & 0 deletions src/shims/unix/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
"lseek64" => {
let [fd, offset, whence] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let fd = this.read_scalar(fd)?.to_i32()?;
let offset = this.read_scalar(offset)?.to_i64()?;
let whence = this.read_scalar(whence)?.to_i32()?;
let result = this.lseek64(fd, offset, whence)?;
this.write_scalar(result, dest)?;
}
"lseek" => {
let [fd, offset, whence] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let fd = this.read_scalar(fd)?.to_i32()?;
let offset = this.read_scalar(offset)?.to_i64()?;
let whence = this.read_scalar(whence)?.to_i32()?;
let result = this.lseek64(fd, offset, whence)?;
this.write_scalar(result, dest)?;
}
Expand Down
6 changes: 6 additions & 0 deletions src/shims/unix/freebsd/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let result = this.macos_fbsd_fstat(fd, buf)?;
this.write_scalar(result, dest)?;
}
"readdir_r" | "readdir_r@FBSD_1.0" => {
let [dirp, entry, result] =
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let result = this.macos_fbsd_readdir_r(dirp, entry, result)?;
this.write_scalar(result, dest)?;
}

// errno
"__error" => {
Expand Down
62 changes: 38 additions & 24 deletions src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -827,18 +827,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

fn lseek64(
&mut self,
fd_op: &OpTy<'tcx, Provenance>,
offset_op: &OpTy<'tcx, Provenance>,
whence_op: &OpTy<'tcx, Provenance>,
fd: i32,
offset: i64,
whence: i32,
) -> InterpResult<'tcx, Scalar<Provenance>> {
let this = self.eval_context_mut();

// Isolation check is done via `FileDescriptor` trait.

let fd = this.read_scalar(fd_op)?.to_i32()?;
let offset = this.read_scalar(offset_op)?.to_i64()?;
let whence = this.read_scalar(whence_op)?.to_i32()?;

let seek_from = if whence == this.eval_libc_i32("SEEK_SET") {
SeekFrom::Start(u64::try_from(offset).unwrap())
} else if whence == this.eval_libc_i32("SEEK_CUR") {
Expand Down Expand Up @@ -919,7 +915,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let this = self.eval_context_mut();

if !matches!(&*this.tcx.sess.target.os, "macos" | "freebsd") {
throw_unsup_format!("macos_fbsd_stat not meant for os {}", this.tcx.sess.target.os);
panic!("macos_fbsd_stat not meant for os {}", this.tcx.sess.target.os);
}

let path_scalar = this.read_pointer(path_op)?;
Expand Down Expand Up @@ -951,7 +947,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let this = self.eval_context_mut();

if !matches!(&*this.tcx.sess.target.os, "macos" | "freebsd") {
throw_unsup_format!("macos_fbsd_lstat not meant for os {}", this.tcx.sess.target.os);
panic!("macos_fbsd_lstat not meant for os {}", this.tcx.sess.target.os);
}

let path_scalar = this.read_pointer(path_op)?;
Expand Down Expand Up @@ -981,7 +977,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let this = self.eval_context_mut();

if !matches!(&*this.tcx.sess.target.os, "macos" | "freebsd") {
throw_unsup_format!("macos_fbsd_fstat not meant for os {}", this.tcx.sess.target.os);
panic!("macos_fbsd_fstat not meant for os {}", this.tcx.sess.target.os);
}

let fd = this.read_scalar(fd_op)?.to_i32()?;
Expand Down Expand Up @@ -1221,7 +1217,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let this = self.eval_context_mut();

#[cfg_attr(not(unix), allow(unused_variables))]
let mode = if this.tcx.sess.target.os == "macos" {
let mode = if matches!(&*this.tcx.sess.target.os, "macos" | "freebsd") {
u32::from(this.read_scalar(mode_op)?.to_u16()?)
} else {
this.read_scalar(mode_op)?.to_u32()?
Expand Down Expand Up @@ -1393,15 +1389,20 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
Ok(Scalar::from_maybe_pointer(entry, this))
}

fn macos_readdir_r(
fn macos_fbsd_readdir_r(
&mut self,
dirp_op: &OpTy<'tcx, Provenance>,
entry_op: &OpTy<'tcx, Provenance>,
result_op: &OpTy<'tcx, Provenance>,
) -> InterpResult<'tcx, Scalar<Provenance>> {
let this = self.eval_context_mut();

this.assert_target_os("macos", "readdir_r");
if !matches!(&*this.tcx.sess.target.os, "macos" | "freebsd") {
panic!("macos_fbsd_readdir_r not meant for os {}", this.tcx.sess.target.os);
}

// FIXME?: oddily it seems to pick up the old libc crate's freebsd11 definition instead of freebsd13, should be 6
let name_offset = if this.tcx.sess.target.os == "macos" { 5 } else { 4 };

let dirp = this.read_target_usize(dirp_op)?;

Expand Down Expand Up @@ -1432,7 +1433,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// }

let entry_place = this.deref_pointer_as(entry_op, this.libc_ty_layout("dirent"))?;
let name_place = this.project_field(&entry_place, 5)?;
let name_place = this.project_field(&entry_place, name_offset)?;

let file_name = dir_entry.file_name(); // not a Path as there are no separators!
let (name_fits, file_name_buf_len) = this.write_os_str_to_c_str(
Expand All @@ -1456,16 +1457,29 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

let file_type = this.file_type_to_d_type(dir_entry.file_type())?;

this.write_int_fields_named(
&[
("d_ino", ino.into()),
("d_seekoff", 0),
("d_reclen", 0),
("d_namlen", file_name_len.into()),
("d_type", file_type.into()),
],
&entry_place,
)?;
if this.tcx.sess.target.os == "macos" {
this.write_int_fields_named(
&[
("d_ino", ino.into()),
("d_seekoff", 0),
("d_reclen", 0),
("d_namlen", file_name_len.into()),
("d_type", file_type.into()),
],
&entry_place,
)?;
} else {
// FIXME?: same remark as above but it works..
this.write_int_fields_named(
&[
("d_fileno", ino.into()),
("d_reclen", 0),
("d_type", file_type.into()),
("d_namlen", file_name_len.into()),
],
&entry_place,
)?;
}

let result_place = this.deref_pointer(result_op)?;
this.write_scalar(this.read_scalar(entry_op)?, &result_place)?;
Expand Down
9 changes: 1 addition & 8 deletions src/shims/unix/macos/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
"readdir_r" | "readdir_r$INODE64" => {
let [dirp, entry, result] =
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let result = this.macos_readdir_r(dirp, entry, result)?;
this.write_scalar(result, dest)?;
}
"lseek" => {
let [fd, offset, whence] =
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
// macOS is 64bit-only, so this is lseek64
let result = this.lseek64(fd, offset, whence)?;
let result = this.macos_fbsd_readdir_r(dirp, entry, result)?;
this.write_scalar(result, dest)?;
}
"realpath$DARWIN_EXTSN" => {
Expand Down
1 change: 1 addition & 0 deletions tests/pass/shims/fs-with-isolation.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//@ignore-target-windows: File handling is not implemented yet
//@ignore-target-freebsd
//@compile-flags: -Zmiri-isolation-error=warn-nobacktrace
//@normalize-stderr-test: "(stat(x)?)" -> "$$STAT"

Expand Down
9 changes: 7 additions & 2 deletions tests/pass/shims/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@
#![feature(io_error_more)]
#![feature(io_error_uncategorized)]

#[cfg(not(target_os = "freebsd"))]
use std::collections::HashMap;
#[cfg(not(target_os = "freebsd"))]
use std::ffi::OsString;
use std::fs::{
canonicalize, create_dir, read_dir, read_link, remove_dir, remove_dir_all, remove_file, rename,
File, OpenOptions,
canonicalize, create_dir, read_link, remove_dir_all, remove_file, rename, File, OpenOptions,
};
#[cfg(not(target_os = "freebsd"))]
use std::fs::{read_dir, remove_dir};
use std::io::{Error, ErrorKind, IsTerminal, Read, Result, Seek, SeekFrom, Write};
use std::path::{Path, PathBuf};

Expand All @@ -28,6 +31,7 @@ fn main() {
test_symlink();
test_errors();
test_rename();
#[cfg(not(target_os = "freebsd"))]
test_directory();
test_canonicalize();
test_from_raw_os_error();
Expand Down Expand Up @@ -296,6 +300,7 @@ fn test_canonicalize() {
remove_dir_all(&dir_path).unwrap();
}

#[cfg(not(target_os = "freebsd"))]
fn test_directory() {
let dir_path = prepare_dir("miri_test_fs_dir");
// Creating a directory should succeed.
Expand Down

0 comments on commit 97446f8

Please sign in to comment.