From 1241abbec473827acf8dbbfff8f79fe08117b967 Mon Sep 17 00:00:00 2001
From: Christian Poveda <christianpoveda@protonmail.com>
Date: Thu, 17 Oct 2019 10:21:06 -0500
Subject: [PATCH] Change helper functions to read/write

---
 src/helpers.rs   | 32 ++++++++++++++++++++++++--------
 src/shims/env.rs | 20 +++-----------------
 src/shims/fs.rs  |  5 +----
 3 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/src/helpers.rs b/src/helpers.rs
index c2e4131a76d..c384b897428 100644
--- a/src/helpers.rs
+++ b/src/helpers.rs
@@ -345,10 +345,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         }
         Ok(())
     }
-}
 
-
-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 {
@@ -358,13 +357,30 @@ 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
+                .get_mut(ptr.alloc_id)?
+                .write_bytes(tcx, ptr, &bytes)
+        } else {
+            throw_unsup_format!("OsString is larger than destination")
         }
     }
+}
diff --git a/src/shims/env.rs b/src/shims/env.rs
index 1fe08aeffb7..b344a5c5494 100644
--- a/src/shims/env.rs
+++ b/src/shims/env.rs
@@ -127,20 +127,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
-                        .get_mut(buf.alloc_id)?
-                        .write_bytes(&*this.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")?;
@@ -156,10 +143,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("chdir")?;
 
-        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)?;
diff --git a/src/shims/fs.rs b/src/shims/fs.rs
index cc776295bd4..d4f9fde24e1 100644
--- a/src/shims/fs.rs
+++ b/src/shims/fs.rs
@@ -94,10 +94,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             throw_unsup_format!("unsupported flags {:#x}", flag & !mirror);
         }
 
-        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;