Implement dup and close for stdin/stdout/stderr

Support F_DUPFD on stdin/stdout/stderr

Enable `close`-ing stdin/stdout/stderr

For `dup`, check if FD is `File` first

If not, clone the appropriate standard IO stream

Merge POSIX `close` and `dup` tests into same module

Also, add assertion that `write` on a closed FD returns an error.

Add `dup` as FileDescriptor trait fn

Also:
- Fix `close` so it drops `self` instead of reference to it
- Remove FD clamping in insert_fd_with_min_fd, since FDs 0-2 can be
closed

Fix fs_libc tests

Make error message when closing stdin/out/err more specific

Return io::Result from `FileDescriptor::dup`

Change error message when closing stdin/out/err

Refactor `FileDescriptor::dup` impl for `FileHandle`

Remove empty line
This commit is contained in:
Samrat Man Singh 2020-08-15 14:15:31 +05:30
parent 604a674ea3
commit 563fb8e43d
5 changed files with 109 additions and 42 deletions

View File

@ -28,6 +28,9 @@ trait FileDescriptor : std::fmt::Debug {
fn read<'tcx>(&mut self, communicate_allowed: bool, bytes: &mut [u8]) -> InterpResult<'tcx, io::Result<usize>>;
fn write<'tcx>(&mut self, communicate_allowed: bool, bytes: &[u8]) -> InterpResult<'tcx, io::Result<usize>>;
fn seek<'tcx>(&mut self, communicate_allowed: bool, offset: SeekFrom) -> InterpResult<'tcx, io::Result<u64>>;
fn close<'tcx>(self: Box<Self>, _communicate_allowed: bool) -> InterpResult<'tcx, io::Result<i32>>;
fn dup<'tcx>(&mut self) -> io::Result<Box<dyn FileDescriptor>>;
}
impl FileDescriptor for FileHandle {
@ -49,6 +52,34 @@ fn seek<'tcx>(&mut self, communicate_allowed: bool, offset: SeekFrom) -> InterpR
assert!(communicate_allowed, "isolation should have prevented even opening a file");
Ok(self.file.seek(offset))
}
fn close<'tcx>(self: Box<Self>, communicate_allowed: bool) -> InterpResult<'tcx, io::Result<i32>> {
assert!(communicate_allowed, "isolation should have prevented even opening a file");
// We sync the file if it was opened in a mode different than read-only.
if self.writable {
// `File::sync_all` does the checks that are done when closing a file. We do this to
// to handle possible errors correctly.
let result = self.file.sync_all().map(|_| 0i32);
// Now we actually close the file.
drop(self);
// And return the result.
Ok(result)
} else {
// We drop the file, this closes it but ignores any errors
// produced when closing it. This is done because
// `File::sync_all` cannot be done over files like
// `/dev/urandom` which are read-only. Check
// https://github.com/rust-lang/miri/issues/999#issuecomment-568920439
// for a deeper discussion.
drop(self);
Ok(Ok(0))
}
}
fn dup<'tcx>(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
let duplicated = self.file.try_clone()?;
Ok(Box::new(FileHandle { file: duplicated, writable: self.writable }))
}
}
impl FileDescriptor for io::Stdin {
@ -71,6 +102,14 @@ fn write<'tcx>(&mut self, _communicate_allowed: bool, _bytes: &[u8]) -> InterpRe
fn seek<'tcx>(&mut self, _communicate_allowed: bool, _offset: SeekFrom) -> InterpResult<'tcx, io::Result<u64>> {
throw_unsup_format!("cannot seek on stdin");
}
fn close<'tcx>(self: Box<Self>, _communicate_allowed: bool) -> InterpResult<'tcx, io::Result<i32>> {
throw_unsup_format!("stdin cannot be closed");
}
fn dup<'tcx>(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
Ok(Box::new(io::stdin()))
}
}
impl FileDescriptor for io::Stdout {
@ -98,6 +137,14 @@ fn write<'tcx>(&mut self, _communicate_allowed: bool, bytes: &[u8]) -> InterpRes
fn seek<'tcx>(&mut self, _communicate_allowed: bool, _offset: SeekFrom) -> InterpResult<'tcx, io::Result<u64>> {
throw_unsup_format!("cannot seek on stdout");
}
fn close<'tcx>(self: Box<Self>, _communicate_allowed: bool) -> InterpResult<'tcx, io::Result<i32>> {
throw_unsup_format!("stdout cannot be closed");
}
fn dup<'tcx>(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
Ok(Box::new(io::stdout()))
}
}
impl FileDescriptor for io::Stderr {
@ -118,6 +165,14 @@ fn write<'tcx>(&mut self, _communicate_allowed: bool, bytes: &[u8]) -> InterpRes
fn seek<'tcx>(&mut self, _communicate_allowed: bool, _offset: SeekFrom) -> InterpResult<'tcx, io::Result<u64>> {
throw_unsup_format!("cannot seek on stderr");
}
fn close<'tcx>(self: Box<Self>, _communicate_allowed: bool) -> InterpResult<'tcx, io::Result<i32>> {
throw_unsup_format!("stderr cannot be closed");
}
fn dup<'tcx>(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
Ok(Box::new(io::stderr()))
}
}
#[derive(Debug)]
@ -137,18 +192,12 @@ fn default() -> Self {
}
}
// fd numbers 0, 1, and 2 are reserved for stdin, stdout, and stderr
const MIN_NORMAL_FILE_FD: i32 = 3;
impl<'tcx> FileHandler {
fn insert_fd(&mut self, file_handle: FileHandle) -> i32 {
fn insert_fd(&mut self, file_handle: Box<dyn FileDescriptor>) -> i32 {
self.insert_fd_with_min_fd(file_handle, 0)
}
fn insert_fd_with_min_fd(&mut self, file_handle: FileHandle, min_fd: i32) -> i32 {
let min_fd = std::cmp::max(min_fd, MIN_NORMAL_FILE_FD);
fn insert_fd_with_min_fd(&mut self, file_handle: Box<dyn FileDescriptor>, min_fd: i32) -> i32 {
// Find the lowest unused FD, starting from min_fd. If the first such unused FD is in
// between used FDs, the find_map combinator will return it. If the first such unused FD
// is after all other used FDs, the find_map combinator will return None, and we will use
@ -173,7 +222,7 @@ fn insert_fd_with_min_fd(&mut self, file_handle: FileHandle, min_fd: i32) -> i32
self.handles.last_key_value().map(|(fd, _)| fd.checked_add(1).unwrap()).unwrap_or(min_fd)
});
self.handles.insert(new_fd, Box::new(file_handle)).unwrap_none();
self.handles.insert(new_fd, file_handle).unwrap_none();
new_fd
}
}
@ -449,7 +498,7 @@ fn open(
let fd = options.open(&path).map(|file| {
let fh = &mut this.machine.file_handler;
fh.insert_fd(FileHandle { file, writable })
fh.insert_fd(Box::new(FileHandle { file, writable }))
});
this.try_unwrap_io_result(fd)
@ -489,22 +538,22 @@ fn fcntl(
// thus they can share the same implementation here.
let &[_, _, start] = check_arg_count(args)?;
let start = this.read_scalar(start)?.to_i32()?;
if fd < MIN_NORMAL_FILE_FD {
throw_unsup_format!("duplicating file descriptors for stdin, stdout, or stderr is not supported")
}
let fh = &mut this.machine.file_handler;
let (file_result, writable) = match fh.handles.get(&fd) {
match fh.handles.get_mut(&fd) {
Some(file_descriptor) => {
// FIXME: Support "dup" for all FDs(stdin, etc)
let FileHandle { file, writable } = file_descriptor.as_file_handle()?;
(file.try_clone(), *writable)
let dup_result = file_descriptor.dup();
match dup_result {
Ok(dup_fd) => Ok(fh.insert_fd_with_min_fd(dup_fd, start)),
Err(e) => {
this.set_last_error_from_io_error(e)?;
Ok(-1)
}
}
},
None => return this.handle_not_found(),
};
let fd_result = file_result.map(|duplicated| {
fh.insert_fd_with_min_fd(FileHandle { file: duplicated, writable }, start)
});
this.try_unwrap_io_result(fd_result)
}
} else if this.tcx.sess.target.target.target_os == "macos"
&& cmd == this.eval_libc_i32("F_FULLFSYNC")?
{
@ -530,26 +579,8 @@ fn close(&mut self, fd_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
let fd = this.read_scalar(fd_op)?.to_i32()?;
if let Some(file_descriptor) = this.machine.file_handler.handles.remove(&fd) {
// FIXME: Support `close` for all FDs(stdin, etc)
let FileHandle { file, writable } = file_descriptor.as_file_handle()?;
// We sync the file if it was opened in a mode different than read-only.
if *writable {
// `File::sync_all` does the checks that are done when closing a file. We do this to
// to handle possible errors correctly.
let result = this.try_unwrap_io_result(file.sync_all().map(|_| 0i32));
// Now we actually close the file.
drop(file);
// And return the result.
result
} else {
// We drop the file, this closes it but ignores any errors produced when closing
// it. This is done because `File::sync_all` cannot be done over files like
// `/dev/urandom` which are read-only. Check
// https://github.com/rust-lang/miri/issues/999#issuecomment-568920439 for a deeper
// discussion.
drop(file);
Ok(0)
}
let result = file_descriptor.close(this.machine.communicate)?;
this.try_unwrap_io_result(result)
} else {
this.handle_not_found()
}

View File

@ -0,0 +1,14 @@
// ignore-windows: No libc on Windows
// compile-flags: -Zmiri-disable-isolation
// FIXME: standard handles cannot be closed (https://github.com/rust-lang/rust/issues/40032)
#![feature(rustc_private)]
extern crate libc;
fn main() {
unsafe {
libc::close(1); //~ ERROR stdout cannot be closed
}
}

20
tests/run-pass/fs_libc.rs Normal file
View File

@ -0,0 +1,20 @@
// ignore-windows
// compile-flags: -Zmiri-disable-isolation
#![feature(rustc_private)]
extern crate libc;
fn main() {
dup_stdout_stderr_test();
}
fn dup_stdout_stderr_test() {
let bytes = b"hello dup fd\n";
unsafe {
let new_stdout = libc::fcntl(1, libc::F_DUPFD, 0);
let new_stderr = libc::fcntl(2, libc::F_DUPFD, 0);
libc::write(new_stdout, bytes.as_ptr() as *const libc::c_void, bytes.len());
libc::write(new_stderr, bytes.as_ptr() as *const libc::c_void, bytes.len());
}
}

View File

@ -0,0 +1 @@
hello dup fd

View File

@ -0,0 +1 @@
hello dup fd