add set_last_error_and_return_i32 helper and use it in a few places

RalfJung · Oct 1, 2024 · 4f4e1d4 · 4f4e1d4
1 parent 9c21fd4
commit 4f4e1d4
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 27 deletions.
diff --git a/src/tools/miri/src/shims/io_error.rs b/src/tools/miri/src/shims/io_error.rs
@@ -119,6 +119,28 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         this.write_scalar(errno, &errno_place)
     }
 
+    /// Sets the last OS error and writes -1 to dest place.
+    fn set_last_error_and_return(
+        &mut self,
+        err: impl Into<IoError>,
+        dest: &MPlaceTy<'tcx>,
+    ) -> InterpResult<'tcx> {
+        let this = self.eval_context_mut();
+        this.set_last_error(err)?;
+        this.write_int(-1, dest)?;
+        Ok(())
+    }
+
+    /// Sets the last OS error and return `-1` as a `i32`-typed Scalar
+    fn set_last_error_and_return_i32(
+        &mut self,
+        err: impl Into<IoError>,
+    ) -> InterpResult<'tcx, Scalar> {
+        let this = self.eval_context_mut();
+        this.set_last_error(err)?;
+        Ok(Scalar::from_i32(-1))
+    }
+
     /// Gets the last error variable.
     fn get_last_error(&mut self) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
@@ -185,18 +207,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         }
     }
 
-    /// Sets the last OS error using a `std::io::ErrorKind` and writes -1 to dest place.
-    fn set_last_error_and_return(
-        &mut self,
-        err: impl Into<IoError>,
-        dest: &MPlaceTy<'tcx>,
-    ) -> InterpResult<'tcx> {
-        let this = self.eval_context_mut();
-        this.set_last_error(err)?;
-        this.write_int(-1, dest)?;
-        Ok(())
-    }
-
     /// Helper function that consumes an `std::io::Result<T>` and returns an
     /// `InterpResult<'tcx,T>::Ok` instead. In case the result is an error, this function returns
     /// `Ok(-1)` and sets the last OS error accordingly.

diff --git a/src/tools/miri/src/shims/unix/env.rs b/src/tools/miri/src/shims/unix/env.rs
@@ -177,8 +177,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             Ok(Scalar::from_i32(0)) // return zero on success
         } else {
             // name argument is a null pointer, points to an empty string, or points to a string containing an '=' character.
-            this.set_last_error(LibcError("EINVAL"))?;
-            Ok(Scalar::from_i32(-1))
+            this.set_last_error_and_return_i32(LibcError("EINVAL"))
         }
     }
 
@@ -202,8 +201,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             Ok(Scalar::from_i32(0))
         } else {
             // name argument is a null pointer, points to an empty string, or points to a string containing an '=' character.
-            this.set_last_error(LibcError("EINVAL"))?;
-            Ok(Scalar::from_i32(-1))
+            this.set_last_error_and_return_i32(LibcError("EINVAL"))
         }
     }
 
@@ -242,9 +240,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
             this.reject_in_isolation("`chdir`", reject_with)?;
-            this.set_last_error(ErrorKind::PermissionDenied)?;
-
-            return Ok(Scalar::from_i32(-1));
+            return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
         }
 
         let result = env::set_current_dir(path).map(|()| 0);

diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs
@@ -524,8 +524,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'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(ErrorKind::PermissionDenied)?;
-                return Ok(Scalar::from_i32(-1));
+                return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
             }
 
             this.ffullsync_fd(fd_num)
@@ -606,9 +605,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             None => fd.read(&fd, communicate, buf, count, dest, this)?,
             Some(offset) => {
                 let Ok(offset) = u64::try_from(offset) else {
-                    this.set_last_error(LibcError("EINVAL"))?;
-                    this.write_int(-1, dest)?;
-                    return Ok(());
+                    return this.set_last_error_and_return(LibcError("EINVAL"), dest);
                 };
                 fd.pread(communicate, offset, buf, count, dest, this)?
             }
@@ -650,9 +647,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             None => fd.write(&fd, communicate, buf, count, dest, this)?,
             Some(offset) => {
                 let Ok(offset) = u64::try_from(offset) else {
-                    this.set_last_error(LibcError("EINVAL"))?;
-                    this.write_int(-1, dest)?;
-                    return Ok(());
+                    return this.set_last_error_and_return(LibcError("EINVAL"), dest);
                 };
                 fd.pwrite(communicate, buf, count, offset, dest, this)?
             }