Fix review

- Add test for multiple file flags
- Add comments to some code

Author: CraftSpider

src/shims/windows/fs.rs

@@ -204,6 +204,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             throw_unsup_format!("CreateFileW: Template files are not supported");
         }
 
+        // We need to know if the file is a directory to correctly open directory handles.
+        // This is racy, but currently the stdlib doesn't appear to offer a better solution.
         let is_dir = file_name.is_dir();
 
         // BACKUP_SEMANTICS is how Windows calls the act of opening a directory handle.
@@ -253,7 +255,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 }
             }
             CreateNew => {
-                // See comments in
                 options.create_new(true);
                 // Per `create_new` documentation:
                 // The file must be opened with write or append access in order to create a new file.
@@ -342,6 +343,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             this.eval_windows_u32("c", "FILE_ATTRIBUTE_DEVICE")
         };
 
+        // Per the Windows documentation:
+        // "If the underlying file system does not support the [...] time, this member is zero (0)."
+        // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/ns-fileapi-by_handle_file_information
         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));

src/shims/windows/handle.rs

@@ -198,7 +198,7 @@ impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
 #[allow(non_snake_case)]
 pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     /// Convert a scalar into a structured `Handle`.
-    /// If the handle is invalid, or references a non-existent item, this returns a machine abort.
+    /// If the handle is invalid, or references a non-existent item, execution is aborted.
     #[track_caller]
     fn read_handle(&self, handle: &OpTy<'tcx>, function_name: &str) -> InterpResult<'tcx, Handle> {
         let this = self.eval_context_ref();

tests/pass-dep/shims/windows-fs.rs

@@ -14,8 +14,9 @@ use windows_sys::Win32::Foundation::{
 };
 use windows_sys::Win32::Storage::FileSystem::{
     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_ALWAYS, OPEN_EXISTING,
+    FILE_ATTRIBUTE_NORMAL, FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OPEN_REPARSE_POINT,
+    FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, GetFileInformationByHandle, OPEN_ALWAYS,
+    OPEN_EXISTING,
 };
 
 fn main() {
@@ -164,6 +165,31 @@ unsafe fn test_open_always_twice() {
     };
 }
 
+// TODO: Once we support more of the std API, it would be nice to test against an actual symlink
+unsafe fn test_open_dir_reparse() {
+    let temp = utils::tmp();
+    let raw_path = to_wide_cstr(&temp);
+    // Open the `temp` directory.
+    let handle = CreateFileW(
+        raw_path.as_ptr(),
+        GENERIC_READ,
+        FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
+        ptr::null_mut(),
+        OPEN_EXISTING,
+        FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT,
+        0,
+    );
+    assert_ne!(handle, -1, "CreateFileW Failed: {}", GetLastError());
+    let mut info = std::mem::zeroed::<BY_HANDLE_FILE_INFORMATION>();
+    if GetFileInformationByHandle(handle, &mut info) == 0 {
+        panic!("Failed to get file information")
+    };
+    assert!(info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY != 0);
+    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]);