Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/shims/unix/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,41 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// fadvise is only informational, we can ignore it.
this.write_null(dest)?;
}

// only macos doesn't support `posix_fallocate`
"posix_fallocate" if &*this.tcx.sess.target.os != "macos" => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the correct way, from what I could find online, only macos doesn't implement posix_fallocate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that is the case then the test should also be for all OSes except macos.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, please check pipe2 for how we make a shim available only on some OSes. Please do not use a match guard.

let [fd, offset, len] = this.check_shim_sig(
shim_sig!(extern "C" fn(i32, libc::off_t, libc::off_t) -> i32),
link_name,
abi,
args,
)?;

let fd = this.read_scalar(fd)?.to_i32()?;
let offset = this.read_scalar(offset)?.to_int(offset.layout.size)?;
let len = this.read_scalar(len)?.to_int(len.layout.size)?;

let result = this.posix_fallocate(fd, offset, len)?;
this.write_scalar(result, dest)?;
}

// only macos doesn't support `posix_fallocate`
"posix_fallocate64" if &*this.tcx.sess.target.os != "macos" => {
let [fd, offset, len] = this.check_shim_sig(
shim_sig!(extern "C" fn(i32, libc::off64_t, libc::off64_t) -> i32),
link_name,
abi,
args,
)?;

let fd = this.read_scalar(fd)?.to_i32()?;
let offset = this.read_scalar(offset)?.to_int(offset.layout.size)?;
let len = this.read_scalar(len)?.to_int(len.layout.size)?;

let result = this.posix_fallocate(fd, offset, len)?;
this.write_scalar(result, dest)?;
}

"realpath" => {
let [path, resolved_path] = this.check_shim_sig(
shim_sig!(extern "C" fn(*const _, *mut _) -> *mut _),
Expand Down
59 changes: 59 additions & 0 deletions src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1197,6 +1197,65 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
}
}

fn posix_fallocate(
&mut self,
fd_num: i32,
offset: i128,
len: i128,
) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();

// According to the man page of `possix_fallocate`, it returns the error code instead
// of setting `errno`.
let ebadf = Scalar::from_i32(this.eval_libc_i32("EBADF"));
let einval = Scalar::from_i32(this.eval_libc_i32("EINVAL"));
let enodev = Scalar::from_i32(this.eval_libc_i32("ENODEV"));
Comment on lines +1210 to +1212
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to just inline these where they are needed; the error path shouldn't take so much space on the happy path.

Also, you can use eval_libc instead of Scalar::from_i32(eval_libc_i32).


// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`posix_fallocate`", reject_with)?;
// Set error code as "EBADF" (bad fd)
return interp_ok(ebadf);
}

let Some(fd) = this.machine.fds.get(fd_num) else {
return interp_ok(ebadf);
};

let file = match fd.downcast::<FileHandle>() {
Some(file_handle) => file_handle,
// Man page specifies to return ENODEV if `fd` is not a regular file.
None => return interp_ok(enodev),
};

// EINVAL is returned when: "offset was less than 0, or len was less than or equal to 0".
if offset < 0 || len <= 0 {
return interp_ok(einval);
}

if file.writable {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this more like

if !file.writable { return error; }

to avoid rightwards drift

let current_size = match file.file.metadata() {
Ok(metadata) => metadata.len(),
Err(err) => return this.io_error_to_errnum(err),
};
let new_size = match offset.checked_add(len) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, if this operation overflows we should return EFBIG.

Some(size) => size.try_into().unwrap(), // We just checked negative `offset` and `len`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this code so it is clear what type we are converting this into.

Also the comment does not explain why the size would fit into the target type.

None => return interp_ok(einval),
};
// `posix_fallocate` only specifies increasing the file size.
if current_size < new_size {
let result = file.file.set_len(new_size);
let result = this.try_unwrap_io_result(result.map(|_| 0i32))?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can set the errno. Didn't you say the function does not do that?

interp_ok(Scalar::from_i32(result))
} else {
interp_ok(Scalar::from_i32(0))
}
} else {
// The file is not writable.
interp_ok(ebadf)
}
}

