Skip to content

Commit 86783be

Browse files
committed
Auto merge of #3712 - tiif:feat/epoll, r=oli-obk
Implement epoll shim This PR: - implemented non-blocking ``epoll`` for #3448 . The design for this PR is documented in https://hackmd.io/`@tiif/SJatftrH0` . - renamed FileDescriptor to FileDescriptionRef - assigned an ID to every file description
2 parents f25ca7e + 607c4f5 commit 86783be

File tree

12 files changed

+1134
-75
lines changed

12 files changed

+1134
-75
lines changed

src/tools/miri/src/helpers.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
371371
path_ty_layout(this, &["std", "sys", "pal", "windows", "c", name])
372372
}
373373

374+
/// Helper function to get `TyAndLayout` of an array that consists of `libc` type.
375+
fn libc_array_ty_layout(&self, name: &str, size: u64) -> TyAndLayout<'tcx> {
376+
let this = self.eval_context_ref();
377+
let elem_ty_layout = this.libc_ty_layout(name);
378+
let array_ty = Ty::new_array(*this.tcx, elem_ty_layout.ty, size);
379+
this.layout_of(array_ty).unwrap()
380+
}
381+
374382
/// Project to the given *named* field (which must be a struct or union type).
375383
fn project_field_named<P: Projectable<'tcx, Provenance>>(
376384
&self,

src/tools/miri/src/machine.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,9 @@ pub struct MiriMachine<'tcx> {
455455
/// The table of directory descriptors.
456456
pub(crate) dirs: shims::DirTable,
457457

458+
/// The list of all EpollEventInterest.
459+
pub(crate) epoll_interests: shims::EpollInterestTable,
460+
458461
/// This machine's monotone clock.
459462
pub(crate) clock: Clock,
460463

@@ -649,6 +652,7 @@ impl<'tcx> MiriMachine<'tcx> {
649652
isolated_op: config.isolated_op,
650653
validation: config.validation,
651654
fds: shims::FdTable::init(config.mute_stdout_stderr),
655+
epoll_interests: shims::EpollInterestTable::new(),
652656
dirs: Default::default(),
653657
layouts,
654658
threads,
@@ -787,6 +791,7 @@ impl VisitProvenance for MiriMachine<'_> {
787791
data_race,
788792
alloc_addresses,
789793
fds,
794+
epoll_interests:_,
790795
tcx: _,
791796
isolated_op: _,
792797
validation: _,

src/tools/miri/src/shims/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub mod panic;
1717
pub mod time;
1818
pub mod tls;
1919

20-
pub use unix::{DirTable, FdTable};
20+
pub use unix::{DirTable, EpollInterestTable, FdTable};
2121

2222
/// What needs to be done after emulating an item (a shim or an intrinsic) is done.
2323
pub enum EmulateItemResult {

src/tools/miri/src/shims/unix/fd.rs

Lines changed: 91 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ use std::cell::{Ref, RefCell, RefMut};
66
use std::collections::BTreeMap;
77
use std::io::{self, ErrorKind, IsTerminal, Read, SeekFrom, Write};
88
use std::rc::Rc;
9+
use std::rc::Weak;
910

1011
use rustc_target::abi::Size;
1112

13+
use crate::shims::unix::linux::epoll::EpollReadyEvents;
1214
use crate::shims::unix::*;
1315
use crate::*;
1416

@@ -27,6 +29,7 @@ pub trait FileDescription: std::fmt::Debug + Any {
2729
fn read<'tcx>(
2830
&mut self,
2931
_communicate_allowed: bool,
32+
_fd_id: FdId,
3033
_bytes: &mut [u8],
3134
_ecx: &mut MiriInterpCx<'tcx>,
3235
) -> InterpResult<'tcx, io::Result<usize>> {
@@ -37,6 +40,7 @@ pub trait FileDescription: std::fmt::Debug + Any {
3740
fn write<'tcx>(
3841
&mut self,
3942
_communicate_allowed: bool,
43+
_fd_id: FdId,
4044
_bytes: &[u8],
4145
_ecx: &mut MiriInterpCx<'tcx>,
4246
) -> InterpResult<'tcx, io::Result<usize>> {
@@ -80,6 +84,7 @@ pub trait FileDescription: std::fmt::Debug + Any {
8084
fn close<'tcx>(
8185
self: Box<Self>,
8286
_communicate_allowed: bool,
87+
_ecx: &mut MiriInterpCx<'tcx>,
8388
) -> InterpResult<'tcx, io::Result<()>> {
8489
throw_unsup_format!("cannot close {}", self.name());
8590
}
@@ -97,6 +102,11 @@ pub trait FileDescription: std::fmt::Debug + Any {
97102
// so we use a default impl here.
98103
false
99104
}
105+
106+
/// Check the readiness of file description.
107+
fn get_epoll_ready_events<'tcx>(&self) -> InterpResult<'tcx, EpollReadyEvents> {
108+
throw_unsup_format!("{}: epoll does not support this file description", self.name());
109+
}
100110
}
101111

