route all path reading/writing through central read/write methods

Author: RalfJung

src/helpers.rs

@@ -1,6 +1,8 @@
 use std::ffi::{OsStr, OsString};
+use std::path::Path;
 use std::{iter, mem};
 use std::convert::TryFrom;
+use std::borrow::Cow;
 
 use rustc::mir;
 use rustc::ty::{
@@ -491,6 +493,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         bytes_to_os_str(bytes)
     }
 
+    /// Read a null-terminated sequence of bytes, and perform path separator conversion if needed.
+    fn read_path_from_c_str<'a>(&'a self, scalar: Scalar<Tag>) -> InterpResult<'tcx, Cow<'a, Path>>
+    where
+        'tcx: 'a,
+        'mir: 'a,
+    {
+        let os_str = self.read_os_str_from_c_str(scalar)?;
+        Ok(Cow::Borrowed(Path::new(os_str)))
+    }
+
     /// Helper function to read an OsString from a 0x0000-terminated sequence of u16,
     /// which is what the Windows APIs usually handle.
     fn read_os_str_from_wide_str<'a>(&'a self, scalar: Scalar<Tag>) -> InterpResult<'tcx, OsString>
@@ -513,7 +525,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         u16vec_to_osstring(u16_vec)
     }
 
-
     /// Helper function to write an OsStr as a null-terminated sequence of bytes, which is what
     /// the Unix APIs usually handle. This function returns `Ok((false, length))` without trying
     /// to write if `size` is not large enough to fit the contents of `os_string` plus a null
@@ -553,6 +564,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         Ok((true, string_length))
     }
 
+    /// Write a Path to the machine memory, adjusting path separators if needed.
+    fn write_path_to_c_str(
+        &mut self,
+        path: &Path,
+        scalar: Scalar<Tag>,
+        size: u64,
+    ) -> InterpResult<'tcx, (bool, u64)> {
+        let os_str = path.as_os_str();
+        self.write_os_str_to_c_str(os_str, scalar, size)
+    }
+
     /// Helper function to write an OsStr as a 0x0000-terminated u16-sequence, which is what
     /// the Windows APIs usually handle. This function returns `Ok((false, length))` without trying
     /// to write if `size` is not large enough to fit the contents of `os_string` plus a null

src/shims/env.rs

