Auto merge of #1266 - JOE1994:write_os_str_to_wide_str, r=RalfJung

Change helper fn 'write_os_str_to_wide_str'

Helper fn `write_os_str_to_wide_str` now works on `Scalar<Tag>`, just like other **read/write os_str** helper functions.

The 2nd commit contains some changes made to `src/shims/env.rs` in PR #1225, in order to keep it from bloating too much.

Author: bors

rust-version

@@ -1 +1 @@
-a5fb9ae5b2ed3cb011ada9dc1e8633aa0927f279
+7b73d14b0b35e7b4f79f2d71dc1bbbab31698288

src/helpers.rs

@@ -463,18 +463,6 @@ 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.
-    /// FIXME: Use `Cow` to avoid copies
-    fn read_os_str_from_target_str(&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" | "macos" => self.read_os_str_from_c_str(scalar).map(|x| x.to_os_string()),
-            "windows" => self.read_os_str_from_wide_str(scalar),
-            unsupported => throw_unsup_format!("OsString support for target OS `{}` not yet available", unsupported),
-        }
-    }
-
     /// Helper function to read an OsString from a null-terminated sequence of bytes, which is what
     /// the Unix APIs usually handle.
     fn read_os_str_from_c_str<'a>(&'a self, scalar: Scalar<Tag>) -> InterpResult<'tcx, &'a OsStr>
@@ -567,7 +555,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     fn write_os_str_to_wide_str(
         &mut self,
         os_str: &OsStr,
-        mplace: MPlaceTy<'tcx, Tag>,
+        scalar: Scalar<Tag>,
         size: u64,
     ) -> InterpResult<'tcx, (bool, u64)> {
         #[cfg(windows)]
@@ -593,32 +581,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             return Ok((false, string_length));
         }
 
-        let this = self.eval_context_mut();
-
         // Store the UTF-16 string.
-        let char_size = Size::from_bytes(2);
-        for (idx, c) in u16_vec.into_iter().chain(iter::once(0x0000)).enumerate() {
-            let place = this.mplace_field(mplace, idx)?;
-            this.write_scalar(Scalar::from_uint(c, char_size), place.into())?;
-        }
+        self.eval_context_mut()
+            .memory
+            .write_u16s(scalar, u16_vec.into_iter().chain(iter::once(0x0000)))?;
         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>,
-    ) -> InterpResult<'tcx, Pointer<Tag>> {
-        let target_os = self.eval_context_ref().tcx.sess.target.target.target_os.as_str();
-        match target_os {
-            "linux" | "macos" => Ok(self.alloc_os_str_as_c_str(os_str, memkind)),
-            "windows" => Ok(self.alloc_os_str_as_wide_str(os_str, memkind)),
-            unsupported => throw_unsup_format!("OsString support for target OS `{}` not yet available", unsupported),
-        }
-    }
-
     /// Allocate enough memory to store the given `OsStr` as a null-terminated sequence of bytes.
     fn alloc_os_str_as_c_str(
         &mut self,
@@ -645,7 +614,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         let arg_type = this.tcx.mk_array(this.tcx.types.u16, size);
         let arg_place = this.allocate(this.layout_of(arg_type).unwrap(), memkind);
-        assert!(self.write_os_str_to_wide_str(os_str, arg_place, size).unwrap().0);
+        assert!(self.write_os_str_to_wide_str(os_str, arg_place.ptr, size).unwrap().0);
         arg_place.ptr.assert_ptr()
     }
 

src/shims/env.rs

@@ -26,10 +26,14 @@ impl<'tcx> EnvVars<'tcx> {
         excluded_env_vars: Vec<String>,
     ) -> InterpResult<'tcx> {
         if ecx.machine.communicate {
+            let target_os = ecx.tcx.sess.target.target.target_os.as_str();
             for (name, value) in env::vars() {
                 if !excluded_env_vars.contains(&name) {
-                    let var_ptr =
-                        alloc_env_var_as_target_str(name.as_ref(), value.as_ref(), ecx)?;
+                    let var_ptr = match target_os {
+                        "linux" | "macos" => alloc_env_var_as_c_str(name.as_ref(), value.as_ref(), ecx)?,
+                        "windows" => alloc_env_var_as_wide_str(name.as_ref(), value.as_ref(), ecx)?,
+                        unsupported => throw_unsup_format!("environment support for target OS `{}` not yet available", unsupported),
+                    };
                     ecx.machine.env_vars.map.insert(OsString::from(name), var_ptr);
                 }
             }
@@ -38,30 +42,41 @@ impl<'tcx> EnvVars<'tcx> {
     }
 }
 
