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

Support getting file metadata on Windows #4067

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9b61c05
Refactor `Handle` slightly
CraftSpider Dec 2, 2024
ab0e138
Implement trivial file operations - opening and closing handles. Just…
CraftSpider Dec 2, 2024
e19deaa
Fix MAIN_THREAD handle in windows_join_main
CraftSpider Dec 2, 2024
f1bb77d
Try fix for Windows paths on non-Windows machines
CraftSpider Dec 2, 2024
9a76a85
Most review comments - still missing shim tests
CraftSpider Dec 6, 2024
cc6eae5
Don't leak miri implementation details
CraftSpider Dec 6, 2024
3653169
Fix clippy
CraftSpider Dec 6, 2024
a64dbd1
Test file creation and metadata shims directly
CraftSpider Dec 6, 2024
274d90f
Move windows-fs to pass-dep and use windows_sys
CraftSpider Dec 8, 2024
51273f5
Move FdNum to shims/files.rs, use it in FdTable definitions
CraftSpider Dec 8, 2024
aa6bbf6
Slightly improve flag handling - parse and validate in one place
CraftSpider Dec 8, 2024
938430f
Fixup imports, compile
CraftSpider Dec 8, 2024
3c2ed8a
Make metadata handle store the metadata, instead of just a path. Add …
CraftSpider Dec 8, 2024
ebfc768
Improve extract_windows_epoch impl
CraftSpider Dec 8, 2024
d989984
Improve extract_windows_epoch impl comments
CraftSpider Dec 8, 2024
e5ada76
Add invalid handle encoding test
CraftSpider Dec 8, 2024
52c1676
Add tests for CREATE_ALWAYS and OPEN_ALWAYS error behavior. Add comme…
CraftSpider Dec 8, 2024
5ac99da
Extract Windows epoch helpers from GetSystemTimeAsFileTime and use th…
CraftSpider Dec 8, 2024
ef5ab7f
Merge FileHandle implementation between Unix and Windows
CraftSpider Dec 9, 2024
6ee99aa
Use u32::MAX constant
CraftSpider Dec 13, 2024
a61daf2
Some fs improvements
CraftSpider Dec 16, 2024
92f41ce
Use FdNum more places
CraftSpider Dec 16, 2024
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
51 changes: 27 additions & 24 deletions src/shims/windows/fs.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use std::borrow::Cow;
use std::fs::{File, Metadata, OpenOptions};
use std::io;
use std::io::{IsTerminal, Read, Seek, SeekFrom, Write};
use std::path::{Path, PathBuf};
use std::path::PathBuf;
use std::time::SystemTime;

use rustc_abi::Size;
Expand Down Expand Up @@ -116,6 +115,14 @@ impl FileDescription for DirHandle {
fn metadata<'tcx>(&self) -> InterpResult<'tcx, io::Result<Metadata>> {
interp_ok(self.path.metadata())
}

fn close<'tcx>(
self: Box<Self>,
_communicate_allowed: bool,
_ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx, io::Result<()>> {
interp_ok(Ok(()))
}
}

#[derive(Debug)]
Expand All @@ -131,6 +138,14 @@ impl FileDescription for MetadataHandle {
fn metadata<'tcx>(&self) -> InterpResult<'tcx, io::Result<Metadata>> {
interp_ok(self.path.metadata())
}

fn close<'tcx>(
self: Box<Self>,
_communicate_allowed: bool,
_ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx, io::Result<()>> {
interp_ok(Ok(()))
}
}

impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
Expand All @@ -151,9 +166,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
this.assert_target_os("windows", "CreateFileW");
this.check_no_isolation("`CreateFileW`")?;

let file_name =
String::from_utf16_lossy(&this.read_wide_str(this.read_pointer(file_name)?)?);
let file_name = local_path(&file_name);
let file_name = this.read_path_from_wide_str(this.read_pointer(file_name)?)?;
let desired_access = this.read_scalar(desired_access)?.to_u32()?;
let share_mode = this.read_scalar(share_mode)?.to_u32()?;
let security_attributes = this.read_pointer(security_attributes)?;
Expand Down Expand Up @@ -267,19 +280,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

let handle = if is_dir && exists {
let fh = &mut this.machine.fds;
CraftSpider marked this conversation as resolved.
Show resolved Hide resolved
let fd = fh.insert_new(DirHandle { path: file_name.into() });
Ok(Handle::File(fd))
let fd_num = fh.insert_new(DirHandle { path: file_name.into() });
Ok(Handle::File(fd_num))
} else if creation_disposition == open_existing && desired_access == 0 {
// Windows supports handles with no permissions. These allow things such as reading
// metadata, but not file content.
let fh = &mut this.machine.fds;
let fd = fh.insert_new(MetadataHandle { path: file_name.into() });
Ok(Handle::File(fd))
let fd_num = fh.insert_new(MetadataHandle { path: file_name.into() });
Ok(Handle::File(fd_num))
} else {
options.open(file_name).map(|file| {
let fh = &mut this.machine.fds;
let fd = fh.insert_new(FileHandle { file, writable: desired_write });
Handle::File(fd)
let fd_num = fh.insert_new(FileHandle { file, writable: desired_write });
Handle::File(fd_num)
})
};

Expand Down Expand Up @@ -308,13 +321,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
this.windows_ty_layout("BY_HANDLE_FILE_INFORMATION"),
)?;

