From 61be3bae40ffe16dff1fc2e0147ecf125064fde3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 6 Aug 2022 11:47:14 -0400 Subject: [PATCH 1/3] support current_exe on macOS, and fix write_os_str length logic --- src/shims/env.rs | 4 ++-- src/shims/os_str.rs | 10 ++++++---- src/shims/unix/fs.rs | 3 ++- src/shims/unix/macos/foreign_items.rs | 27 +++++++++++++++++++++++++++ tests/pass/current_exe.rs | 8 ++++++++ 5 files changed, 45 insertions(+), 7 deletions(-) create mode 100644 tests/pass/current_exe.rs diff --git a/src/shims/env.rs b/src/shims/env.rs index db1ddf6291..d333e78e52 100644 --- a/src/shims/env.rs +++ b/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() } } diff --git a/src/shims/os_str.rs b/src/shims/os_str.rs index b9f3a435ea..f99e2d174b 100644 --- a/src/shims/os_str.rs +++ b/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. diff --git a/src/shims/unix/fs.rs b/src/shims/unix/fs.rs index 3dccdd5e74..951ddae2c1 100644 --- a/src/shims/unix/fs.rs +++ b/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" diff --git a/src/shims/unix/macos/foreign_items.rs b/src/shims/unix/macos/foreign_items.rs index fb545d8b58..35751d5818 100644 --- a/src/shims/unix/macos/foreign_items.rs +++ b/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" => { diff --git a/tests/pass/current_exe.rs b/tests/pass/current_exe.rs new file mode 100644 index 0000000000..64f62b230e --- /dev/null +++ b/tests/pass/current_exe.rs @@ -0,0 +1,8 @@ +//@ignore-target-windows +//@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(); +} From 437d2414124b9f7409c00ceb86008c1945e1fb7a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 6 Aug 2022 11:53:18 -0400 Subject: [PATCH 2/3] move tests covering the env:: module into their own directory --- ci.sh | 2 +- tests/pass/{ => env}/args.rs | 0 tests/pass/{ => env}/args.stdout | 0 tests/pass/{ => env}/current_dir.rs | 0 tests/pass/{ => env}/current_dir_with_isolation.rs | 0 tests/pass/{ => env}/current_dir_with_isolation.stderr | 0 tests/pass/{ => env}/current_exe.rs | 1 + tests/pass/{ => env}/home.rs | 0 tests/pass/{env-exclude.rs => env/var-exclude.rs} | 0 tests/pass/{env-forward.rs => env/var-forward.rs} | 0 .../var-without-isolation.rs} | 0 tests/pass/{env.rs => env/var.rs} | 0 tests/pass/{env.stdout => env/var.stdout} | 0 tests/pass/libc.rs | 4 ++-- 14 files changed, 4 insertions(+), 3 deletions(-) rename tests/pass/{ => env}/args.rs (100%) rename tests/pass/{ => env}/args.stdout (100%) rename tests/pass/{ => env}/current_dir.rs (100%) rename tests/pass/{ => env}/current_dir_with_isolation.rs (100%) rename tests/pass/{ => env}/current_dir_with_isolation.stderr (100%) rename tests/pass/{ => env}/current_exe.rs (68%) rename tests/pass/{ => env}/home.rs (100%) rename tests/pass/{env-exclude.rs => env/var-exclude.rs} (100%) rename tests/pass/{env-forward.rs => env/var-forward.rs} (100%) rename tests/pass/{env-without-isolation.rs => env/var-without-isolation.rs} (100%) rename tests/pass/{env.rs => env/var.rs} (100%) rename tests/pass/{env.stdout => env/var.stdout} (100%) diff --git a/ci.sh b/ci.sh index f93c218f1b..67cbed4f82 100755 --- a/ci.sh +++ b/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) diff --git a/tests/pass/args.rs b/tests/pass/env/args.rs similarity index 100% rename from tests/pass/args.rs rename to tests/pass/env/args.rs diff --git a/tests/pass/args.stdout b/tests/pass/env/args.stdout similarity index 100% rename from tests/pass/args.stdout rename to tests/pass/env/args.stdout diff --git a/tests/pass/current_dir.rs b/tests/pass/env/current_dir.rs similarity index 100% rename from tests/pass/current_dir.rs rename to tests/pass/env/current_dir.rs diff --git a/tests/pass/current_dir_with_isolation.rs b/tests/pass/env/current_dir_with_isolation.rs similarity index 100% rename from tests/pass/current_dir_with_isolation.rs rename to tests/pass/env/current_dir_with_isolation.rs diff --git a/tests/pass/current_dir_with_isolation.stderr b/tests/pass/env/current_dir_with_isolation.stderr similarity index 100% rename from tests/pass/current_dir_with_isolation.stderr rename to tests/pass/env/current_dir_with_isolation.stderr diff --git a/tests/pass/current_exe.rs b/tests/pass/env/current_exe.rs similarity index 68% rename from tests/pass/current_exe.rs rename to tests/pass/env/current_exe.rs index 64f62b230e..15ea6a52b7 100644 --- a/tests/pass/current_exe.rs +++ b/tests/pass/env/current_exe.rs @@ -1,4 +1,5 @@ //@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; diff --git a/tests/pass/home.rs b/tests/pass/env/home.rs similarity index 100% rename from tests/pass/home.rs rename to tests/pass/env/home.rs diff --git a/tests/pass/env-exclude.rs b/tests/pass/env/var-exclude.rs similarity index 100% rename from tests/pass/env-exclude.rs rename to tests/pass/env/var-exclude.rs diff --git a/tests/pass/env-forward.rs b/tests/pass/env/var-forward.rs similarity index 100% rename from tests/pass/env-forward.rs rename to tests/pass/env/var-forward.rs diff --git a/tests/pass/env-without-isolation.rs b/tests/pass/env/var-without-isolation.rs similarity index 100% rename from tests/pass/env-without-isolation.rs rename to tests/pass/env/var-without-isolation.rs diff --git a/tests/pass/env.rs b/tests/pass/env/var.rs similarity index 100% rename from tests/pass/env.rs rename to tests/pass/env/var.rs diff --git a/tests/pass/env.stdout b/tests/pass/env/var.stdout similarity index 100% rename from tests/pass/env.stdout rename to tests/pass/env/var.stdout diff --git a/tests/pass/libc.rs b/tests/pass/libc.rs index 468da0845a..b584856021 100644 --- a/tests/pass/libc.rs +++ b/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(); From 79d147edb75299f9d4789b689f9c7f34a7db7709 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 6 Aug 2022 14:34:44 -0400 Subject: [PATCH 3/3] make home_dir work on macOS --- src/shims/unix/foreign_items.rs | 36 +++++++++++++++++++++++++++ src/shims/unix/linux/foreign_items.rs | 35 -------------------------- tests/pass/env/home.rs | 2 +- 3 files changed, 37 insertions(+), 36 deletions(-) diff --git a/src/shims/unix/foreign_items.rs b/src/shims/unix/foreign_items.rs index 7dde2d7d2c..6ea10de0b8 100644 --- a/src/shims/unix/foreign_items.rs +++ b/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() { diff --git a/src/shims/unix/linux/foreign_items.rs b/src/shims/unix/linux/foreign_items.rs index 61016c4249..bae3780b46 100644 --- a/src/shims/unix/linux/foreign_items.rs +++ b/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), }; diff --git a/tests/pass/env/home.rs b/tests/pass/env/home.rs index 0fb69aad00..9eb9c3af56 100644 --- a/tests/pass/env/home.rs +++ b/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;