diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index d756c06ebdb..48bf959538b 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -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 { - let fd = self.fds.get(&fd)?; + pub fn get(&self, fd_num: i32) -> Option { + let fd = self.fds.get(&fd_num)?; Some(fd.clone()) } - pub fn remove(&mut self, fd: i32) -> Option { - self.fds.remove(&fd) + pub fn remove(&mut self, fd_num: i32) -> Option { + 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, @@ -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, @@ -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)); }; diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 58d3a73b481..e1697a47415 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -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::().ok_or_else(|| { - err_unsup_format!("`ftruncate64` is only supported on file-backed file descriptors") - })?; + let FileHandle { file, writable } = fd.downcast::().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::().ok_or_else(|| { - err_unsup_format!("`fsync` is only supported on file-backed file descriptors") - })?; + let FileHandle { file, writable } = fd.downcast::().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::().ok_or_else(|| { - err_unsup_format!("`fdatasync` is only supported on file-backed file descriptors") - })?; + let FileHandle { file, writable } = fd.downcast::().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::().ok_or_else(|| { - err_unsup_format!( - "`sync_data_range` is only supported on file-backed file descriptors" - ) - })?; + let FileHandle { file, writable } = fd.downcast::().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> { - 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::() .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) } diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index ee86cf5f26d..d91ce45e101 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -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>>, /// 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.