Auto merge of #2470 - RalfJung:macos-env, r=RalfJung

support current_exe and home_dir on macOS

also fix write_os_str length logic

ci.sh

@@ -80,7 +80,7 @@ case $HOST_TARGET in
     MIRI_TEST_TARGET=i686-unknown-linux-gnu run_tests
     MIRI_TEST_TARGET=aarch64-apple-darwin run_tests
     MIRI_TEST_TARGET=i686-pc-windows-msvc run_tests
-    MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal hello integer vec current_dir data_race env
+    MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal hello integer vec data_race env/var
     MIRI_TEST_TARGET=thumbv7em-none-eabihf MIRI_NO_STD=1 run_tests_minimal no_std # no_std embedded architecture
     ;;
   x86_64-apple-darwin)

src/shims/env.rs

@@ -18,11 +18,11 @@ fn windows_check_buffer_size((success, len): (bool, u64)) -> u32 {
     if success {
         // If the function succeeds, the return value is the number of characters stored in the target buffer,
         // not including the terminating null character.
-        u32::try_from(len).unwrap()
+        u32::try_from(len.checked_sub(1).unwrap()).unwrap()
     } else {
         // If the target buffer was not large enough to hold the data, the return value is the buffer size, in characters,
         // required to hold the string and its terminating null character.
-        u32::try_from(len.checked_add(1).unwrap()).unwrap()
+        u32::try_from(len).unwrap()
     }
 }
 

src/shims/os_str.rs

@@ -92,7 +92,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     /// 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
     /// terminator. It returns `Ok((true, length))` if the writing process was successful. The
-    /// string length returned does not include the null terminator.
+    /// string length returned does include the null terminator.
     fn write_os_str_to_c_str(
         &mut self,
         os_str: &OsStr,
@@ -103,7 +103,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         // 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 out-of-bounds access.
         let string_length = u64::try_from(bytes.len()).unwrap();
-        if size <= string_length {
+        let string_length = string_length.checked_add(1).unwrap();
+        if size < string_length {
             return Ok((false, string_length));
         }
         self.eval_context_mut()
@@ -115,7 +116,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     /// 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
     /// terminator. It returns `Ok((true, length))` if the writing process was successful. The
-    /// string length returned does not include the null terminator.
+    /// string length returned does include the null terminator. Length is measured in units of
+    /// `u16.`
     fn write_os_str_to_wide_str(
         &mut self,
         os_str: &OsStr,
@@ -157,7 +159,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             alloc
                 .write_scalar(alloc_range(size2 * offset, size2), Scalar::from_u16(wchar).into())?;
         }
-        Ok((true, string_length - 1))
+        Ok((true, string_length))
     }
 
     /// Allocate enough memory to store the given `OsStr` as a null-terminated sequence of bytes.

src/shims/unix/foreign_items.rs

@@ -553,6 +553,42 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 this.write_int(super::UID, dest)?;
             }
 