102112
impl dyn FileDescription {
@@ -119,6 +129,7 @@ impl FileDescription for io::Stdin {
119129
fn read<'tcx>(
120130
&mut self,
121131
communicate_allowed: bool,
132+
_fd_id: FdId,
122133
bytes: &mut [u8],
123134
_ecx: &mut MiriInterpCx<'tcx>,
124135
) -> InterpResult<'tcx, io::Result<usize>> {
@@ -142,6 +153,7 @@ impl FileDescription for io::Stdout {
142153
fn write<'tcx>(
143154
&mut self,
144155
_communicate_allowed: bool,
156+
_fd_id: FdId,
145157
bytes: &[u8],
146158
_ecx: &mut MiriInterpCx<'tcx>,
147159
) -> InterpResult<'tcx, io::Result<usize>> {
@@ -170,6 +182,7 @@ impl FileDescription for io::Stderr {
170182
fn write<'tcx>(
171183
&mut self,
172184
_communicate_allowed: bool,
185+
_fd_id: FdId,
173186
bytes: &[u8],
174187
_ecx: &mut MiriInterpCx<'tcx>,
175188
) -> InterpResult<'tcx, io::Result<usize>> {
@@ -195,6 +208,7 @@ impl FileDescription for NullOutput {
195208
fn write<'tcx>(
196209
&mut self,
197210
_communicate_allowed: bool,
211+
_fd_id: FdId,
198212
bytes: &[u8],
199213
_ecx: &mut MiriInterpCx<'tcx>,
200214
) -> InterpResult<'tcx, io::Result<usize>> {
@@ -203,36 +217,98 @@ impl FileDescription for NullOutput {
203217
}
204218
}
205219

220+
/// Structure contains both the file description and its unique identifier.
221+
#[derive(Clone, Debug)]
222+
pub struct FileDescWithId<T: FileDescription + ?Sized> {
223+
id: FdId,
224+
file_description: RefCell<Box<T>>,
225+
}
226+
206227
#[derive(Clone, Debug)]
207-
pub struct FileDescriptionRef(Rc<RefCell<Box<dyn FileDescription>>>);
228+
pub struct FileDescriptionRef(Rc<FileDescWithId<dyn FileDescription>>);
208229

209230
impl FileDescriptionRef {
210-
fn new(fd: impl FileDescription) -> Self {
211-
FileDescriptionRef(Rc::new(RefCell::new(Box::new(fd))))
231+
fn new(fd: impl FileDescription, id: FdId) -> Self {
232+
FileDescriptionRef(Rc::new(FileDescWithId {
233+
id,
234+
file_description: RefCell::new(Box::new(fd)),
235+
}))
212236
}
213237

214238
pub fn borrow(&self) -> Ref<'_, dyn FileDescription> {
215-
Ref::map(self.0.borrow(), |fd| fd.as_ref())
239+
Ref::map(self.0.file_description.borrow(), |fd| fd.as_ref())
216240
}
217241

218242
pub fn borrow_mut(&self) -> RefMut<'_, dyn FileDescription> {
219-
RefMut::map(self.0.borrow_mut(), |fd| fd.as_mut())
243+
RefMut::map(self.0.file_description.borrow_mut(), |fd| fd.as_mut())
220244
}
221245

