Skip to content

Commit d98f7a2

Browse files
committed
Add callback support to FileDescription
- Implementing atomic reads for contiguous buffers - Supports read operations with callback-based completion. Signed-off-by: shamb0 <r.raajey@gmail.com>
1 parent 3736728 commit d98f7a2

File tree

3 files changed

+178
-15
lines changed

3 files changed

+178
-15
lines changed

src/shims/files.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ impl<T: FileDescription + 'static> FileDescriptionExt for T {
121121

122122
pub type DynFileDescriptionRef = FileDescriptionRef<dyn FileDescription>;
123123

124+
/// Represents a dynamic callback for file I/O operations.
125+
pub type DynFileIOCallback<'tcx> = DynMachineCallback<'tcx, Result<usize, std::io::Error>>;
126+
124127
impl FileDescriptionRef<dyn FileDescription> {
125128
pub fn downcast<T: FileDescription + 'static>(self) -> Option<FileDescriptionRef<T>> {
126129
let inner = self.into_rc_any().downcast::<FdIdWith<T>>().ok()?;
@@ -135,6 +138,7 @@ pub trait FileDescription: std::fmt::Debug + FileDescriptionExt {
135138
/// Reads as much as possible into the given buffer `ptr`.
136139
/// `len` indicates how many bytes we should try to read.
137140
/// `dest` is where the return value should be stored: number of bytes read, or `-1` in case of error.
141+
#[allow(dead_code)]
138142
fn read<'tcx>(
139143
self: FileDescriptionRef<Self>,
140144
_communicate_allowed: bool,
@@ -146,6 +150,21 @@ pub trait FileDescription: std::fmt::Debug + FileDescriptionExt {
146150
throw_unsup_format!("cannot read from {}", self.name());
147151
}
148152

153+
/// Performs an asynchronous read operation on the file description.
154+
/// The caller must ensure that:
155+
/// * The buffer pointer points to valid memory of sufficient size
156+
/// * The file description remains valid for the duration of the operation
157+
fn read_with_callback<'tcx>(
158+
self: FileDescriptionRef<Self>,
159+
_communicate_allowed: bool,
160+
_ptr: Pointer,
161+
_len: usize,
162+
_completion_callback: DynFileIOCallback<'tcx>,
163+
_ecx: &mut MiriInterpCx<'tcx>,
164+
) -> InterpResult<'tcx> {
165+
throw_unsup_format!("cannot read from {}", self.name());
166+
}
167+
149168
/// Writes as much as possible from the given buffer `ptr`.
150169
/// `len` indicates how many bytes we should try to write.
151170
/// `dest` is where the return value should be stored: number of bytes written, or `-1` in case of error.

src/shims/unix/fd.rs

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::io::ErrorKind;
77
use rustc_abi::Size;
88

99
use crate::helpers::check_min_arg_count;
10-
use crate::shims::files::FileDescription;
10+
use crate::shims::files::{DynFileIOCallback, FileDescription};
1111
use crate::shims::unix::linux_like::epoll::EpollReadyEvents;
1212
use crate::shims::unix::*;
1313
use crate::*;
@@ -203,7 +203,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
203203
interp_ok(Scalar::from_i32(this.try_unwrap_io_result(result)?))
204204
}
205205

206-
/// Read data from `fd` into buffer specified by `buf` and `count`.
206+
/// Reads data from a file descriptor using callback-based completion.
207207
///
208208
/// If `offset` is `None`, reads data from current cursor position associated with `fd`
209209
/// and updates cursor position on completion. Otherwise, reads from the specified offset
@@ -244,8 +244,27 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
244244
// because it was a target's `usize`. Also we are sure that its smaller than
245245
// `usize::MAX` because it is bounded by the host's `isize`.
246246

