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 3 commits
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
36 changes: 26 additions & 10 deletions src/shims/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,16 +218,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

let filetime = this.deref_pointer_as(LPFILETIME_op, this.windows_ty_layout("FILETIME"))?;

let NANOS_PER_SEC = this.eval_windows_u64("time", "NANOS_PER_SEC");
let INTERVALS_PER_SEC = this.eval_windows_u64("time", "INTERVALS_PER_SEC");
let INTERVALS_TO_UNIX_EPOCH = this.eval_windows_u64("time", "INTERVALS_TO_UNIX_EPOCH");
let NANOS_PER_INTERVAL = NANOS_PER_SEC / INTERVALS_PER_SEC;
let SECONDS_TO_UNIX_EPOCH = INTERVALS_TO_UNIX_EPOCH / INTERVALS_PER_SEC;

let duration = system_time_to_duration(&SystemTime::now())?
+ Duration::from_secs(SECONDS_TO_UNIX_EPOCH);
let duration_ticks = u64::try_from(duration.as_nanos() / u128::from(NANOS_PER_INTERVAL))
.map_err(|_| err_unsup_format!("programs running more than 2^64 Windows ticks after the Windows epoch are not supported"))?;
let duration = this.system_time_since_windows_epoch(&SystemTime::now())?;
let duration_ticks = this.windows_ticks_for(duration)?;

let dwLowDateTime = u32::try_from(duration_ticks & 0x00000000FFFFFFFF).unwrap();
let dwHighDateTime = u32::try_from((duration_ticks & 0xFFFFFFFF00000000) >> 32).unwrap();
Expand Down Expand Up @@ -276,6 +268,30 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
interp_ok(Scalar::from_i32(-1)) // Return non-zero on success
}

#[allow(non_snake_case, clippy::arithmetic_side_effects)]
fn system_time_since_windows_epoch(&self, time: &SystemTime) -> InterpResult<'tcx, Duration> {
let this = self.eval_context_ref();

let INTERVALS_PER_SEC = this.eval_windows_u64("time", "INTERVALS_PER_SEC");
let INTERVALS_TO_UNIX_EPOCH = this.eval_windows_u64("time", "INTERVALS_TO_UNIX_EPOCH");
let SECONDS_TO_UNIX_EPOCH = INTERVALS_TO_UNIX_EPOCH / INTERVALS_PER_SEC;

interp_ok(system_time_to_duration(time)? + Duration::from_secs(SECONDS_TO_UNIX_EPOCH))
}

#[allow(non_snake_case, clippy::arithmetic_side_effects)]
fn windows_ticks_for(&self, duration: Duration) -> InterpResult<'tcx, u64> {
let this = self.eval_context_ref();

let NANOS_PER_SEC = this.eval_windows_u64("time", "NANOS_PER_SEC");
let INTERVALS_PER_SEC = this.eval_windows_u64("time", "INTERVALS_PER_SEC");
let NANOS_PER_INTERVAL = NANOS_PER_SEC / INTERVALS_PER_SEC;

let ticks = u64::try_from(duration.as_nanos() / u128::from(NANOS_PER_INTERVAL))
.map_err(|_| err_unsup_format!("programs running more than 2^64 Windows ticks after the Windows epoch are not supported"))?;
interp_ok(ticks)
}

fn mach_absolute_time(&self) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_ref();

Expand Down
54 changes: 36 additions & 18 deletions src/shims/windows/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use std::path::PathBuf;
use std::time::SystemTime;

use crate::shims::files::FileDescription;
use crate::shims::time::system_time_to_duration;
use crate::shims::windows::handle::{EvalContextExt as _, Handle};
use crate::*;

Expand Down Expand Up @@ -177,7 +176,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
throw_unsup_format!("CreateFileW: Template files are not supported");
}

let exists = file_name.exists();
let is_dir = file_name.is_dir();

if flags_and_attributes == file_attribute_normal && is_dir {
Expand All @@ -204,20 +202,46 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
);
}

// This is racy, but there doesn't appear to be an std API that both succeeds if a file
// exists but tells us it isn't new. Either we accept racing one way or another, or we
// use an iffy heuristic like file creation time. This implementation prefers to fail in the
// direction of erroring more often.
let exists = file_name.exists();
Copy link
Member

Choose a reason for hiding this comment

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

This comment is too early, since one doesn't know yet that this is used for. IMO it'd be better placed at the if exists below.

