follow-up2 to reviews from RalfJung

Author: JOE1994

src/helpers.rs

@@ -447,10 +447,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         }
     }
 
+    /// Dispatches to appropriate implementations for reading an OsString from Memory,
+    /// depending on the interpretation target.
     fn read_os_str_from_target_str<'a>(&'a self, scalar: Scalar<Tag>) -> InterpResult<'tcx, OsString> {
         let target_os = self.eval_context_ref().tcx.sess.target.target.target_os.as_str();
         match target_os {
-            "linux" => self.read_os_str_from_c_str(scalar).map(|x| x.to_os_string()),
+            "linux" | "macos" => self.read_os_str_from_c_str(scalar).map(|x| x.to_os_string()),
             "windows" => self.read_os_str_from_wide_str(scalar),
             _ => throw_unsup_format!("support for target OS not yet available"),
         }
@@ -481,11 +483,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
     /// 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>
-    where
-        'tcx: 'a,
-        'mir: 'a,
-    {
+    fn read_os_str_from_wide_str<'a>(&'a self, scalar: Scalar<Tag>) -> InterpResult<'tcx, OsString> {
         #[cfg(target_os = "windows")]
         fn u16vec_to_osstring<'tcx>(u16_vec: Vec<u16>) -> InterpResult<'tcx, OsString> {
             Ok(std::os::windows::ffi::OsStringExt::from_wide(&u16_vec[..]))
@@ -497,11 +495,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             Ok(s.into())
         }
 
-        let this = self.eval_context_ref();
-        let u16_vec = this.memory.read_wide_str(scalar)?;
+        let u16_vec = self.eval_context_ref().memory.read_wide_str(scalar)?;
         u16vec_to_osstring(u16_vec)
     }
 
+    /// Dispatches to appropriate implementations for writing an OsString to Memory,
+    /// depending on the interpretation target.
     fn write_os_str_to_target_str(
         &mut self,
         os_str: &OsStr,
@@ -510,9 +509,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     ) -> InterpResult<'tcx, (bool, u64)> {
         let target_os = self.eval_context_ref().tcx.sess.target.target.target_os.as_str();
         match target_os {
-            "linux" => self.write_os_str_to_c_str(os_str, mplace.ptr, size),
+            "linux" | "macos" => self.write_os_str_to_c_str(os_str, mplace.ptr, size),
             "windows" => self.write_os_str_to_wide_str(os_str, mplace, size),
-            _ => panic!("support for target OS not yet available"),
+            _ => throw_unsup_format!("support for target OS not yet available"),
         }
     }
 
@@ -567,15 +566,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         size: u64,
     ) -> InterpResult<'tcx, (bool, u64)> {
         #[cfg(target_os = "windows")]
-        fn os_str_to_u16vec(os_str: &OsStr) -> Vec<u16> {
-            std::os::windows::ffi::OsStrExt::encode_wide(os_str).collect()
+        fn os_str_to_u16vec<'tcx>(os_str: &OsStr) -> InterpResult<'tcx, Vec<u16>> {
+            Ok(std::os::windows::ffi::OsStrExt::encode_wide(os_str).collect())
         }
         #[cfg(not(target_os = "windows"))]
-        fn os_str_to_u16vec(os_str: &OsStr) -> Vec<u16> {
-            os_str.to_str().unwrap().encode_utf16().collect()
+        fn os_str_to_u16vec<'tcx>(os_str: &OsStr) -> InterpResult<'tcx, Vec<u16>> {
+            // On non-unix platforms the best we can do to transform Vec<u16> from/to OS strings is to do the
+            // intermediate transformation into strings. Which invalidates non-utf8 paths that are actually
+            // valid.
+            os_str
+                .to_str()
+                .map(|s| s.encode_utf16().collect())
+                .ok_or_else(|| err_unsup_format!("{:?} is not a valid utf-8 string", os_str).into())
         }
 
-        let u16_vec = os_str_to_u16vec(os_str);
+        let u16_vec = os_str_to_u16vec(os_str)?;
         // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required
         // 0x0000 terminator to memory would cause an out-of-bounds access.
         let string_length = u16_vec.len() as u64;
@@ -594,15 +599,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         Ok((true, string_length))
     }
 
+    /// Dispatches to appropriate implementations for allocating & writing OsString in Memory,
+    /// depending on the interpretation target.
     fn alloc_os_str_as_target_str(
         &mut self,
         os_str: &OsStr,
         memkind: MemoryKind<MiriMemoryKind>,
     ) -> MPlaceTy<'tcx, Tag> {
         let target_os = self.eval_context_ref().tcx.sess.target.target.target_os.as_str();
         match target_os {
-            "linux" => self.alloc_os_str_as_c_str(os_str, memkind),
+            "linux" | "macos" => self.alloc_os_str_as_c_str(os_str, memkind),
             "windows" => self.alloc_os_str_as_wide_str(os_str, memkind),
+            _ => panic!("support for target OS not yet available"),
         }
     }