Skip to content

Commit

Permalink
Auto merge of rust-lang#3589 - RalfJung:io-error, r=RalfJung
Browse files Browse the repository at this point in the history
io::Error handling: keep around the full io::Error for longer so we can give better errors

This should help with the error message in rust-lang/miri#3587.
  • Loading branch information
bors committed May 8, 2024
2 parents 720ff0d + 1601b27 commit 15305a7
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 34 deletions.
19 changes: 8 additions & 11 deletions src/tools/miri/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -751,26 +751,23 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

/// This function tries to produce the most similar OS error from the `std::io::ErrorKind`
/// as a platform-specific errnum.
fn io_error_to_errnum(
&self,
err_kind: std::io::ErrorKind,
) -> InterpResult<'tcx, Scalar<Provenance>> {
fn io_error_to_errnum(&self, err: std::io::Error) -> InterpResult<'tcx, Scalar<Provenance>> {
let this = self.eval_context_ref();
let target = &this.tcx.sess.target;
if target.families.iter().any(|f| f == "unix") {
for &(name, kind) in UNIX_IO_ERROR_TABLE {
if err_kind == kind {
if err.kind() == kind {
return Ok(this.eval_libc(name));
}
}
throw_unsup_format!("io error {:?} cannot be translated into a raw os error", err_kind)
throw_unsup_format!("unsupported io error: {err}")
} else if target.families.iter().any(|f| f == "windows") {
for &(name, kind) in WINDOWS_IO_ERROR_TABLE {
if err_kind == kind {
if err.kind() == kind {
return Ok(this.eval_windows("c", name));
}
}
throw_unsup_format!("io error {:?} cannot be translated into a raw os error", err_kind);
throw_unsup_format!("unsupported io error: {err}");
} else {
throw_unsup_format!(
"converting io::Error into errnum is unsupported for OS {}",
Expand Down Expand Up @@ -812,8 +809,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}

/// Sets the last OS error using a `std::io::ErrorKind`.
fn set_last_error_from_io_error(&mut self, err_kind: std::io::ErrorKind) -> InterpResult<'tcx> {
self.set_last_error(self.io_error_to_errnum(err_kind)?)
fn set_last_error_from_io_error(&mut self, err: std::io::Error) -> InterpResult<'tcx> {
self.set_last_error(self.io_error_to_errnum(err)?)
}

/// Helper function that consumes an `std::io::Result<T>` and returns an
Expand All @@ -829,7 +826,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
match result {
Ok(ok) => Ok(ok),
Err(e) => {
self.eval_context_mut().set_last_error_from_io_error(e.kind())?;
self.eval_context_mut().set_last_error_from_io_error(e)?;
Ok((-1).into())
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/tools/miri/src/shims/unix/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`getcwd`", reject_with)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;
return Ok(Pointer::null());
}

Expand All @@ -241,7 +241,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let erange = this.eval_libc("ERANGE");
this.set_last_error(erange)?;
}
Err(e) => this.set_last_error_from_io_error(e.kind())?,
Err(e) => this.set_last_error_from_io_error(e)?,
}

Ok(Pointer::null())
Expand All @@ -255,15 +255,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`chdir`", reject_with)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;

return Ok(-1);
}

match env::set_current_dir(path) {
Ok(()) => Ok(0),
Err(e) => {
this.set_last_error_from_io_error(e.kind())?;
this.set_last_error_from_io_error(e)?;
Ok(-1)
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/src/shims/unix/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`fcntl`", reject_with)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;
return Ok(-1);
}

Expand Down Expand Up @@ -394,7 +394,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
Ok(read_bytes)
}
Err(e) => {
this.set_last_error_from_io_error(e.kind())?;
this.set_last_error_from_io_error(e)?;
Ok(-1)
}
}
Expand Down
24 changes: 12 additions & 12 deletions src/tools/miri/src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`open`", reject_with)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;
return Ok(-1);
}

Expand Down Expand Up @@ -434,7 +434,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`unlink`", reject_with)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;
return Ok(-1);
}

Expand Down Expand Up @@ -465,7 +465,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`symlink`", reject_with)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;
return Ok(-1);
}

Expand Down Expand Up @@ -766,7 +766,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`rename`", reject_with)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;
return Ok(-1);
}

Expand Down Expand Up @@ -794,7 +794,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`mkdir`", reject_with)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;
return Ok(-1);
}

Expand Down Expand Up @@ -822,7 +822,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`rmdir`", reject_with)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;
return Ok(-1);
}

Expand Down Expand Up @@ -859,7 +859,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
Ok(Scalar::from_target_usize(id, this))
}
Err(e) => {
this.set_last_error_from_io_error(e.kind())?;
this.set_last_error_from_io_error(e)?;
Ok(Scalar::null_ptr(this))
}
}
Expand Down Expand Up @@ -947,7 +947,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
None
}
Some(Err(e)) => {
this.set_last_error_from_io_error(e.kind())?;
this.set_last_error_from_io_error(e)?;
None
}
};
Expand Down Expand Up @@ -1299,7 +1299,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
Ok(path_bytes.len().try_into().unwrap())
}
Err(e) => {
this.set_last_error_from_io_error(e.kind())?;
this.set_last_error_from_io_error(e)?;
Ok(-1)
}
}
Expand Down Expand Up @@ -1382,7 +1382,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
Ok(Scalar::from_maybe_pointer(dest, this))
}
Err(e) => {
this.set_last_error_from_io_error(e.kind())?;
this.set_last_error_from_io_error(e)?;
Ok(Scalar::from_target_usize(0, this))
}
}
Expand Down Expand Up @@ -1503,7 +1503,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
_ => {
// "On error, -1 is returned, and errno is set to
// indicate the error"
this.set_last_error_from_io_error(e.kind())?;
this.set_last_error_from_io_error(e)?;
return Ok(-1);
}
},
Expand Down Expand Up @@ -1582,7 +1582,7 @@ impl FileMetadata {
let metadata = match metadata {
Ok(metadata) => metadata,
Err(e) => {
ecx.set_last_error_from_io_error(e.kind())?;
ecx.set_last_error_from_io_error(e)?;
return Ok(None);
}
};
Expand Down
8 changes: 4 additions & 4 deletions src/tools/miri/src/shims/windows/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`GetCurrentDirectoryW`", reject_with)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;
return Ok(Scalar::from_u32(0));
}

Expand All @@ -166,7 +166,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.write_path_to_wide_str(&cwd, buf, size)?,
)));
}
Err(e) => this.set_last_error_from_io_error(e.kind())?,
Err(e) => this.set_last_error_from_io_error(e)?,
}
Ok(Scalar::from_u32(0))
}
Expand All @@ -185,15 +185,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`SetCurrentDirectoryW`", reject_with)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;

return Ok(this.eval_windows("c", "FALSE"));
}

match env::set_current_dir(path) {
Ok(()) => Ok(this.eval_windows("c", "TRUE")),
Err(e) => {
this.set_last_error_from_io_error(e.kind())?;
this.set_last_error_from_io_error(e)?;
Ok(this.eval_windows("c", "FALSE"))
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/windows/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let filename = this.read_path_from_wide_str(filename)?;
let result = match win_absolute(&filename)? {
Err(err) => {
this.set_last_error_from_io_error(err.kind())?;
this.set_last_error_from_io_error(err)?;
Scalar::from_u32(0) // return zero upon failure
}
Ok(abs_filename) => {
Expand Down

0 comments on commit 15305a7

Please sign in to comment.