@@ -134,7 +134,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) => {
-                if this.write_os_str_to_c_str(&OsString::from(cwd), buf, size)?.0 {
+                if this.write_path_to_c_str(&cwd, buf, size)?.0 {
                     return Ok(buf);
                 }
                 let erange = this.eval_libc("ERANGE")?;
@@ -150,7 +150,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("chdir")?;
 
-        let path = this.read_os_str_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
+        let path = this.read_path_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
 
         match env::set_current_dir(path) {
             Ok(()) => Ok(0),

src/shims/fs.rs

@@ -2,7 +2,7 @@ use std::collections::BTreeMap;
 use std::convert::{TryFrom, TryInto};
 use std::fs::{read_dir, remove_dir, remove_file, rename, DirBuilder, File, FileType, OpenOptions, ReadDir};
 use std::io::{Read, Seek, SeekFrom, Write};
-use std::path::PathBuf;
+use std::path::Path;
 use std::time::SystemTime;
 
 use rustc_data_structures::fx::FxHashMap;
@@ -79,9 +79,9 @@ trait EvalContextExtPrivate<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, '
         let this = self.eval_context_mut();
 
         let path_scalar = this.read_scalar(path_op)?.not_undef()?;
-        let path: PathBuf = this.read_os_str_from_c_str(path_scalar)?.into();
+        let path = this.read_path_from_c_str(path_scalar)?.into_owned();
 
-        let metadata = match FileMetadata::from_path(this, path, follow_symlink)? {
+        let metadata = match FileMetadata::from_path(this, &path, follow_symlink)? {
             Some(metadata) => metadata,
             None => return Ok(-1),
         };
@@ -303,7 +303,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             throw_unsup_format!("unsupported flags {:#x}", flag & !mirror);
         }
 
-        let path = this.read_os_str_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
+        let path = this.read_path_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
 
         let fd = options.open(&path).map(|file| {
             let fh = &mut this.machine.file_handler;
@@ -524,10 +524,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("unlink")?;
 
-        let path = this.read_os_str_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
+        let path = this.read_path_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
 
         let result = remove_file(path).map(|_| 0);
-
         this.try_unwrap_io_result(result)
     }
 
@@ -537,12 +536,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         linkpath_op: OpTy<'tcx, Tag>
     ) -> InterpResult<'tcx, i32> {
         #[cfg(target_family = "unix")]
-        fn create_link(src: PathBuf, dst: PathBuf) -> std::io::Result<()> {
+        fn create_link(src: &Path, dst: &Path) -> std::io::Result<()> {
             std::os::unix::fs::symlink(src, dst)
         }
 
         #[cfg(target_family = "windows")]
-        fn create_link(src: PathBuf, dst: PathBuf) -> std::io::Result<()> {
+        fn create_link(src: &Path, dst: &Path) -> std::io::Result<()> {
             use std::os::windows::fs;
             if src.is_dir() {
                 fs::symlink_dir(src, dst)
@@ -555,10 +554,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("symlink")?;
 
-        let target = this.read_os_str_from_c_str(this.read_scalar(target_op)?.not_undef()?)?.into();
-        let linkpath = this.read_os_str_from_c_str(this.read_scalar(linkpath_op)?.not_undef()?)?.into();
+        let target = this.read_path_from_c_str(this.read_scalar(target_op)?.not_undef()?)?;
+        let linkpath = this.read_path_from_c_str(this.read_scalar(linkpath_op)?.not_undef()?)?;
 
-        this.try_unwrap_io_result(create_link(target, linkpath).map(|_| 0))
+        let result = create_link(&target, &linkpath).map(|_| 0);
+        this.try_unwrap_io_result(result)
     }
 
     fn macos_stat(
@@ -644,7 +644,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             this.ref_to_mplace(statxbuf_imm)?
         };
 
-        let path: PathBuf = this.read_os_str_from_c_str(pathname_scalar)?.into();
+        let path = this.read_path_from_c_str(pathname_scalar)?.into_owned();
         // `flags` should be a `c_int` but the `syscall` function provides an `isize`.
         let flags: i32 =
             this.read_scalar(flags_op)?.to_machine_isize(&*this.tcx)?.try_into().map_err(|e| {
@@ -691,7 +691,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let metadata = if path.as_os_str().is_empty() && empty_path_flag {
             FileMetadata::from_fd(this, dirfd)?
         } else {
-            FileMetadata::from_path(this, path, follow_symlink)?
+            FileMetadata::from_path(this, &path, follow_symlink)?
         };
         let metadata = match metadata {
             Some(metadata) => metadata,
@@ -785,8 +785,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             return Ok(-1);
         }
 
-        let oldpath = this.read_os_str_from_c_str(oldpath_scalar)?;
-        let newpath = this.read_os_str_from_c_str(newpath_scalar)?;
+        let oldpath = this.read_path_from_c_str(oldpath_scalar)?;
+        let newpath = this.read_path_from_c_str(newpath_scalar)?;
 
         let result = rename(oldpath, newpath).map(|_| 0);
 
@@ -808,7 +808,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             this.read_scalar(mode_op)?.to_u32()?
         };
 
-        let path = this.read_os_str_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
+        let path = this.read_path_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
 
         let mut builder = DirBuilder::new();
 
@@ -833,7 +833,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("rmdir")?;
 
-        let path = this.read_os_str_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
+        let path = this.read_path_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
 
         let result = remove_dir(path).map(|_| 0i32);
 
@@ -845,7 +845,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("opendir")?;
 
-        let name = this.read_os_str_from_c_str(this.read_scalar(name_op)?.not_undef()?)?;
+        let name = this.read_path_from_c_str(this.read_scalar(name_op)?.not_undef()?)?;
 
         let result = read_dir(name);
 
@@ -899,7 +899,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let entry_place = this.deref_operand(entry_op)?;
                 let name_place = this.mplace_field(entry_place, 4)?;
 
-                let file_name = dir_entry.file_name();
+                let file_name = dir_entry.file_name(); // not a Path as there are no separators!
                 let (name_fits, _) = this.write_os_str_to_c_str(
                     &file_name,
                     name_place.ptr,
@@ -987,7 +987,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let entry_place = this.deref_operand(entry_op)?;
                 let name_place = this.mplace_field(entry_place, 5)?;
 
-                let file_name = dir_entry.file_name();
+                let file_name = dir_entry.file_name(); // not a Path as there are no separators!
                 let (name_fits, file_name_len) = this.write_os_str_to_c_str(
                     &file_name,
                     name_place.ptr,
@@ -1082,7 +1082,7 @@ struct FileMetadata {
 impl FileMetadata {
     fn from_path<'tcx, 'mir>(
         ecx: &mut MiriEvalContext<'mir, 'tcx>,
-        path: PathBuf,
+        path: &Path,
         follow_symlink: bool
     ) -> InterpResult<'tcx, Option<FileMetadata>> {
         let metadata = if follow_symlink {