use fd_num for file descriptors, so we can use fd for file description

This commit is contained in:
Ralf Jung 2024-09-09 16:01:47 +02:00
parent 4db9c01f8b
commit d4eeb31c2e
3 changed files with 83 additions and 92 deletions

View File

@ -303,7 +303,7 @@ pub struct FdTable {
impl VisitProvenance for FdTable {
fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {
// All our FileDescriptionRef do not have any tags.
// All our FileDescription instances do not have any tags.
}
}
@ -337,18 +337,18 @@ impl FdTable {
}
pub fn insert(&mut self, fd_ref: FileDescriptionRef) -> i32 {
self.insert_ref_with_min_fd(fd_ref, 0)
self.insert_with_min_num(fd_ref, 0)
}
/// Insert a file description, giving it a file descriptor that is at least `min_fd`.
fn insert_ref_with_min_fd(&mut self, file_handle: FileDescriptionRef, min_fd: i32) -> i32 {
/// Insert a file description, giving it a file descriptor that is at least `min_fd_num`.
fn insert_with_min_num(&mut self, file_handle: FileDescriptionRef, min_fd_num: 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
// the FD following the greatest FD thus far.
let candidate_new_fd =
self.fds.range(min_fd..).zip(min_fd..).find_map(|((fd, _fh), counter)| {
if *fd != counter {
self.fds.range(min_fd_num..).zip(min_fd_num..).find_map(|((fd_num, _fd), counter)| {
if *fd_num != counter {
// There was a gap in the fds stored, return the first unused one
// (note that this relies on BTreeMap iterating in key order)
Some(counter)
@ -357,61 +357,61 @@ impl FdTable {
None
}
});
let new_fd = candidate_new_fd.unwrap_or_else(|| {
let new_fd_num = candidate_new_fd.unwrap_or_else(|| {
// find_map ran out of BTreeMap entries before finding a free fd, use one plus the
// maximum fd in the map
self.fds.last_key_value().map(|(fd, _)| fd.strict_add(1)).unwrap_or(min_fd)
self.fds.last_key_value().map(|(fd_num, _)| fd_num.strict_add(1)).unwrap_or(min_fd_num)
});
self.fds.try_insert(new_fd, file_handle).unwrap();
new_fd
self.fds.try_insert(new_fd_num, file_handle).unwrap();
new_fd_num
}
pub fn get(&self, fd: i32) -> Option<FileDescriptionRef> {
let fd = self.fds.get(&fd)?;
pub fn get(&self, fd_num: i32) -> Option<FileDescriptionRef> {
let fd = self.fds.get(&fd_num)?;
Some(fd.clone())
}
pub fn remove(&mut self, fd: i32) -> Option<FileDescriptionRef> {
self.fds.remove(&fd)
pub fn remove(&mut self, fd_num: i32) -> Option<FileDescriptionRef> {
self.fds.remove(&fd_num)
}
pub fn is_fd(&self, fd: i32) -> bool {
self.fds.contains_key(&fd)
pub fn is_fd_num(&self, fd_num: i32) -> bool {
self.fds.contains_key(&fd_num)
}
}
impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
fn dup(&mut self, old_fd: i32) -> InterpResult<'tcx, Scalar> {
fn dup(&mut self, old_fd_num: i32) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();
let Some(dup_fd) = this.machine.fds.get(old_fd) else {
let Some(fd) = this.machine.fds.get(old_fd_num) else {
return Ok(Scalar::from_i32(this.fd_not_found()?));
};
Ok(Scalar::from_i32(this.machine.fds.insert_ref_with_min_fd(dup_fd, 0)))
Ok(Scalar::from_i32(this.machine.fds.insert(fd)))
}
fn dup2(&mut self, old_fd: i32, new_fd: i32) -> InterpResult<'tcx, Scalar> {
fn dup2(&mut self, old_fd_num: i32, new_fd_num: i32) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();
let Some(dup_fd) = this.machine.fds.get(old_fd) else {
let Some(fd) = this.machine.fds.get(old_fd_num) else {
return Ok(Scalar::from_i32(this.fd_not_found()?));
};
if new_fd != old_fd {
if new_fd_num != old_fd_num {
// Close new_fd if it is previously opened.
// If old_fd and new_fd point to the same description, then `dup_fd` ensures we keep the underlying file description alive.
if let Some(file_description) = this.machine.fds.fds.insert(new_fd, dup_fd) {
if let Some(old_new_fd) = this.machine.fds.fds.insert(new_fd_num, fd) {
// Ignore close error (not interpreter's) according to dup2() doc.
file_description.close(this.machine.communicate(), this)?.ok();
old_new_fd.close(this.machine.communicate(), this)?.ok();
}
}
Ok(Scalar::from_i32(new_fd))
Ok(Scalar::from_i32(new_fd_num))
}
fn flock(&mut self, fd: i32, op: i32) -> InterpResult<'tcx, Scalar> {
fn flock(&mut self, fd_num: i32, op: i32) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();
let Some(fd_ref) = this.machine.fds.get(fd) else {
let Some(fd) = this.machine.fds.get(fd_num) else {
return Ok(Scalar::from_i32(this.fd_not_found()?));
};
@ -436,8 +436,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
throw_unsup_format!("unsupported flags {:#x}", op);
};
let result = fd_ref.flock(this.machine.communicate(), parsed_op)?;
drop(fd_ref);
let result = fd.flock(this.machine.communicate(), parsed_op)?;
drop(fd);
// return `0` if flock is successful
let result = result.map(|()| 0i32);
Ok(Scalar::from_i32(this.try_unwrap_io_result(result)?))
@ -452,7 +452,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
args.len()
);
}
let fd = this.read_scalar(&args[0])?.to_i32()?;
let fd_num = this.read_scalar(&args[0])?.to_i32()?;
let cmd = this.read_scalar(&args[1])?.to_i32()?;
// We only support getting the flags for a descriptor.
@ -461,7 +461,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// `FD_CLOEXEC` value without checking if the flag is set for the file because `std`
// always sets this flag when opening a file. However we still need to check that the
// file itself is open.
Ok(Scalar::from_i32(if this.machine.fds.is_fd(fd) {
Ok(Scalar::from_i32(if this.machine.fds.is_fd_num(fd_num) {
this.eval_libc_i32("FD_CLOEXEC")
} else {
this.fd_not_found()?
@ -481,9 +481,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
}
let start = this.read_scalar(&args[2])?.to_i32()?;
match this.machine.fds.get(fd) {
Some(dup_fd) =>
Ok(Scalar::from_i32(this.machine.fds.insert_ref_with_min_fd(dup_fd, start))),
match this.machine.fds.get(fd_num) {
Some(fd) => Ok(Scalar::from_i32(this.machine.fds.insert_with_min_num(fd, start))),
None => Ok(Scalar::from_i32(this.fd_not_found()?)),
}
} else if this.tcx.sess.target.os == "macos" && cmd == this.eval_libc_i32("F_FULLFSYNC") {
@ -494,7 +493,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
return Ok(Scalar::from_i32(-1));
}
this.ffullsync_fd(fd)
this.ffullsync_fd(fd_num)
} else {
throw_unsup_format!("the {:#x} command is not supported for `fcntl`)", cmd);
}
@ -503,12 +502,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
fn close(&mut self, fd_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();
let fd = this.read_scalar(fd_op)?.to_i32()?;
let fd_num = this.read_scalar(fd_op)?.to_i32()?;
let Some(file_description) = this.machine.fds.remove(fd) else {
let Some(fd) = this.machine.fds.remove(fd_num) else {
return Ok(Scalar::from_i32(this.fd_not_found()?));
};
let result = file_description.close(this.machine.communicate(), this)?;
let result = fd.close(this.machine.communicate(), this)?;
// return `0` if close is successful
let result = result.map(|()| 0i32);
Ok(Scalar::from_i32(this.try_unwrap_io_result(result)?))
@ -532,7 +531,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
/// and keeps the cursor unchanged.
fn read(
&mut self,
fd: i32,
fd_num: i32,
buf: Pointer,
count: u64,
offset: Option<i128>,
@ -541,7 +540,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Isolation check is done via `FileDescription` trait.
trace!("Reading from FD {}, size {}", fd, count);
trace!("Reading from FD {}, size {}", fd_num, count);
// Check that the *entire* buffer is actually valid memory.
this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?;
@ -554,7 +553,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let communicate = this.machine.communicate();
// We temporarily dup the FD to be able to retain mutable access to `this`.
let Some(fd) = this.machine.fds.get(fd) else {
let Some(fd) = this.machine.fds.get(fd_num) else {
trace!("read: FD not found");
return Ok(Scalar::from_target_isize(this.fd_not_found()?, this));
};
@ -597,7 +596,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
fn write(
&mut self,
fd: i32,
fd_num: i32,
buf: Pointer,
count: u64,
offset: Option<i128>,
@ -618,7 +617,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?.to_owned();
// We temporarily dup the FD to be able to retain mutable access to `this`.
let Some(fd) = this.machine.fds.get(fd) else {
let Some(fd) = this.machine.fds.get(fd_num) else {
return Ok(Scalar::from_target_isize(this.fd_not_found()?, this));
};

View File

@ -554,7 +554,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
Ok(Scalar::from_i32(this.try_unwrap_io_result(fd)?))
}
fn lseek64(&mut self, fd: i32, offset: i128, whence: i32) -> InterpResult<'tcx, Scalar> {
fn lseek64(&mut self, fd_num: i32, offset: i128, whence: i32) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();
// Isolation check is done via `FileDescription` trait.
@ -580,13 +580,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let communicate = this.machine.communicate();
let Some(file_description) = this.machine.fds.get(fd) else {
let Some(fd) = this.machine.fds.get(fd_num) else {
return Ok(Scalar::from_i64(this.fd_not_found()?));
};
let result = file_description
.seek(communicate, seek_from)?
.map(|offset| i64::try_from(offset).unwrap());
drop(file_description);
let result = fd.seek(communicate, seek_from)?.map(|offset| i64::try_from(offset).unwrap());
drop(fd);
let result = this.try_unwrap_io_result(result)?;
Ok(Scalar::from_i64(result))
@ -721,7 +719,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
return Ok(Scalar::from_i32(this.fd_not_found()?));
}
let metadata = match FileMetadata::from_fd(this, fd)? {
let metadata = match FileMetadata::from_fd_num(this, fd)? {
Some(metadata) => metadata,
None => return Ok(Scalar::from_i32(-1)),
};
@ -808,7 +806,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// If the path is empty, and the AT_EMPTY_PATH flag is set, we query the open file
// represented by dirfd, whether it's a directory or otherwise.
let metadata = if path.as_os_str().is_empty() && empty_path_flag {
FileMetadata::from_fd(this, dirfd)?
FileMetadata::from_fd_num(this, dirfd)?
} else {
FileMetadata::from_path(this, &path, follow_symlink)?
};
@ -1260,7 +1258,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
}))
}
fn ftruncate64(&mut self, fd: i32, length: i128) -> InterpResult<'tcx, Scalar> {
fn ftruncate64(&mut self, fd_num: i32, length: i128) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();
// Reject if isolation is enabled.
@ -1270,30 +1268,29 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
return Ok(Scalar::from_i32(this.fd_not_found()?));
}
let Some(file_description) = this.machine.fds.get(fd) else {
let Some(fd) = this.machine.fds.get(fd_num) else {
return Ok(Scalar::from_i32(this.fd_not_found()?));
};
// FIXME: Support ftruncate64 for all FDs
let FileHandle { file, writable } =
file_description.downcast::<FileHandle>().ok_or_else(|| {
err_unsup_format!("`ftruncate64` is only supported on file-backed file descriptors")
})?;
let FileHandle { file, writable } = fd.downcast::<FileHandle>().ok_or_else(|| {
err_unsup_format!("`ftruncate64` is only supported on file-backed file descriptors")
})?;
if *writable {
if let Ok(length) = length.try_into() {
let result = file.set_len(length);
drop(file_description);
drop(fd);
let result = this.try_unwrap_io_result(result.map(|_| 0i32))?;
Ok(Scalar::from_i32(result))
} else {
drop(file_description);
drop(fd);
let einval = this.eval_libc("EINVAL");
this.set_last_error(einval)?;
Ok(Scalar::from_i32(-1))
}
} else {
drop(file_description);
drop(fd);
// The file is not writable
let einval = this.eval_libc("EINVAL");
this.set_last_error(einval)?;
@ -1321,18 +1318,17 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
self.ffullsync_fd(fd)
}
fn ffullsync_fd(&mut self, fd: i32) -> InterpResult<'tcx, Scalar> {
fn ffullsync_fd(&mut self, fd_num: i32) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();
let Some(file_description) = this.machine.fds.get(fd) else {
let Some(fd) = this.machine.fds.get(fd_num) else {
return Ok(Scalar::from_i32(this.fd_not_found()?));
};
// Only regular files support synchronization.
let FileHandle { file, writable } =
file_description.downcast::<FileHandle>().ok_or_else(|| {
err_unsup_format!("`fsync` is only supported on file-backed file descriptors")
})?;
let FileHandle { file, writable } = fd.downcast::<FileHandle>().ok_or_else(|| {
err_unsup_format!("`fsync` is only supported on file-backed file descriptors")
})?;
let io_result = maybe_sync_file(file, *writable, File::sync_all);
drop(file_description);
drop(fd);
Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?))
}
@ -1348,16 +1344,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
return Ok(Scalar::from_i32(this.fd_not_found()?));
}
let Some(file_description) = this.machine.fds.get(fd) else {
let Some(fd) = this.machine.fds.get(fd) else {
return Ok(Scalar::from_i32(this.fd_not_found()?));
};
// Only regular files support synchronization.
let FileHandle { file, writable } =
file_description.downcast::<FileHandle>().ok_or_else(|| {
err_unsup_format!("`fdatasync` is only supported on file-backed file descriptors")
})?;
let FileHandle { file, writable } = fd.downcast::<FileHandle>().ok_or_else(|| {
err_unsup_format!("`fdatasync` is only supported on file-backed file descriptors")
})?;
let io_result = maybe_sync_file(file, *writable, File::sync_data);
drop(file_description);
drop(fd);
Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?))
}
@ -1396,18 +1391,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
return Ok(Scalar::from_i32(this.fd_not_found()?));
}
let Some(file_description) = this.machine.fds.get(fd) else {
let Some(fd) = this.machine.fds.get(fd) else {
return Ok(Scalar::from_i32(this.fd_not_found()?));
};
// Only regular files support synchronization.
let FileHandle { file, writable } =
file_description.downcast::<FileHandle>().ok_or_else(|| {
err_unsup_format!(
"`sync_data_range` is only supported on file-backed file descriptors"
)
})?;
let FileHandle { file, writable } = fd.downcast::<FileHandle>().ok_or_else(|| {
err_unsup_format!("`sync_data_range` is only supported on file-backed file descriptors")
})?;
let io_result = maybe_sync_file(file, *writable, File::sync_data);
drop(file_description);
drop(fd);
Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?))
}
@ -1699,15 +1691,15 @@ impl FileMetadata {
FileMetadata::from_meta(ecx, metadata)
}
fn from_fd<'tcx>(
fn from_fd_num<'tcx>(
ecx: &mut MiriInterpCx<'tcx>,
fd: i32,
fd_num: i32,
) -> InterpResult<'tcx, Option<FileMetadata>> {
let Some(file_description) = ecx.machine.fds.get(fd) else {
let Some(fd) = ecx.machine.fds.get(fd_num) else {
return ecx.fd_not_found().map(|_: i32| None);
};
let file = &file_description
let file = &fd
.downcast::<FileHandle>()
.ok_or_else(|| {
err_unsup_format!(
@ -1717,7 +1709,7 @@ impl FileMetadata {
.file;
let metadata = file.metadata();
drop(file_description);
drop(fd);
FileMetadata::from_meta(ecx, metadata)
}

View File

@ -51,7 +51,7 @@ impl EpollEventInstance {
#[derive(Clone, Debug)]
pub struct EpollEventInterest {
/// The file descriptor value of the file description registered.
file_descriptor: i32,
fd_num: i32,
/// The events bitmask retrieved from `epoll_event`.
events: u32,
/// The data retrieved from `epoll_event`.
@ -62,7 +62,7 @@ pub struct EpollEventInterest {
/// Ready list of the epoll instance under which this EpollEventInterest is registered.
ready_list: Rc<RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>>,
/// The file descriptor value that this EpollEventInterest is registered under.
epfd: i32,
epfd_num: i32,
}
/// EpollReadyEvents reflects the readiness of a file description.
@ -339,11 +339,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Create an epoll_interest.
let interest = Rc::new(RefCell::new(EpollEventInterest {
file_descriptor: fd,
fd_num: fd,
events,
data,
ready_list: Rc::clone(ready_list),
epfd: epfd_value,
epfd_num: epfd_value,
}));
if op == epoll_ctl_add {
@ -553,7 +553,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
if is_updated {
// Edge-triggered notification only notify one thread even if there are
// multiple threads block on the same epfd.
let epfd = this.machine.fds.get(epoll_interest.borrow().epfd).unwrap();
let epfd = this.machine.fds.get(epoll_interest.borrow().epfd_num).unwrap();
// This unwrap can never fail because if the current epoll instance were
// closed and its epfd value reused, the upgrade of weak_epoll_interest
@ -615,7 +615,7 @@ fn check_and_update_one_event_interest<'tcx>(
// If there is any event that we are interested in being specified as ready,
// insert an epoll_return to the ready list.
if flags != 0 {
let epoll_key = (id, epoll_event_interest.file_descriptor);
let epoll_key = (id, epoll_event_interest.fd_num);
let ready_list = &mut epoll_event_interest.ready_list.borrow_mut();
let event_instance = EpollEventInstance::new(flags, epoll_event_interest.data);
// Triggers the notification by inserting it to the ready list.