Auto merge of #991 - christianpoveda:errno-place, r=RalfJung

Change the last OS error location to a place

r? @RalfJung

Author: bors

src/eval.rs

@@ -183,8 +183,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
     let errno_layout = ecx.layout_of(ecx.tcx.types.u32)?;
     let errno_place = ecx.allocate(errno_layout, MiriMemoryKind::Static.into());
     ecx.write_scalar(Scalar::from_u32(0), errno_place.into())?;
-    let errno_ptr = ecx.check_mplace_access(errno_place.into(), Some(Size::from_bits(32)))?;
-    ecx.machine.last_error = errno_ptr;
+    ecx.machine.last_error = Some(errno_place);
 
     Ok(ecx)
 }

src/helpers.rs

@@ -345,4 +345,68 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         }
         Ok(())
     }
+
+    /// Sets the last error variable.
+    fn set_last_error(&mut self, scalar: Scalar<Tag>) -> InterpResult<'tcx> {
+        let this = self.eval_context_mut();
+        let errno_place = this.machine.last_error.unwrap();
+        this.write_scalar(scalar, errno_place.into())
+    }
+
+    /// Gets the last error variable.
+    fn get_last_error(&mut self) -> InterpResult<'tcx, Scalar<Tag>> {
+        let this = self.eval_context_mut();
+        let errno_place = this.machine.last_error.unwrap();
+        this.read_scalar(errno_place.into())?.not_undef()
+    }
+
+    /// Sets the last OS error using a `std::io::Error`. This function tries to produce the most
+    /// similar OS error from the `std::io::ErrorKind` and sets it as the last OS error.
+    fn set_last_error_from_io_error(&mut self, e: std::io::Error) -> InterpResult<'tcx> {
+        use std::io::ErrorKind::*;
+        let this = self.eval_context_mut();
+        let target = &this.tcx.tcx.sess.target.target;
+        let last_error = if target.options.target_family == Some("unix".to_owned()) {
+            this.eval_libc(match e.kind() {
+                ConnectionRefused => "ECONNREFUSED",
+                ConnectionReset => "ECONNRESET",
+                PermissionDenied => "EPERM",
+                BrokenPipe => "EPIPE",
+                NotConnected => "ENOTCONN",
+                ConnectionAborted => "ECONNABORTED",
+                AddrNotAvailable => "EADDRNOTAVAIL",
+                AddrInUse => "EADDRINUSE",
+                NotFound => "ENOENT",
+                Interrupted => "EINTR",
+                InvalidInput => "EINVAL",
+                TimedOut => "ETIMEDOUT",
+                AlreadyExists => "EEXIST",
+                WouldBlock => "EWOULDBLOCK",
+                _ => throw_unsup_format!("The {} error cannot be transformed into a raw os error", e)
+            })?
+        } else {
+            // FIXME: we have to implement the windows' equivalent of this.
+            throw_unsup_format!("Setting the last OS error from an io::Error is unsupported for {}.", target.target_os)
+        };
+        this.set_last_error(last_error)
+    }
+
+    /// 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.
+    ///
+    /// This function uses `T: From<i32>` instead of `i32` directly because some IO related
+    /// functions return different integer types (like `read`, that returns an `i64`)
+    fn try_unwrap_io_result<T: From<i32>>(
+        &mut self,
+        result: std::io::Result<T>,
+    ) -> InterpResult<'tcx, T> {
+        match result {
+            Ok(ok) => Ok(ok),
+            Err(e) => {
+                self.eval_context_mut().set_last_error_from_io_error(e)?;
+                Ok((-1).into())
+            }
+        }
+    }
 }

src/machine.rs

@@ -91,8 +91,8 @@ pub struct Evaluator<'tcx> {
     pub(crate) argv: Option<Pointer<Tag>>,
     pub(crate) cmd_line: Option<Pointer<Tag>>,
 
-    /// Last OS error.
-    pub(crate) last_error: Option<Pointer<Tag>>,
+    /// Last OS error location in memory. It is a 32-bit integer
+    pub(crate) last_error: Option<MPlaceTy<'tcx, Tag>>,
 
     /// TLS state.
     pub(crate) tls: TlsData<'tcx>,

src/shims/env.rs

@@ -146,7 +146,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let erange = this.eval_libc("ERANGE")?;
                 this.set_last_error(erange)?;
             }
-            Err(e) => this.consume_io_error(e)?,
+            Err(e) => this.set_last_error_from_io_error(e)?,
         }
         Ok(Scalar::ptr_null(&*this.tcx))
     }