+            "getpwuid_r" if this.frame_in_std() => {
+                let [uid, pwd, buf, buflen, result] =
+                    this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+                this.check_no_isolation("`getpwuid_r`")?;
+
+                let uid = this.read_scalar(uid)?.to_u32()?;
+                let pwd = this.deref_operand(pwd)?;
+                let buf = this.read_pointer(buf)?;
+                let buflen = this.read_scalar(buflen)?.to_machine_usize(this)?;
+                let result = this.deref_operand(result)?;
+
+                // Must be for "us".
+                if uid != crate::shims::unix::UID {
+                    throw_unsup_format!("`getpwuid_r` on other users is not supported");
+                }
+
+                // Reset all fields to `uninit` to make sure nobody reads them.
+                // (This is a std-only shim so we are okay with such hacks.)
+                this.write_uninit(&pwd.into())?;
+
+                // We only set the home_dir field.
+                #[allow(deprecated)]
+                let home_dir = std::env::home_dir().unwrap();
+                let (written, _) = this.write_path_to_c_str(&home_dir, buf, buflen)?;
+                let pw_dir = this.mplace_field_named(&pwd, "pw_dir")?;
+                this.write_pointer(buf, &pw_dir.into())?;
+
+                if written {
+                    this.write_pointer(pwd.ptr, &result.into())?;
+                    this.write_null(dest)?;
+                } else {
+                    this.write_null(&result.into())?;
+                    this.write_scalar(this.eval_libc("ERANGE")?, dest)?;
+                }
+            }
+
             // Platform-specific shims
             _ => {
                 match this.tcx.sess.target.os.as_ref() {

src/shims/unix/fs.rs

@@ -1380,11 +1380,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let name_place = this.mplace_field(&entry_place, 5)?;
 
                 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(
+                let (name_fits, file_name_buf_len) = this.write_os_str_to_c_str(
                     &file_name,
                     name_place.ptr,
                     name_place.layout.size.bytes(),
                 )?;
+                let file_name_len = file_name_buf_len.checked_sub(1).unwrap();
                 if !name_fits {
                     throw_unsup_format!(
                         "a directory entry had a name too large to fit in libc::dirent"

src/shims/unix/linux/foreign_items.rs

@@ -155,41 +155,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 this.write_null(dest)?;
             }
 
-            "getpwuid_r" if this.frame_in_std() => {
-                let [uid, pwd, buf, buflen, result] =
-                    this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
-                this.check_no_isolation("`getpwuid_r`")?;
-
-                let uid = this.read_scalar(uid)?.to_u32()?;
-                let pwd = this.deref_operand(pwd)?;
-                let buf = this.read_pointer(buf)?;
-                let buflen = this.read_scalar(buflen)?.to_machine_usize(this)?;
-                let result = this.deref_operand(result)?;
-
-                // Must be for "us".
-                if uid != crate::shims::unix::UID {
-                    throw_unsup_format!("`getpwuid_r` on other users is not supported");
-                }
-
-                // Reset all fields to `uninit` to make sure nobody reads them.
-                this.write_uninit(&pwd.into())?;
-
-                // We only set the home_dir field.
-                #[allow(deprecated)]
-                let home_dir = std::env::home_dir().unwrap();
-                let (written, _) = this.write_path_to_c_str(&home_dir, buf, buflen)?;
-                let pw_dir = this.mplace_field_named(&pwd, "pw_dir")?;
-                this.write_pointer(buf, &pw_dir.into())?;
-
-                if written {
-                    this.write_pointer(pwd.ptr, &result.into())?;
-                    this.write_null(dest)?;
-                } else {
-                    this.write_null(&result.into())?;
-                    this.write_scalar(this.eval_libc("ERANGE")?, dest)?;
-                }
-            }
-
             _ => return Ok(EmulateByNameResult::NotSupported),
         };
 

src/shims/unix/macos/foreign_items.rs

@@ -117,6 +117,33 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     dest,
                 )?;
             }
+            "_NSGetExecutablePath" => {
+                let [buf, bufsize] =
+                    this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+                this.check_no_isolation("`_NSGetExecutablePath`")?;
+
+                let buf_ptr = this.read_pointer(buf)?;
+                let bufsize = this.deref_operand(bufsize)?;
+
+                // Using the host current_exe is a bit off, but consistent with Linux
+                // (where stdlib reads /proc/self/exe).
+                let path = std::env::current_exe().unwrap();
+                let (written, size_needed) = this.write_path_to_c_str(
+                    &path,
+                    buf_ptr,
+                    this.read_scalar(&bufsize.into())?.to_u32()?.into(),
+                )?;
+
+                if written {
+                    this.write_null(dest)?;
+                } else {
+                    this.write_scalar(
+                        Scalar::from_u32(size_needed.try_into().unwrap()),
+                        &bufsize.into(),
+                    )?;
+                    this.write_int(-1, dest)?;
+                }
+            }
 
             // Thread-local storage
             "_tlv_atexit" => {

tests/pass/env/current_exe.rs

@@ -0,0 +1,9 @@
+//@ignore-target-windows
+//@only-on-host: the Linux std implementation opens /proc/self/exe, which doesn't work cross-target
+//@compile-flags: -Zmiri-disable-isolation
+use std::env;
+
+fn main() {
+    // The actual value we get is a bit odd: we get the Miri binary that interprets us.
+    env::current_exe().unwrap();
+}

tests/pass/env/home.rs

@@ -1,4 +1,4 @@
-//@only-target-linux: home_dir is only supported on Linux
+//@ignore-target-windows: home_dir is not supported on Windows
 //@compile-flags: -Zmiri-disable-isolation
 use std::env;
 

tests/pass/libc.rs

@@ -88,7 +88,7 @@ fn test_posix_realpath_errors() {
     assert_eq!(e.kind(), ErrorKind::NotFound);
 }
 
-#[cfg(any(target_os = "linux", target_os = "freebsd"))]
+#[cfg(any(target_os = "linux"))]
 fn test_posix_fadvise() {
     use std::convert::TryInto;
     use std::io::Write;
@@ -452,7 +452,7 @@ fn test_posix_mkstemp() {
 }
 
 fn main() {
-    #[cfg(any(target_os = "linux", target_os = "freebsd"))]
+    #[cfg(any(target_os = "linux"))]
     test_posix_fadvise();
 
     test_posix_gettimeofday();