222-
pub fn close<'ctx>(self, communicate_allowed: bool) -> InterpResult<'ctx, io::Result<()>> {
246+
pub fn close<'tcx>(
247+
self,
248+
communicate_allowed: bool,
249+
ecx: &mut MiriInterpCx<'tcx>,
250+
) -> InterpResult<'tcx, io::Result<()>> {
223251
// Destroy this `Rc` using `into_inner` so we can call `close` instead of
224252
// implicitly running the destructor of the file description.
253+
let id = self.get_id();
225254
match Rc::into_inner(self.0) {
226-
Some(fd) => RefCell::into_inner(fd).close(communicate_allowed),
255+
Some(fd) => {
256+
// Remove entry from the global epoll_event_interest table.
257+
ecx.machine.epoll_interests.remove(id);
258+
259+
RefCell::into_inner(fd.file_description).close(communicate_allowed, ecx)
260+
}
227261
None => Ok(Ok(())),
228262
}
229263
}
264+
265+
pub fn downgrade(&self) -> WeakFileDescriptionRef {
266+
WeakFileDescriptionRef { weak_ref: Rc::downgrade(&self.0) }
267+
}
268+
269+
pub fn get_id(&self) -> FdId {
270+
self.0.id
271+
}
272+
273+
/// Function used to retrieve the readiness events of a file description and insert
274+
/// an `EpollEventInstance` into the ready list if the file description is ready.
275+
pub(crate) fn check_and_update_readiness<'tcx>(
276+
&self,
277+
ecx: &mut InterpCx<'tcx, MiriMachine<'tcx>>,
278+
) -> InterpResult<'tcx, ()> {
279+
use crate::shims::unix::linux::epoll::EvalContextExt;
280+
ecx.check_and_update_readiness(self.get_id(), || self.borrow_mut().get_epoll_ready_events())
281+
}
282+
}
283+
284+
/// Holds a weak reference to the actual file description.
285+
#[derive(Clone, Debug, Default)]
286+
pub struct WeakFileDescriptionRef {
287+
weak_ref: Weak<FileDescWithId<dyn FileDescription>>,
288+
}
289+
290+
impl WeakFileDescriptionRef {
291+
pub fn upgrade(&self) -> Option<FileDescriptionRef> {
292+
if let Some(file_desc_with_id) = self.weak_ref.upgrade() {
293+
return Some(FileDescriptionRef(file_desc_with_id));
294+
}
295+
None
296+
}
230297
}
231298

299+
/// A unique id for file descriptions. While we could use the address, considering that
300+
/// is definitely unique, the address would expose interpreter internal state when used
301+
/// for sorting things. So instead we generate a unique id per file description that stays
302+
/// the same even if a file descriptor is duplicated and gets a new integer file descriptor.
303+
#[derive(Debug, Copy, Clone, Default, Eq, PartialEq, Ord, PartialOrd)]
304+
pub struct FdId(usize);
305+
232306
/// The file descriptor table
233307
#[derive(Debug)]
234308
pub struct FdTable {
235-
fds: BTreeMap<i32, FileDescriptionRef>,
309+
pub fds: BTreeMap<i32, FileDescriptionRef>,
310+
/// Unique identifier for file description, used to differentiate between various file description.
311+
next_file_description_id: FdId,
236312
}
237313

