Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows: basic support for GetUserProfileDirectoryW #3502

Merged
merged 4 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ aes = { version = "0.8.3", features = ["hazmat"] }
measureme = "11"
ctrlc = "3.2.5"
chrono = { version = "0.4.38", default-features = false, features = ["clock"] }
directories = "5"

# Copied from `compiler/rustc/Cargo.toml`.
# But only for some targets, it fails for others. Rustc configures this in its CI, but we can't
Expand Down
77 changes: 65 additions & 12 deletions src/shims/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.assert_target_os("windows", "GetEnvironmentVariableW");

let name_ptr = this.read_pointer(name_op)?;
let buf_ptr = this.read_pointer(buf_op)?;
let buf_size = this.read_scalar(size_op)?.to_u32()?; // in characters

let name = this.read_os_str_from_wide_str(name_ptr)?;
Ok(match this.machine.env_vars.map.get(&name) {
Some(&var_ptr) => {
this.set_last_error(Scalar::from_u32(0))?; // make sure this is unambiguously not an error
// The offset is used to strip the "{name}=" part of the string.
#[rustfmt::skip]
let name_offset_bytes = u64::try_from(name.len()).unwrap()
Expand All @@ -172,14 +174,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let var_ptr = var_ptr.offset(Size::from_bytes(name_offset_bytes), this)?;
let var = this.read_os_str_from_wide_str(var_ptr)?;

let buf_ptr = this.read_pointer(buf_op)?;
// `buf_size` represents the size in characters.
let buf_size = u64::from(this.read_scalar(size_op)?.to_u32()?);
Scalar::from_u32(windows_check_buffer_size(
this.write_os_str_to_wide_str(
&var, buf_ptr, buf_size, /*truncate*/ false,
)?,
))
Scalar::from_u32(windows_check_buffer_size(this.write_os_str_to_wide_str(
&var,
buf_ptr,
buf_size.into(),
)?))
// This can in fact return 0. It is up to the caller to set last_error to 0
// beforehand and check it afterwards to exclude that case.
}
None => {
let envvar_not_found = this.eval_windows("c", "ERROR_ENVVAR_NOT_FOUND");
Expand Down Expand Up @@ -375,9 +376,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// If we cannot get the current directory, we return 0
match env::current_dir() {
Ok(cwd) => {
this.set_last_error(Scalar::from_u32(0))?; // make sure this is unambiguously not an error
// This can in fact return 0. It is up to the caller to set last_error to 0
// beforehand and check it afterwards to exclude that case.
return Ok(Scalar::from_u32(windows_check_buffer_size(
this.write_path_to_wide_str(&cwd, buf, size, /*truncate*/ false)?,
this.write_path_to_wide_str(&cwd, buf, size)?,
)));
}
Err(e) => this.set_last_error_from_io_error(e.kind())?,
Expand Down Expand Up @@ -494,9 +496,60 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
fn GetCurrentProcessId(&mut self) -> InterpResult<'tcx, u32> {
let this = self.eval_context_mut();
this.assert_target_os("windows", "GetCurrentProcessId");

this.check_no_isolation("`GetCurrentProcessId`")?;

Ok(std::process::id())
}

#[allow(non_snake_case)]
fn GetUserProfileDirectoryW(
&mut self,
token: &OpTy<'tcx, Provenance>, // HANDLE
buf: &OpTy<'tcx, Provenance>, // LPWSTR
size: &OpTy<'tcx, Provenance>, // LPDWORD
) -> InterpResult<'tcx, Scalar<Provenance>> // returns BOOL
{
let this = self.eval_context_mut();
this.assert_target_os("windows", "GetUserProfileDirectoryW");
this.check_no_isolation("`GetUserProfileDirectoryW`")?;

let token = this.read_target_isize(token)?;
let buf = this.read_pointer(buf)?;
let size = this.deref_pointer(size)?;

if token != -4 {
throw_unsup_format!(
"GetUserProfileDirectoryW: only CURRENT_PROCESS_TOKEN is supported"
);
}

// See <https://learn.microsoft.com/en-us/windows/win32/api/userenv/nf-userenv-getuserprofiledirectoryw> for docs.
Ok(match directories::UserDirs::new() {
Some(dirs) => {
let home = dirs.home_dir();
let size_avail = if this.ptr_is_null(size.ptr())? {
0 // if the buf pointer is null, we can't write to it; `size` will be updated to the required length
} else {
this.read_scalar(&size)?.to_u32()?
};
// Of course we cannot use `windows_check_buffer_size` here since this uses
// a different method for dealing with a too-small buffer than the other functions...
let (success, len) = this.write_path_to_wide_str(home, buf, size_avail.into())?;
// The Windows docs just say that this is written on failure. But std
// seems to rely on it always being written.
this.write_scalar(Scalar::from_u32(len.try_into().unwrap()), &size)?;
Copy link
Member Author

@RalfJung RalfJung Apr 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChrisDenton something seems to be odd here, or I am misunderstanding something. The docs say about the size pointer

If the buffer specified by lpProfileDir is not large enough or lpProfileDir is NULL, the function fails and this parameter receives the necessary buffer size, including the terminating null character.

So in particular, if the buffer is large enough, it doesn't say that the pointer is changed at all.

But the way std calls this function, it assumes that on success, sz is updated to the actual size (including the null terminator).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. We shouldn't be doing that if the behaviour isn't documented.

Given that's how it's worked in practice and Rust has apparently been relying on this for a long time, I do think updating the API docs is warranted. However, unless or until that actually happens we should act as if such a documentation change would be rejected.

Copy link
Member Author

@RalfJung RalfJung Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for confirming! I will then land this PR (if I stopped updating the size the tests would fail). I opened an issue to track this: rust-lang/rust#124325.

if success {
Scalar::from_i32(1) // return TRUE
} else {
this.set_last_error(this.eval_windows("c", "ERROR_INSUFFICIENT_BUFFER"))?;
Scalar::from_i32(0) // return FALSE
}
}
None => {
// We have to pick some error code.
this.set_last_error(this.eval_windows("c", "ERROR_BAD_USER_PROFILE"))?;
Scalar::from_i32(0) // return FALSE
}
})
}
}
68 changes: 45 additions & 23 deletions src/shims/os_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'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
/// terminator. It returns `Ok((true, length))` if the writing process was successful. The
/// string length returned does include the null terminator.
/// Helper function to write an OsStr as a null-terminated sequence of bytes, which is what the
/// Unix APIs usually handle. Returns `(success, full_len)`, where length includes the null
/// terminator. On failure, nothing is written.
fn write_os_str_to_c_str(
&mut self,
os_str: &OsStr,
Expand All @@ -87,19 +85,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
self.eval_context_mut().write_c_str(bytes, ptr, size)
}

/// Helper function to write an OsStr as a 0x0000-terminated u16-sequence, which is what the
/// Windows APIs usually handle.
///
/// If `truncate == false` (the usual mode of operation), 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 include the null terminator. Length is
/// measured in units of `u16.`
///
/// If `truncate == true`, then in case `size` is not large enough it *will* write the first
/// `size.saturating_sub(1)` many items, followed by a null terminator (if `size > 0`).
/// The return value is still `(false, length)` in that case.
fn write_os_str_to_wide_str(
/// Internal helper to share code between `write_os_str_to_wide_str` and
/// `write_os_str_to_wide_str_truncated`.
fn write_os_str_to_wide_str_helper(
&mut self,
os_str: &OsStr,
ptr: Pointer<Option<Provenance>>,
Expand Down Expand Up @@ -133,6 +121,29 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
Ok((written, size_needed))
}

/// Helper function to write an OsStr as a 0x0000-terminated u16-sequence, which is what the
/// Windows APIs usually handle. Returns `(success, full_len)`, where length is measured
/// in units of `u16` and includes the null terminator. On failure, nothing is written.
fn write_os_str_to_wide_str(
&mut self,
os_str: &OsStr,
ptr: Pointer<Option<Provenance>>,
size: u64,
) -> InterpResult<'tcx, (bool, u64)> {
self.write_os_str_to_wide_str_helper(os_str, ptr, size, /*truncate*/ false)
}

/// Like `write_os_str_to_wide_str`, but on failure as much as possible is written into
/// the buffer (always with a null terminator).
fn write_os_str_to_wide_str_truncated(
&mut self,
os_str: &OsStr,
ptr: Pointer<Option<Provenance>>,
size: u64,
) -> InterpResult<'tcx, (bool, u64)> {
self.write_os_str_to_wide_str_helper(os_str, ptr, size, /*truncate*/ true)
}

/// Allocate enough memory to store the given `OsStr` as a null-terminated sequence of bytes.
fn alloc_os_str_as_c_str(
&mut self,
Expand Down Expand Up @@ -160,9 +171,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

let arg_type = Ty::new_array(this.tcx.tcx, this.tcx.types.u16, size);
let arg_place = this.allocate(this.layout_of(arg_type).unwrap(), memkind)?;
let (written, _) = self
.write_os_str_to_wide_str(os_str, arg_place.ptr(), size, /*truncate*/ false)
.unwrap();
let (written, _) = self.write_os_str_to_wide_str(os_str, arg_place.ptr(), size).unwrap();
assert!(written);
Ok(arg_place.ptr())
}
Expand Down Expand Up @@ -217,12 +226,25 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
path: &Path,
ptr: Pointer<Option<Provenance>>,
size: u64,
truncate: bool,
) -> InterpResult<'tcx, (bool, u64)> {
let this = self.eval_context_mut();
let os_str =
this.convert_path(Cow::Borrowed(path.as_os_str()), PathConversion::HostToTarget);
this.write_os_str_to_wide_str(&os_str, ptr, size, truncate)
this.write_os_str_to_wide_str(&os_str, ptr, size)
}