@@ -168,7 +168,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         match env::set_current_dir(path) {
             Ok(()) => Ok(0),
             Err(e) => {
-                this.consume_io_error(e)?;
+                this.set_last_error_from_io_error(e)?;
                 Ok(-1)
             }
         }

src/shims/foreign_items.rs

@@ -414,8 +414,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             }
 
             "__errno_location" | "__error" => {
-                let errno_scalar: Scalar<Tag> = this.machine.last_error.unwrap().into();
-                this.write_scalar(errno_scalar, dest)?;
+                let errno_place = this.machine.last_error.unwrap();
+                this.write_scalar(errno_place.to_ref().to_scalar()?, dest)?;
             }
 
             "getenv" => {
@@ -977,34 +977,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         }
         return Ok(None);
     }
-
-    fn set_last_error(&mut self, scalar: Scalar<Tag>) -> InterpResult<'tcx> {
-        let this = self.eval_context_mut();
-        let errno_ptr = this.machine.last_error.unwrap();
-        // We allocated this during machine initialziation so the bounds are fine.
-        this.memory.get_mut(errno_ptr.alloc_id)?.write_scalar(
-            &*this.tcx,
-            errno_ptr,
-            scalar.into(),
-            Size::from_bits(32),
-        )
-    }
-
-    fn get_last_error(&mut self) -> InterpResult<'tcx, Scalar<Tag>> {
-        let this = self.eval_context_mut();
-        let errno_ptr = this.machine.last_error.unwrap();
-        this.memory
-            .get(errno_ptr.alloc_id)?
-            .read_scalar(&*this.tcx, errno_ptr, Size::from_bits(32))?
-            .not_undef()
-    }
-
-    fn consume_io_error(&mut self, e: std::io::Error) -> InterpResult<'tcx> {
-        self.eval_context_mut().set_last_error(Scalar::from_int(
-            e.raw_os_error().unwrap(),
-            Size::from_bits(32),
-        ))
-    }
 }
 
 // Shims the linux 'getrandom()' syscall.

src/shims/fs.rs

@@ -108,7 +108,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             fh.low
         });
 
-        this.consume_result(fd)
+        this.try_unwrap_io_result(fd)
     }
 
     fn fcntl(
@@ -144,7 +144,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let fd = this.read_scalar(fd_op)?.to_i32()?;
 
         this.remove_handle_and(fd, |handle, this| {
-            this.consume_result(handle.file.sync_all().map(|_| 0i32))
+            this.try_unwrap_io_result(handle.file.sync_all().map(|_| 0i32))
         })
     }
 
@@ -175,9 +175,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     .get_bytes_mut(&*this.tcx, buf, Size::from_bytes(count))
                     .map(|buffer| handle.file.read(buffer))
             });
-            // Reinsert the file handle
             this.machine.file_handler.handles.insert(fd, handle).unwrap_none();
-            this.consume_result(bytes?.map(|bytes| bytes as i64))
+            this.try_unwrap_io_result(bytes?.map(|bytes| bytes as i64))
         })
     }
 
@@ -206,7 +205,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     .map(|bytes| handle.file.write(bytes).map(|bytes| bytes as i64))
             });
             this.machine.file_handler.handles.insert(fd, handle).unwrap_none();
-            this.consume_result(bytes?)
+            this.try_unwrap_io_result(bytes?)
         })
     }
 
@@ -223,7 +222,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         let result = remove_file(path).map(|_| 0);
 
-        this.consume_result(result)
+        this.try_unwrap_io_result(result)
     }
 
     /// Helper function that gets a `FileHandle` immutable reference and allows to manipulate it
@@ -271,23 +270,4 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             Ok((-1).into())
         }
     }
-
-    /// Helper function that consumes an `std::io::Result<T>` and returns an
-    /// `InterpResult<'tcx,T>::Ok` instead. It is expected that the result can be converted to an
-    /// OS error using `std::io::Error::raw_os_error`.
-    ///
-    /// This function uses `T: From<i32>` instead of `i32` directly because some IO related
-    /// functions return different integer types (like `read`, that returns an `i64`)
-    fn consume_result<T: From<i32>>(
-        &mut self,
-        result: std::io::Result<T>,
-    ) -> InterpResult<'tcx, T> {
-        match result {
-            Ok(ok) => Ok(ok),
-            Err(e) => {
-                self.eval_context_mut().consume_io_error(e)?;
-                Ok((-1).into())
-            }
-        }
-    }
 }