fn fsync(&mut self, fd_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> {
// On macOS, `fsync` (unlike `fcntl(F_FULLFSYNC)`) does not wait for the
// underlying disk to finish writing. In the interest of host compatibility,
Expand Down
72 changes: 72 additions & 0 deletions tests/pass-dep/libc/libc-fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ fn main() {
#[cfg(target_os = "linux")]
test_posix_fadvise();
#[cfg(target_os = "linux")]
test_posix_fallocate::<libc::off_t>(libc::posix_fallocate);
#[cfg(target_os = "linux")]
test_posix_fallocate::<libc::off64_t>(libc::posix_fallocate64);
#[cfg(target_os = "linux")]
test_sync_file_range();
test_isatty();
test_read_and_uninit();
Expand Down Expand Up @@ -335,6 +339,74 @@ fn test_posix_fadvise() {
assert_eq!(result, 0);
}

#[cfg(target_os = "linux")]
fn test_posix_fallocate<T: From<i32>>(
posix_fallocate: unsafe extern "C" fn(fd: libc::c_int, offset: T, len: T) -> libc::c_int,
) {
// libc::off_t is i32 in target i686-unknown-linux-gnu
// https://docs.rs/libc/latest/i686-unknown-linux-gnu/libc/type.off_t.html

let test_errors = || {
// invalid fd
let ret = unsafe { posix_fallocate(42, T::from(0), T::from(10)) };
assert_eq!(ret, libc::EBADF);

let path = utils::prepare("miri_test_libc_possix_fallocate_errors.txt");
let file = File::create(&path).unwrap();

// invalid offset
let ret = unsafe { posix_fallocate(file.as_raw_fd(), T::from(-10), T::from(10)) };
assert_eq!(ret, libc::EINVAL);

// invalid len
let ret = unsafe { posix_fallocate(file.as_raw_fd(), T::from(0), T::from(-10)) };
assert_eq!(ret, libc::EINVAL);

// fd not writable
let c_path = CString::new(path.as_os_str().as_bytes()).expect("CString::new failed");
let fd = unsafe { libc::open(c_path.as_ptr(), libc::O_RDONLY) };
let ret = unsafe { posix_fallocate(fd, T::from(0), T::from(10)) };
assert_eq!(ret, libc::EBADF);
};

let test = || {
let bytes = b"hello";
let path = utils::prepare("miri_test_libc_fs_ftruncate.txt");
let mut file = File::create(&path).unwrap();
file.write_all(bytes).unwrap();
file.sync_all().unwrap();
assert_eq!(file.metadata().unwrap().len(), 5);

let c_path = CString::new(path.as_os_str().as_bytes()).expect("CString::new failed");
let fd = unsafe { libc::open(c_path.as_ptr(), libc::O_RDWR) };

// Allocate to a bigger size from offset 0
let mut res = unsafe { posix_fallocate(fd, T::from(0), T::from(10)) };
assert_eq!(res, 0);
assert_eq!(file.metadata().unwrap().len(), 10);

// Write after allocation
file.write(b"dup").unwrap();
file.sync_all().unwrap();
assert_eq!(file.metadata().unwrap().len(), 10);

// Can't truncate to a smaller size with possix_fallocate
res = unsafe { posix_fallocate(fd, T::from(0), T::from(3)) };
assert_eq!(res, 0);
assert_eq!(file.metadata().unwrap().len(), 10);

// Allocate from offset
res = unsafe { posix_fallocate(fd, T::from(7), T::from(7)) };
assert_eq!(res, 0);
assert_eq!(file.metadata().unwrap().len(), 14);

remove_file(&path).unwrap();
};

test_errors();
test();
}

#[cfg(target_os = "linux")]
fn test_sync_file_range() {
use std::io::Write;
Expand Down