/// Write a Path to the machine memory (as a null-terminated sequence of `u16`s),
/// adjusting path separators if needed.
fn write_path_to_wide_str_truncated(
&mut self,
path: &Path,
ptr: Pointer<Option<Provenance>>,
size: u64,
) -> InterpResult<'tcx, (bool, u64)> {
let this = self.eval_context_mut();
let os_str =
this.convert_path(Cow::Borrowed(path.as_os_str()), PathConversion::HostToTarget);
this.write_os_str_to_wide_str_truncated(&os_str, ptr, size)
}

/// Allocate enough memory to store a Path as a null-terminated sequence of bytes,
Expand Down
4 changes: 2 additions & 2 deletions src/shims/unix/linux/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// old_address must be a multiple of the page size
#[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero
if old_address.addr().bytes() % this.machine.page_size != 0 || new_size == 0 {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
this.set_last_error(this.eval_libc("EINVAL"))?;
return Ok(this.eval_libc("MAP_FAILED"));
}

Expand All @@ -37,7 +37,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

if flags & this.eval_libc_i32("MREMAP_MAYMOVE") == 0 {
// We only support MREMAP_MAYMOVE, so not passing the flag is just a failure
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
this.set_last_error(this.eval_libc("EINVAL"))?;
return Ok(this.eval_libc("MAP_FAILED"));
}