238314
impl VisitProvenance for FdTable {
@@ -243,7 +319,7 @@ impl VisitProvenance for FdTable {
243319

244320
impl FdTable {
245321
fn new() -> Self {
246-
FdTable { fds: BTreeMap::new() }
322+
FdTable { fds: BTreeMap::new(), next_file_description_id: FdId(0) }
247323
}
248324
pub(crate) fn init(mute_stdout_stderr: bool) -> FdTable {
249325
let mut fds = FdTable::new();
@@ -260,7 +336,8 @@ impl FdTable {
260336

261337
/// Insert a new file description to the FdTable.
262338
pub fn insert_new(&mut self, fd: impl FileDescription) -> i32 {
263-
let file_handle = FileDescriptionRef::new(fd);
339+
let file_handle = FileDescriptionRef::new(fd, self.next_file_description_id);
340+
self.next_file_description_id = FdId(self.next_file_description_id.0.strict_add(1));
264341
self.insert_ref_with_min_fd(file_handle, 0)
265342
}
266343

@@ -337,7 +414,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
337414
// If old_fd and new_fd point to the same description, then `dup_fd` ensures we keep the underlying file description alive.
338415
if let Some(file_description) = this.machine.fds.fds.insert(new_fd, dup_fd) {
339416
// Ignore close error (not interpreter's) according to dup2() doc.
340-
file_description.close(this.machine.communicate())?.ok();
417+
file_description.close(this.machine.communicate(), this)?.ok();
341418
}
342419
}
343420
Ok(Scalar::from_i32(new_fd))
@@ -442,7 +519,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
442519
let Some(file_description) = this.machine.fds.remove(fd) else {
443520
return Ok(Scalar::from_i32(this.fd_not_found()?));
444521
};
445-
let result = file_description.close(this.machine.communicate())?;
522+
let result = file_description.close(this.machine.communicate(), this)?;
446523
// return `0` if close is successful
447524
let result = result.map(|()| 0i32);
448525
Ok(Scalar::from_i32(this.try_unwrap_io_result(result)?))
@@ -499,7 +576,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
499576
// `usize::MAX` because it is bounded by the host's `isize`.
500577
let mut bytes = vec![0; usize::try_from(count).unwrap()];
501578
let result = match offset {
502-
None => fd.borrow_mut().read(communicate, &mut bytes, this),
579+
None => fd.borrow_mut().read(communicate, fd.get_id(), &mut bytes, this),
503580
Some(offset) => {
504581
let Ok(offset) = u64::try_from(offset) else {
505582
let einval = this.eval_libc("EINVAL");
@@ -509,7 +586,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
509586
fd.borrow_mut().pread(communicate, &mut bytes, offset, this)
510587
}
511588
};
512-
drop(fd);
513589

514590
// `File::read` never returns a value larger than `count`, so this cannot fail.
515591
match result?.map(|c| i64::try_from(c).unwrap()) {
@@ -558,7 +634,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
558634
};
559635

560636
let result = match offset {
561-
None => fd.borrow_mut().write(communicate, &bytes, this),
637+
None => fd.borrow_mut().write(communicate, fd.get_id(), &bytes, this),
562638
Some(offset) => {
563639
let Ok(offset) = u64::try_from(offset) else {
564640
let einval = this.eval_libc("EINVAL");
@@ -568,7 +644,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
568644
fd.borrow_mut().pwrite(communicate, &bytes, offset, this)
569645
}
570646
};
571-
drop(fd);
572647

573648
let result = result?.map(|c| i64::try_from(c).unwrap());
574649
Ok(Scalar::from_target_isize(this.try_unwrap_io_result(result)?, this))

src/tools/miri/src/shims/unix/fs.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use rustc_data_structures::fx::FxHashMap;
1212
use rustc_target::abi::Size;
1313

1414
use crate::shims::os_str::bytes_to_os_str;
15+
use crate::shims::unix::fd::FdId;
1516
use crate::shims::unix::*;
1617
use crate::*;
1718
use shims::time::system_time_to_duration;
@@ -32,6 +33,7 @@ impl FileDescription for FileHandle {
3233
fn read<'tcx>(
3334
&mut self,
3435
communicate_allowed: bool,
36+
_fd_id: FdId,
3537
bytes: &mut [u8],
3638
_ecx: &mut MiriInterpCx<'tcx>,
3739
) -> InterpResult<'tcx, io::Result<usize>> {
@@ -42,6 +44,7 @@ impl FileDescription for FileHandle {
4244
fn write<'tcx>(
4345
&mut self,
4446
communicate_allowed: bool,
47+
_fd_id: FdId,
4548
bytes: &[u8],
4649
_ecx: &mut MiriInterpCx<'tcx>,
4750
) -> InterpResult<'tcx, io::Result<usize>> {
@@ -109,6 +112,7 @@ impl FileDescription for FileHandle {
109112
fn close<'tcx>(
110113
self: Box<Self>,
111114
communicate_allowed: bool,
115+
_ecx: &mut MiriInterpCx<'tcx>,
112116
) -> InterpResult<'tcx, io::Result<()>> {
113117
assert!(communicate_allowed, "isolation should have prevented even opening a file");
114118
// We sync the file if it was opened in a mode different than read-only.

0 commit comments

Comments
 (0)