Use Scalar consistently in foreign item emulation

Author: oli-obk

src/tools/miri/src/shims/time.rs

@@ -93,7 +93,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         Ok(Scalar::from_i32(0))
     }
 
-    fn gettimeofday(&mut self, tv_op: &OpTy<'tcx>, tz_op: &OpTy<'tcx>) -> InterpResult<'tcx, i32> {
+    fn gettimeofday(
+        &mut self,
+        tv_op: &OpTy<'tcx>,
+        tz_op: &OpTy<'tcx>,
+    ) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
 
         this.assert_target_os_is_unix("gettimeofday");
@@ -106,7 +110,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         if !this.ptr_is_null(tz)? {
             let einval = this.eval_libc("EINVAL");
             this.set_last_error(einval)?;
-            return Ok(-1);
+            return Ok(Scalar::from_i32(-1));
         }
 
         let duration = system_time_to_duration(&SystemTime::now())?;
@@ -115,7 +119,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         this.write_int_fields(&[tv_sec.into(), tv_usec.into()], &tv)?;
 
-        Ok(0)
+        Ok(Scalar::from_i32(0))
     }
 
     // The localtime() function shall convert the time in seconds since the Epoch pointed to by
@@ -308,7 +312,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         &mut self,
         req_op: &OpTy<'tcx>,
         _rem: &OpTy<'tcx>, // Signal handlers are not supported, so rem will never be written to.
-    ) -> InterpResult<'tcx, i32> {
+    ) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
 
         this.assert_target_os_is_unix("nanosleep");
@@ -320,7 +324,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             None => {
                 let einval = this.eval_libc("EINVAL");
                 this.set_last_error(einval)?;
-                return Ok(-1);
+                return Ok(Scalar::from_i32(-1));
             }
         };
 
@@ -333,7 +337,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 @timeout = |_this| { Ok(()) }
             ),
         );
-        Ok(0)
+        Ok(Scalar::from_i32(0))
     }
 
     #[allow(non_snake_case)]

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

@@ -148,7 +148,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         Ok(var_ptr.unwrap_or_else(Pointer::null))
     }
 
-    fn setenv(&mut self, name_op: &OpTy<'tcx>, value_op: &OpTy<'tcx>) -> InterpResult<'tcx, i32> {
+    fn setenv(
+        &mut self,
+        name_op: &OpTy<'tcx>,
+        value_op: &OpTy<'tcx>,
+    ) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
         this.assert_target_os_is_unix("setenv");
 
@@ -169,16 +173,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 this.deallocate_ptr(var, None, MiriMemoryKind::Runtime.into())?;
             }
             this.update_environ()?;
-            Ok(0) // return zero on success
+            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.
             let einval = this.eval_libc("EINVAL");
             this.set_last_error(einval)?;
-            Ok(-1)
+            Ok(Scalar::from_i32(-1))
         }
     }
 
-    fn unsetenv(&mut self, name_op: &OpTy<'tcx>) -> InterpResult<'tcx, i32> {
+    fn unsetenv(&mut self, name_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
         this.assert_target_os_is_unix("unsetenv");
 
@@ -195,12 +199,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 this.deallocate_ptr(var, None, MiriMemoryKind::Runtime.into())?;
             }
             this.update_environ()?;
-            Ok(0)
+            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.
             let einval = this.eval_libc("EINVAL");
             this.set_last_error(einval)?;
-            Ok(-1)
+            Ok(Scalar::from_i32(-1))
         }
     }
 
@@ -232,7 +236,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         Ok(Pointer::null())
     }
 
-    fn chdir(&mut self, path_op: &OpTy<'tcx>) -> InterpResult<'tcx, i32> {
+    fn chdir(&mut self, path_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
         this.assert_target_os_is_unix("chdir");
 
@@ -242,16 +246,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             this.reject_in_isolation("`chdir`", reject_with)?;
             this.set_last_error_from_io_error(ErrorKind::PermissionDenied.into())?;
 
-            return Ok(-1);
+            return Ok(Scalar::from_i32(-1));
         }
 
-        match env::set_current_dir(path) {
-            Ok(()) => Ok(0),
-            Err(e) => {
-                this.set_last_error_from_io_error(e)?;
-                Ok(-1)
-            }
-        }
+        let result = env::set_current_dir(path).map(|()| 0);
+        Ok(Scalar::from_i32(this.try_unwrap_io_result(result)?))
     }
 
     /// Updates the `environ` static.
@@ -270,18 +269,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         Ok(())
     }
 
-    fn getpid(&mut self) -> InterpResult<'tcx, i32> {
+    fn getpid(&mut self) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
         this.assert_target_os_is_unix("getpid");
 
         // The reason we need to do this wacky of a conversion is because
         // `libc::getpid` returns an i32, however, `std::process::id()` return an u32.
         // So we un-do the conversion that stdlib does and turn it back into an i32.
-        #[allow(clippy::cast_possible_wrap)]
-        Ok(this.get_pid() as i32)
+        // In `Scalar` representation, these are the same, so we don't need to anything else.
+        Ok(Scalar::from_u32(this.get_pid()))
     }
 
-    fn linux_gettid(&mut self) -> InterpResult<'tcx, i32> {
+    fn linux_gettid(&mut self) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_ref();
         this.assert_target_os("linux", "gettid");
 
@@ -290,7 +289,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // Compute a TID for this thread, ensuring that the main thread has PID == TID.
         let tid = this.get_pid().strict_add(index);
 
-        #[allow(clippy::cast_possible_wrap)]
-        Ok(tid as i32)
+        Ok(Scalar::from_u32(tid))
     }
 }

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