Expand Down
16 changes: 8 additions & 8 deletions src/shims/unix/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

// First, we do some basic argument validation as required by mmap
if (flags & (map_private | map_shared)).count_ones() != 1 {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
this.set_last_error(this.eval_libc("EINVAL"))?;
return Ok(this.eval_libc("MAP_FAILED"));
}
if length == 0 {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
this.set_last_error(this.eval_libc("EINVAL"))?;
return Ok(this.eval_libc("MAP_FAILED"));
}

Expand All @@ -77,7 +77,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
//
// Miri doesn't support MAP_FIXED or any any protections other than PROT_READ|PROT_WRITE.
if flags & map_fixed != 0 || prot != prot_read | prot_write {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("ENOTSUP")))?;
this.set_last_error(this.eval_libc("ENOTSUP"))?;
return Ok(this.eval_libc("MAP_FAILED"));
}

Expand All @@ -96,11 +96,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

let align = this.machine.page_align();
let Some(map_length) = length.checked_next_multiple_of(this.machine.page_size) else {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
this.set_last_error(this.eval_libc("EINVAL"))?;
return Ok(this.eval_libc("MAP_FAILED"));
};
if map_length > this.target_usize_max() {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
this.set_last_error(this.eval_libc("EINVAL"))?;
return Ok(this.eval_libc("MAP_FAILED"));
}

Expand Down Expand Up @@ -131,16 +131,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// as a dealloc.
#[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero
if addr.addr().bytes() % this.machine.page_size != 0 {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
this.set_last_error(this.eval_libc("EINVAL"))?;
return Ok(Scalar::from_i32(-1));
}

let Some(length) = length.checked_next_multiple_of(this.machine.page_size) else {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
this.set_last_error(this.eval_libc("EINVAL"))?;
return Ok(Scalar::from_i32(-1));
};
if length > this.target_usize_max() {
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
this.set_last_error(this.eval_libc("EINVAL"))?;
return Ok(this.eval_libc("MAP_FAILED"));
}

Expand Down
Loading
Loading