let fd = if let Handle::File(fd) = file {
fd
let fd_num = if let Handle::File(fd_num) = file {
fd_num
} else {
this.invalid_handle("GetFileInformationByHandle")?
};

let Some(desc) = this.machine.fds.get(fd) else {
let Some(desc) = this.machine.fds.get(fd_num) else {
this.invalid_handle("GetFileInformationByHandle")?
};

Expand Down Expand Up @@ -391,13 +404,3 @@ fn write_filetime_field<'tcx>(
&cx.project_field_named(val, name)?,
)
}

fn local_path(path: &str) -> Cow<'_, Path> {
if cfg!(windows) {
Cow::Borrowed(path.as_ref())
} else {
let stripped = if path.starts_with(r"\\?\") { &path[3..] } else { path };
let path = PathBuf::from(stripped.replace("\\", "/"));
Cow::Owned(path)
}
}
36 changes: 25 additions & 11 deletions src/shims/windows/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ use rustc_abi::HasDataLayout;
use crate::concurrency::thread::ThreadNotFound;
use crate::*;

/// Internal type of a file-descriptor - this is what [`FdTable`](shims::files::FdTable) expects
type FdNum = i32;
CraftSpider marked this conversation as resolved.
Show resolved Hide resolved

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub enum PseudoHandle {
CurrentThread,
Expand All @@ -17,7 +20,7 @@ pub enum Handle {
Null,
Pseudo(PseudoHandle),
Thread(ThreadId),
File(i32),
File(FdNum),
Invalid,
}

Expand Down Expand Up @@ -51,6 +54,8 @@ impl Handle {
const PSEUDO_DISCRIMINANT: u32 = 1;
const THREAD_DISCRIMINANT: u32 = 2;
const FILE_DISCRIMINANT: u32 = 3;
// Chosen to ensure Handle::Invalid encodes to -1. Update this value if there are ever more than
// 8 discriminants.
const INVALID_DISCRIMINANT: u32 = 7;
CraftSpider marked this conversation as resolved.
Show resolved Hide resolved

fn discriminant(self) -> u32 {
Expand All @@ -70,20 +75,25 @@ impl Handle {
Self::Thread(thread) => thread.to_u32(),
#[expect(clippy::cast_sign_loss)]
Self::File(fd) => fd as u32,
// INVALID_HANDLE_VALUE is -1. This fact is explicitly declared or implied in several
// pages of Windows documentation.
// 1: https://learn.microsoft.com/en-us/dotnet/api/microsoft.win32.safehandles.safefilehandle?view=net-9.0
// 2: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle?view=msvc-170
Self::Invalid => 0x1FFFFFFF,
CraftSpider marked this conversation as resolved.
Show resolved Hide resolved
}
}

fn packed_disc_size() -> u32 {
// ceil(log2(x)) is how many bits it takes to store x numbers
// We ensure that INVALID_HANDLE_VALUE (0xFFFFFFFF) decodes to Handle::Invalid
// see https://devblogs.microsoft.com/oldnewthing/20230914-00/?p=108766
// ceil(log2(x)) is how many bits it takes to store x numbers.
// We ensure that INVALID_HANDLE_VALUE (0xFFFFFFFF) decodes to Handle::Invalid.
// see https://devblogs.microsoft.com/oldnewthing/20230914-00/?p=108766 for more detail on
// INVALID_HANDLE_VALUE.
let variant_count = variant_count::<Self>();

// however, std's ilog2 is floor(log2(x))
// However, std's ilog2 is floor(log2(x))
let floor_log2 = variant_count.ilog2();

// we need to add one for non powers of two to compensate for the difference
// We need to add one for non powers of two to compensate for the difference.
#[expect(clippy::arithmetic_side_effects)] // cannot overflow
if variant_count.is_power_of_two() { floor_log2 } else { floor_log2 + 1 }
}
Expand All @@ -105,7 +115,7 @@ impl Handle {
assert!(discriminant < 2u32.pow(disc_size));

// make sure the data fits into `data_size` bits
assert!(data <= 2u32.pow(data_size));
assert!(data < 2u32.pow(data_size));

// packs the data into the lower `data_size` bits
// and packs the discriminant right above the data
Expand All @@ -118,7 +128,11 @@ impl Handle {
Self::PSEUDO_DISCRIMINANT => Some(Self::Pseudo(PseudoHandle::from_value(data)?)),
Self::THREAD_DISCRIMINANT => Some(Self::Thread(ThreadId::new_unchecked(data))),
#[expect(clippy::cast_possible_wrap)]
Self::FILE_DISCRIMINANT => Some(Self::File(data as i32)),
Self::FILE_DISCRIMINANT => {
// This cast preserves all bits.
assert_eq!(size_of_val(&data), size_of::<FdNum>());
Some(Self::File(data as FdNum))
}
Self::INVALID_DISCRIMINANT => Some(Self::Invalid),
_ => None,
}
Expand Down Expand Up @@ -221,9 +235,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
this.detach_thread(thread, /*allow_terminated_joined*/ true)?;
this.eval_windows("c", "TRUE")
}
Handle::File(fd) =>
if let Some(file) = this.machine.fds.get(fd) {
let err = file.close(this.machine.communicate(), this)?;
Handle::File(fd_num) =>
if let Some(fd) = this.machine.fds.remove(fd_num) {
let err = fd.close(this.machine.communicate(), this)?;
if let Err(e) = err {
this.set_last_error(e)?;
this.eval_windows("c", "FALSE")
Expand Down
Loading