Change helper functions to read/write

Author: pvdrz

src/helpers.rs

@@ -305,10 +305,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     fn eval_libc_i32(&mut self, name: &str) -> InterpResult<'tcx, i32> {
         self.eval_libc(name)?.to_i32()
     }
-}
-
 
-pub fn bytes_to_os_string<'tcx>(bytes: Vec<u8>) -> InterpResult<'tcx, OsString> {
+    fn read_os_string(&mut self, scalar: Scalar<Tag>) -> InterpResult<'tcx, OsString> {
+        let bytes = self.eval_context_mut().memory().read_c_str(scalar)?.to_vec();
         if cfg!(unix) {
             Ok(std::os::unix::ffi::OsStringExt::from_vec(bytes))
         } else {
@@ -318,13 +317,32 @@ pub fn bytes_to_os_string<'tcx>(bytes: Vec<u8>) -> InterpResult<'tcx, OsString>
         }
     }
 
-pub fn os_string_to_bytes<'tcx>(os_string: OsString) -> InterpResult<'tcx, Vec<u8>> {
-        if cfg!(unix) {
-            Ok(std::os::unix::ffi::OsStringExt::into_vec(os_string))
+    fn write_os_string(&mut self, os_string: OsString, ptr: Pointer<Tag>, size: u64) -> InterpResult<'tcx> {
+        let mut bytes = if cfg!(unix) {
+            std::os::unix::ffi::OsStringExt::into_vec(os_string)
         } else {
             os_string
                 .into_string()
-                .map_err(|os_string| err_unsup_format!("{:?} is not a valid utf-8 string", os_string).into())
-                .map(|s| s.into_bytes())
+                .map_err(|os_string| err_unsup_format!("{:?} is not a valid utf-8 string", os_string))?
+                .into_bytes()
+        };
+        // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required null
+        // terminator to memory using the `ptr` pointer would cause an overflow.
+        if (bytes.len() as u64) < size {
+            // We add a `/0` terminator
+            bytes.push(0);
+            let this = self.eval_context_mut();
+            let tcx = &{ this.tcx.tcx };
+            // This is ok because the buffer was strictly larger than `bytes`, so after adding the
+            // null terminator, the buffer size is larger or equal to `bytes.len()`, meaning that
+            // `bytes` actually fit inside tbe buffer.
+            this.memory_mut()
+                .get_mut(ptr.alloc_id)?
+                .write_bytes(tcx, ptr, &bytes)
+        } else {
+            throw_unsup_format!("OsString is larger than destination")
         }
     }
+}
+
+

src/shims/env.rs

@@ -131,20 +131,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         // If we cannot get the current directory, we return null
         match env::current_dir() {
             Ok(cwd) => {
-                // It is not clear what happens with non-utf8 paths here
-                let mut bytes = helpers::os_string_to_bytes(cwd.into())?;
-                // If `size` is smaller or equal than the `bytes.len()`, writing `bytes` plus the
-                // required null terminator to memory using the `buf` pointer would cause an
-                // overflow. The desired behavior in this case is to return null.
-                if (bytes.len() as u64) < size {
-                    // We add a `/0` terminator
-                    bytes.push(0);
-                    // This is ok because the buffer was strictly larger than `bytes`, so after
-                    // adding the null terminator, the buffer size is larger or equal to
-                    // `bytes.len()`, meaning that `bytes` actually fit inside tbe buffer.
-                    this.memory_mut()
-                        .get_mut(buf.alloc_id)?
-                        .write_bytes(tcx, buf, &bytes)?;
+                if this.write_os_string(cwd.into(), buf, size).is_ok() {
                     return Ok(Scalar::Ptr(buf));
                 }
                 let erange = this.eval_libc("ERANGE")?;
@@ -162,10 +149,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             throw_unsup_format!("`chdir` not available when isolation is enabled")
         }
 
-        let bytes = this.memory().read_c_str(this.read_scalar(path_op)?.not_undef()?)?;
-        let path = helpers::bytes_to_os_string(bytes.to_vec());
+        let path = this.read_os_string(this.read_scalar(path_op)?.not_undef()?)?;
 
-        match env::set_current_dir(path?) {
+        match env::set_current_dir(path) {
             Ok(()) => Ok(0),
             Err(e) => {
                 this.consume_io_error(e)?;

src/shims/fs.rs

@@ -67,10 +67,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             options.create(true);
         }
 
-        let bytes = this
-            .memory()
-            .read_c_str(this.read_scalar(path_op)?.not_undef()?)?;
-        let path: std::path::PathBuf = helpers::bytes_to_os_string(bytes.to_vec())?.into();
+        let path: std::path::PathBuf = this.read_os_string(this.read_scalar(path_op)?.not_undef()?)?.into();
 
         let fd = options.open(path).map(|file| {
             let mut fh = &mut this.machine.file_handler;