247+
// Clone the result destination for use in the completion callback
248+
let result_destination = dest.clone();
249+
250+
let completion_callback: DynFileIOCallback<'tcx> = callback!(
251+
@capture<'tcx> {
252+
result_destination: MPlaceTy<'tcx>,
253+
}
254+
|this, read_result: Result<usize, std::io::Error>| {
255+
match read_result {
256+
Ok(read_size) => {
257+
this.write_int(u64::try_from(read_size).unwrap(), &result_destination)
258+
}
259+
Err(_err_code) => {
260+
this.set_last_error_and_return(LibcError("EIO"), &result_destination)
261+
}
262+
}
263+
}
264+
);
265+
247266
match offset {
248-
None => fd.read(communicate, buf, count, dest, this)?,
267+
None => fd.read_with_callback(communicate, buf, count, completion_callback, this)?,
249268
Some(offset) => {
250269
let Ok(offset) = u64::try_from(offset) else {
251270
return this.set_last_error_and_return(LibcError("EINVAL"), dest);

src/shims/unix/fs.rs

Lines changed: 137 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ use rustc_data_structures::fx::FxHashMap;
1414

1515
use self::shims::time::system_time_to_duration;
1616
use crate::helpers::check_min_arg_count;
17-
use crate::shims::files::{EvalContextExt as _, FileDescription, FileDescriptionRef};
17+
use crate::shims::files::{
18+
DynFileIOCallback, EvalContextExt as _, FileDescription, FileDescriptionRef,
19+
WeakFileDescriptionRef,
20+
};
1821
use crate::shims::os_str::bytes_to_os_str;
1922
use crate::shims::unix::fd::{FlockOp, UnixFileDescription};
2023
use crate::*;
@@ -23,27 +26,141 @@ use crate::*;
2326
struct FileHandle {
2427
file: File,
2528
writable: bool,
29+
/// Mutex for synchronizing file access across threads.
30+
file_lock: MutexRef,
31+
}
32+
33+
impl VisitProvenance for FileHandle {
34+
fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {
35+
// No provenance tracking needed for FileHandle as it contains no references.
36+
// This implementation satisfies the trait requirement but performs no operations.
37+
}
38+
}
39+
40+
impl FileHandle {
41+
/// Creates a new FileHandle with specified permissions and synchronization primitive.
42+
fn new(file: File, writable: bool, file_lock: MutexRef) -> Self {
43+
Self { file, writable, file_lock }
44+
}
45+
46+
/// Attempts to create a clone of the file handle while preserving all attributes.
47+
///
48+
/// # Errors
49+
/// Returns an `InterpResult` error if file handle cloning fails.
50+
fn try_clone<'tcx>(&self) -> InterpResult<'tcx, FileHandle> {
51+
let cloned_file = self
52+
.file
53+
.try_clone()
54+
.map_err(|e| err_unsup_format!("Failed to clone file handle: {}", e))?;
55+
56+
interp_ok(FileHandle {
57+
file: cloned_file,
58+
writable: self.writable,
59+
file_lock: self.file_lock.clone(),
60+
})
61+
}
62+
63+
/// Performs a synchronized file read operation with callback completion.
64+
/// Acquires a mutex lock, validates the file descriptor, performs the read,
65+
/// and invokes the callback with the result.
66+
fn perform_read<'tcx>(
67+
this: &mut MiriInterpCx<'tcx>,
68+
completion_callback: DynFileIOCallback<'tcx>,
69+
mut file_handle: FileHandle,
70+
weak_fd: WeakFileDescriptionRef<FileHandle>,
71+
buffer_ptr: Pointer,
72+
length: usize,
73+
) -> InterpResult<'tcx> {
74+
this.mutex_lock(&file_handle.file_lock);
75+
76+
let result = {
77+
// Verify file descriptor is still valid
78+
if weak_fd.upgrade().is_none() {
79+
throw_unsup_format!("file got closed while blocking")
80+
}
81+
82+
let mut bytes = vec![0; length];
83+
let read_result = file_handle.file.read(&mut bytes);
84+
85+
// Handle the read result
86+
match read_result {
87+
Ok(read_size) => {
88+
// Write the bytes to memory
89+
if let Err(err_code) = this
90+
.write_bytes_ptr(buffer_ptr, bytes[..read_size].iter().copied())
91+
.report_err()
92+
{
93+
throw_unsup_format!(
94+
"Memory write failed during file read operation: {:#?}",
95+
err_code
96+
)
97+
}
98+
completion_callback.call(this, Ok(read_size))
99+
}
100+
Err(err_code) => completion_callback.call(this, Err(err_code)),
101+
}
102+
};
103+
104+
// Always unlock the mutex, even if the read operation failed
105+
this.mutex_unlock(&file_handle.file_lock)?;
106+
107+
result
108+
}
26109
}
27110

28111
impl FileDescription for FileHandle {
29112
fn name(&self) -> &'static str {
30113
"file"
31114
}
32115

33-
fn read<'tcx>(
116+
fn read_with_callback<'tcx>(
34117
self: FileDescriptionRef<Self>,
35118
communicate_allowed: bool,
36119
ptr: Pointer,
37120
len: usize,
38-
dest: &MPlaceTy<'tcx>,
121+
completion_callback: DynFileIOCallback<'tcx>,
39122
ecx: &mut MiriInterpCx<'tcx>,
40123
) -> InterpResult<'tcx> {
124+
let this = ecx;
41125
assert!(communicate_allowed, "isolation should have prevented even opening a file");
42-
let mut bytes = vec![0; len];
43-
let result = (&mut &self.file).read(&mut bytes);
44-
match result {
45-
Ok(read_size) => ecx.return_read_success(ptr, &bytes, read_size, dest),
46-
Err(e) => ecx.set_last_error_and_return(e, dest),
126+
127+
// Clone the underlying File
128+
let clone_file_handle = match self.try_clone().report_err() {
129+
Ok(handle) => handle,
130+
Err(ec) => throw_unsup_format!("unable to clone file discp {:#?}", ec),
131+
};
132+
133+
let weak_fd = FileDescriptionRef::downgrade(&self);
134+
135+
if this.mutex_is_locked(&self.file_lock) {
136+
this.block_thread(
137+
BlockReason::Mutex,
138+
None,
139+
callback!(
140+
@capture<'tcx> {
141+
completion_callback: DynFileIOCallback<'tcx>,
142+
clone_file_handle: FileHandle,
143+
weak_fd: WeakFileDescriptionRef<FileHandle>,
144+
ptr: Pointer,
145+
len: usize,
146+
}
147+
|this, unblock: UnblockKind| {
148+
assert_eq!(unblock, UnblockKind::Ready);
149+
FileHandle::perform_read(this, completion_callback, clone_file_handle, weak_fd, ptr, len)
150+
}
151+
),
152+
);
153+
154+
unreachable!()
155+
} else {
156+
FileHandle::perform_read(
157+
this,
158+
completion_callback,
159+
clone_file_handle,
160+
weak_fd,
161+
ptr,
162+
len,
163+
)
47164
}
48165
}
49166

@@ -584,9 +701,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
584701
return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
585702
}
586703

587-
let fd = options
588-
.open(path)
589-
.map(|file| this.machine.fds.insert_new(FileHandle { file, writable }));
704+
let fd = options.open(path).map(|file| {
705+
this.machine.fds.insert_new(FileHandle::new(
706+
file,
707+
writable,
708+
this.machine.sync.mutex_create(),
709+
))
710+
});
590711

591712
interp_ok(Scalar::from_i32(this.try_unwrap_io_result(fd)?))
592713
}
@@ -1645,7 +1766,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
16451766

16461767
match file {
16471768
Ok(f) => {
1648-
let fd = this.machine.fds.insert_new(FileHandle { file: f, writable: true });
1769+
let fd = this.machine.fds.insert_new(FileHandle::new(
1770+
f,
1771+
true,
1772+
this.machine.sync.mutex_create(),
1773+
));
16491774
return interp_ok(Scalar::from_i32(fd));
16501775
}
16511776
Err(e) =>

0 commit comments

Comments
 (0)