(Also, no need to let-bind, I don't feel like this makes things much clearer.)


if creation_disposition == create_always {
if file_name.exists() {
// Per the documentation:
// If the specified file exists and is writable, the function truncates the file, the
// function succeeds, and last-error code is set to ERROR_ALREADY_EXISTS.
// If the specified file does not exist and is a valid path, a new file is created, the
// function succeeds, and the last-error code is set to zero.
// https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew
if exists {
this.set_last_error(IoError::WindowsError("ERROR_ALREADY_EXISTS"))?;
} else {
this.set_last_error(IoError::Raw(Scalar::from_u32(0)))?;
}
options.create(true);
options.truncate(true);
} else if creation_disposition == create_new {
options.create_new(true);
// Per `create_new` documentation:
// If the specified file does not exist and is a valid path to a writable location, the
// function creates a file and the last-error code is set to zero.
Copy link
Member

Choose a reason for hiding this comment

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

The setting to zero does not seem to happen here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the wrong copy-paste - I meant to put the std docs for create_new here. Microsoft doesn't say anything about zero error here

Copy link
Member

Choose a reason for hiding this comment

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

If you have access to a Windows host, maybe you can try this:

  • make sure the last error is non-zero
  • then call CreateFile in one of the cases where the docs say nothing about the last error, and see what the last error looks like when the function returns successfully

// https://doc.rust-lang.org/std/fs/struct.OpenOptions.html#method.create_new
if !desired_write {
options.append(true);
}
} else if creation_disposition == open_always {
if file_name.exists() {
// Per the documentation:
// If the specified file exists, the function succeeds and the last-error code is set
// to ERROR_ALREADY_EXISTS.
// If the specified file does not exist and is a valid path to a writable location, the
// function creates a file and the last-error code is set to zero.
// https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew
if exists {
this.set_last_error(IoError::WindowsError("ERROR_ALREADY_EXISTS"))?;
} else {
this.set_last_error(IoError::Raw(Scalar::from_u32(0)))?;
}
options.create(true);
} else if creation_disposition == open_existing {
Expand All @@ -230,7 +254,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
);
}

let handle = if is_dir && exists {
let handle = if is_dir {
let fh = &mut this.machine.fds;
let fd_num = fh.insert_new(DirHandle { path: file_name });
Ok(Handle::File(fd_num))
Expand Down Expand Up @@ -304,9 +328,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
this.eval_windows_u32("c", "FILE_ATTRIBUTE_DEVICE")
};

let created = extract_windows_epoch(metadata.created())?.unwrap_or((0, 0));
let accessed = extract_windows_epoch(metadata.accessed())?.unwrap_or((0, 0));
let written = extract_windows_epoch(metadata.modified())?.unwrap_or((0, 0));
let created = extract_windows_epoch(this, metadata.created())?.unwrap_or((0, 0));
let accessed = extract_windows_epoch(this, metadata.accessed())?.unwrap_or((0, 0));
let written = extract_windows_epoch(this, metadata.modified())?.unwrap_or((0, 0));

this.write_int_fields_named(&[("dwFileAttributes", attributes.into())], &file_information)?;
write_filetime_field(this, &file_information, "ftCreationTime", created)?;
Expand All @@ -330,21 +354,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

/// Windows FILETIME is measured in 100-nanosecs since 1601
fn extract_windows_epoch<'tcx>(
ecx: &MiriInterpCx<'tcx>,
time: io::Result<SystemTime>,
) -> InterpResult<'tcx, Option<(u32, u32)>> {
// (seconds in a year * years between 1970 and 1601) + (89 leap days in that span)
const SECONDS_TO_EPOCH: u64 = 3600 * 24 * 365 * (1970 - 1601) + 3600 * 24 * 89;
// seconds between unix and windows epochs * 10 million (nanoseconds/second / 100)
const TIME_TO_EPOCH: u64 = SECONDS_TO_EPOCH * 10_000_000;
match time.ok() {
Some(time) => {
let duration = system_time_to_duration(&time)?;
// 10 million is the number of 100ns periods per second.
let secs = duration.as_secs().saturating_mul(10_000_000);
let nanos_hundred: u64 = (duration.subsec_nanos() / 100).into();
let total = secs.saturating_add(nanos_hundred).saturating_add(TIME_TO_EPOCH);
let duration = ecx.system_time_since_windows_epoch(&time)?;
let duration_ticks = ecx.windows_ticks_for(duration)?;
#[allow(clippy::cast_possible_truncation)]
interp_ok(Some((total as u32, (total >> 32) as u32)))
interp_ok(Some((duration_ticks as u32, (duration_ticks >> 32) as u32)))
}
None => interp_ok(None),
}
Expand Down
11 changes: 11 additions & 0 deletions src/shims/windows/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
interp_ok(ret)
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_invalid_encoding() {
// Ensure the invalid handle encodes to `u32::MAX`/`INVALID_HANDLE_VALUE`.
assert_eq!(Handle::Invalid.to_packed(), 0xFFFFFFFF)
CraftSpider marked this conversation as resolved.
Show resolved Hide resolved
}
}
80 changes: 77 additions & 3 deletions tests/pass-dep/shims/windows-fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,21 @@ use std::ptr;
#[path = "../../utils/mod.rs"]
mod utils;

use windows_sys::Win32::Foundation::{CloseHandle, GENERIC_READ, GENERIC_WRITE, GetLastError};
use windows_sys::Win32::Foundation::{
CloseHandle, ERROR_ALREADY_EXISTS, GENERIC_READ, GENERIC_WRITE, GetLastError,
};
use windows_sys::Win32::Storage::FileSystem::{
BY_HANDLE_FILE_INFORMATION, CREATE_NEW, CreateFileW, FILE_ATTRIBUTE_DIRECTORY,
BY_HANDLE_FILE_INFORMATION, CREATE_ALWAYS, CREATE_NEW, CreateFileW, FILE_ATTRIBUTE_DIRECTORY,
FILE_ATTRIBUTE_NORMAL, FILE_FLAG_BACKUP_SEMANTICS, FILE_SHARE_DELETE, FILE_SHARE_READ,
FILE_SHARE_WRITE, GetFileInformationByHandle, OPEN_EXISTING,
FILE_SHARE_WRITE, GetFileInformationByHandle, OPEN_ALWAYS, OPEN_EXISTING,
};

fn main() {
unsafe {
test_create_dir_file();
test_create_normal_file();
test_create_always_twice();
test_open_always_twice();
}
}

Expand Down Expand Up @@ -89,6 +93,76 @@ unsafe fn test_create_normal_file() {
};
}