-fn alloc_env_var_as_target_str<'mir, 'tcx>(
+fn alloc_env_var_as_c_str<'mir, 'tcx>(
     name: &OsStr,
     value: &OsStr,
     ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
 ) -> InterpResult<'tcx, Pointer<Tag>> {
     let mut name_osstring = name.to_os_string();
     name_osstring.push("=");
     name_osstring.push(value);
-    Ok(ecx.alloc_os_str_as_target_str(name_osstring.as_os_str(), MiriMemoryKind::Machine.into())?)
+    Ok(ecx.alloc_os_str_as_c_str(name_osstring.as_os_str(), MiriMemoryKind::Machine.into()))
+}
+
+fn alloc_env_var_as_wide_str<'mir, 'tcx>(
+    name: &OsStr,
+    value: &OsStr,
+    ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
+) -> InterpResult<'tcx, Pointer<Tag>> {
+    let mut name_osstring = name.to_os_string();
+    name_osstring.push("=");
+    name_osstring.push(value);
+    Ok(ecx.alloc_os_str_as_wide_str(name_osstring.as_os_str(), MiriMemoryKind::Machine.into()))
 }
 
 impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
     fn getenv(&mut self, name_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, Scalar<Tag>> {
         let this = self.eval_context_mut();
-        let target_os = this.tcx.sess.target.target.target_os.as_str();
-        assert!(target_os == "linux" || target_os == "macos", "`{}` is only available for the UNIX target family");
+        let target_os = &this.tcx.sess.target.target.target_os;
+        assert!(target_os == "linux" || target_os == "macos", "`getenv` is only available for the UNIX target family");
 
         let name_ptr = this.read_scalar(name_op)?.not_undef()?;
         let name = this.read_os_str_from_c_str(name_ptr)?;
         Ok(match this.machine.env_vars.map.get(name) {
-            // The offset is used to strip the "{name}=" part of the string.
             Some(var_ptr) => {
-                Scalar::from(var_ptr.offset(Size::from_bytes(name.len()) + Size::from_bytes(1), this)?)
+                // The offset is used to strip the "{name}=" part of the string.
+                Scalar::from(var_ptr.offset(Size::from_bytes(u64::try_from(name.len()).unwrap().checked_add(1).unwrap()), this)?)
             }
             None => Scalar::ptr_null(&*this.tcx),
         })
@@ -73,32 +88,40 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         value_op: OpTy<'tcx, Tag>,
     ) -> InterpResult<'tcx, i32> {
         let mut this = self.eval_context_mut();
+        let target_os = &this.tcx.sess.target.target.target_os;
+        assert!(target_os == "linux" || target_os == "macos", "`setenv` is only available for the UNIX target family");
 
         let name_ptr = this.read_scalar(name_op)?.not_undef()?;
         let value_ptr = this.read_scalar(value_op)?.not_undef()?;
-        let value = this.read_os_str_from_target_str(value_ptr)?;
+
         let mut new = None;
         if !this.is_null(name_ptr)? {
-            let name = this.read_os_str_from_target_str(name_ptr)?;
+            let name = this.read_os_str_from_c_str(name_ptr)?;
             if !name.is_empty() && !name.to_string_lossy().contains('=') {
+                let value = this.read_os_str_from_c_str(value_ptr)?;
                 new = Some((name.to_owned(), value.to_owned()));
             }
         }
         if let Some((name, value)) = new {
-            let var_ptr = alloc_env_var_as_target_str(&name, &value, &mut this)?;
+            let var_ptr = alloc_env_var_as_c_str(&name, &value, &mut this)?;
             if let Some(var) = this.machine.env_vars.map.insert(name, var_ptr) {
                 this.memory
                     .deallocate(var, None, MiriMemoryKind::Machine.into())?;
             }
             this.update_environ()?;
-            Ok(0)
+            Ok(0) // return zero on success
         } else {
+            // name argument is a null pointer, points to an empty string, or points to a string containing an '=' character.
+            let einval = this.eval_libc("EINVAL")?;
+            this.set_last_error(einval)?;
             Ok(-1)
         }
     }
 
     fn unsetenv(&mut self, name_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
+        let target_os = &this.tcx.sess.target.target.target_os;
+        assert!(target_os == "linux" || target_os == "macos", "`unsetenv` is only available for the UNIX target family");
 
         let name_ptr = this.read_scalar(name_op)?.not_undef()?;
         let mut success = None;
@@ -116,6 +139,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             this.update_environ()?;
             Ok(0)
         } else {
+            // name argument is a null pointer, points to an empty string, or points to a string containing an '=' character.
+            let einval = this.eval_libc("EINVAL")?;
+            this.set_last_error(einval)?;
             Ok(-1)
         }
     }