From 1601b27d1ce29b29a011cd036c5e10e120ec89f1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 8 May 2024 18:01:21 +0200 Subject: [PATCH] io::Error handling: keep around the full io::Error for longer so we can give better errors --- src/tools/miri/src/helpers.rs | 19 +++++++-------- src/tools/miri/src/shims/unix/env.rs | 8 +++---- src/tools/miri/src/shims/unix/fd.rs | 4 ++-- src/tools/miri/src/shims/unix/fs.rs | 24 +++++++++---------- src/tools/miri/src/shims/windows/env.rs | 8 +++---- .../miri/src/shims/windows/foreign_items.rs | 2 +- 6 files changed, 31 insertions(+), 34 deletions(-) diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 795f21ddb9b13..4050ae3c4bf97 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -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> { + fn io_error_to_errnum(&self, err: std::io::Error) -> InterpResult<'tcx, Scalar> { 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 {}", @@ -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` and returns an @@ -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()) } } diff --git a/src/tools/miri/src/shims/unix/env.rs b/src/tools/miri/src/shims/unix/env.rs index 9082d13da8404..6fcfb69391536 100644 --- a/src/tools/miri/src/shims/unix/env.rs +++ b/src/tools/miri/src/shims/unix/env.rs @@ -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()); } @@ -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()) @@ -255,7 +255,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("`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); } @@ -263,7 +263,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { 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) } } diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index e536b78d1c0bc..a53cd607ef063 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -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); } @@ -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) } } diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 57ae209416c86..eb241556a5691 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -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); } @@ -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); } @@ -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); } @@ -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); } @@ -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); } @@ -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); } @@ -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)) } } @@ -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 } }; @@ -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) } } @@ -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)) } } @@ -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); } }, @@ -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); } }; diff --git a/src/tools/miri/src/shims/windows/env.rs b/src/tools/miri/src/shims/windows/env.rs index b3bc5d4d85235..0e52959b762e6 100644 --- a/src/tools/miri/src/shims/windows/env.rs +++ b/src/tools/miri/src/shims/windows/env.rs @@ -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)); } @@ -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)) } @@ -185,7 +185,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("`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")); } @@ -193,7 +193,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { 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")) } } diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index 28dad974771d6..44b0f25b55c0f 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/src/shims/windows/foreign_items.rs @@ -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) => {