/// Tests that CREATE_ALWAYS sets the error value correctly based on whether the file already exists
unsafe fn test_create_always_twice() {
let temp = utils::tmp().join("test_create_always.txt");
let raw_path = to_wide_cstr(&temp);
let handle = CreateFileW(
raw_path.as_ptr(),
GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
ptr::null_mut(),
CREATE_ALWAYS,
0,
0,
);
assert_ne!(handle, -1, "CreateFileW Failed: {}", GetLastError());
assert_eq!(GetLastError(), 0);
if CloseHandle(handle) == 0 {
panic!("Failed to close file")
};

let handle = CreateFileW(
raw_path.as_ptr(),
GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
ptr::null_mut(),
CREATE_ALWAYS,
0,
0,
);
assert_ne!(handle, -1, "CreateFileW Failed: {}", GetLastError());
assert_eq!(GetLastError(), ERROR_ALREADY_EXISTS);
if CloseHandle(handle) == 0 {
panic!("Failed to close file")
};
}

/// Tests that OPEN_ALWAYS sets the error value correctly based on whether the file already exists
unsafe fn test_open_always_twice() {
let temp = utils::tmp().join("test_open_always.txt");
let raw_path = to_wide_cstr(&temp);
let handle = CreateFileW(
raw_path.as_ptr(),
GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
ptr::null_mut(),
OPEN_ALWAYS,
0,
0,
);
assert_ne!(handle, -1, "CreateFileW Failed: {}", GetLastError());
assert_eq!(GetLastError(), 0);
if CloseHandle(handle) == 0 {
panic!("Failed to close file")
};

let handle = CreateFileW(
raw_path.as_ptr(),
GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
ptr::null_mut(),
OPEN_ALWAYS,
0,
0,
);
assert_ne!(handle, -1, "CreateFileW Failed: {}", GetLastError());
assert_eq!(GetLastError(), ERROR_ALREADY_EXISTS);
if CloseHandle(handle) == 0 {
panic!("Failed to close file")
};
}

fn to_wide_cstr(path: &Path) -> Vec<u16> {
let mut raw_path = path.as_os_str().encode_wide().collect::<Vec<_>>();
raw_path.extend([0, 0]);
Expand Down
Loading