@@ -310,20 +310,20 @@ impl FdTable {
 
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
 pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
-    fn dup(&mut self, old_fd: i32) -> InterpResult<'tcx, i32> {
+    fn dup(&mut self, old_fd: i32) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
 
         let Some(dup_fd) = this.machine.fds.dup(old_fd) else {
-            return this.fd_not_found();
+            return Ok(Scalar::from_i32(this.fd_not_found()?));
         };
-        Ok(this.machine.fds.insert_fd_with_min_fd(dup_fd, 0))
+        Ok(Scalar::from_i32(this.machine.fds.insert_fd_with_min_fd(dup_fd, 0)))
     }
 
-    fn dup2(&mut self, old_fd: i32, new_fd: i32) -> InterpResult<'tcx, i32> {
+    fn dup2(&mut self, old_fd: i32, new_fd: i32) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
 
         let Some(dup_fd) = this.machine.fds.dup(old_fd) else {
-            return this.fd_not_found();
+            return Ok(Scalar::from_i32(this.fd_not_found()?));
         };
         if new_fd != old_fd {
             // Close new_fd if it is previously opened.
@@ -333,7 +333,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 file_description.close(this.machine.communicate())?.ok();
             }
         }
-        Ok(new_fd)
+        Ok(Scalar::from_i32(new_fd))
     }
 
     fn flock(&mut self, fd: i32, op: i32) -> InterpResult<'tcx, Scalar> {
@@ -370,7 +370,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         Ok(Scalar::from_i32(this.try_unwrap_io_result(result)?))
     }
 
-    fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, i32> {
+    fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
 
         if args.len() < 2 {
@@ -388,11 +388,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             // `FD_CLOEXEC` value without checking if the flag is set for the file because `std`
             // always sets this flag when opening a file. However we still need to check that the
             // file itself is open.
-            if this.machine.fds.is_fd(fd) {
-                Ok(this.eval_libc_i32("FD_CLOEXEC"))
+            Ok(Scalar::from_i32(if this.machine.fds.is_fd(fd) {
+                this.eval_libc_i32("FD_CLOEXEC")
             } else {
-                this.fd_not_found()
-            }
+                this.fd_not_found()?
+            }))
         } else if cmd == this.eval_libc_i32("F_DUPFD")
             || cmd == this.eval_libc_i32("F_DUPFD_CLOEXEC")
         {
@@ -409,15 +409,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             let start = this.read_scalar(&args[2])?.to_i32()?;
 
             match this.machine.fds.dup(fd) {
-                Some(dup_fd) => Ok(this.machine.fds.insert_fd_with_min_fd(dup_fd, start)),
-                None => this.fd_not_found(),
+                Some(dup_fd) =>
+                    Ok(Scalar::from_i32(this.machine.fds.insert_fd_with_min_fd(dup_fd, start))),
+                None => Ok(Scalar::from_i32(this.fd_not_found()?)),
             }
         } else if this.tcx.sess.target.os == "macos" && cmd == this.eval_libc_i32("F_FULLFSYNC") {
             // 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.into())?;
-                return Ok(-1);
+                return Ok(Scalar::from_i32(-1));
             }
 
             this.ffullsync_fd(fd)
@@ -462,7 +463,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         buf: Pointer,
         count: u64,
         offset: Option<i128>,
-    ) -> InterpResult<'tcx, i64> {
+    ) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
 
         // Isolation check is done via `FileDescriptor` trait.
@@ -482,7 +483,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // We temporarily dup the FD to be able to retain mutable access to `this`.
         let Some(fd) = this.machine.fds.dup(fd) else {
             trace!("read: FD not found");
-            return this.fd_not_found();
+            return Ok(Scalar::from_target_isize(this.fd_not_found()?, this));
         };
 
         trace!("read: FD mapped to {fd:?}");
@@ -496,7 +497,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 let Ok(offset) = u64::try_from(offset) else {
                     let einval = this.eval_libc("EINVAL");
                     this.set_last_error(einval)?;
-                    return Ok(-1);
+                    return Ok(Scalar::from_target_isize(-1, this));
                 };
                 fd.borrow_mut().pread(communicate, &mut bytes, offset, this)
             }
@@ -513,11 +514,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     buf,
                     bytes[..usize::try_from(read_bytes).unwrap()].iter().copied(),
                 )?;
-                Ok(read_bytes)
+                Ok(Scalar::from_target_isize(read_bytes, this))
             }
             Err(e) => {
                 this.set_last_error_from_io_error(e)?;
-                Ok(-1)
+                Ok(Scalar::from_target_isize(-1, this))
             }
         }
     }
@@ -528,7 +529,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         buf: Pointer,
         count: u64,
         offset: Option<i128>,
-    ) -> InterpResult<'tcx, i64> {
+    ) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
 
         // Isolation check is done via `FileDescriptor` trait.
@@ -546,7 +547,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?.to_owned();
         // We temporarily dup the FD to be able to retain mutable access to `this`.
         let Some(fd) = this.machine.fds.dup(fd) else {
-            return this.fd_not_found();
+            return Ok(Scalar::from_target_isize(this.fd_not_found()?, this));
         };
 
         let result = match offset {
@@ -555,15 +556,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 let Ok(offset) = u64::try_from(offset) else {
                     let einval = this.eval_libc("EINVAL");
                     this.set_last_error(einval)?;
-                    return Ok(-1);
+                    return Ok(Scalar::from_target_isize(-1, this));
                 };
                 fd.borrow_mut().pwrite(communicate, &bytes, offset, this)
             }
         };
         drop(fd);
 
         let result = result?.map(|c| i64::try_from(c).unwrap());
-        this.try_unwrap_io_result(result)
+        Ok(Scalar::from_target_isize(this.try_unwrap_io_result(result)?, this))
     }
 }