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: FILE_DISPOSITION_POSIX_SEMANTICS is not supported on FAT filesystems #16497

Closed
squeek502 opened this issue Jul 23, 2023 · 1 comment · Fixed by #16499
Closed

windows: FILE_DISPOSITION_POSIX_SEMANTICS is not supported on FAT filesystems #16497

squeek502 opened this issue Jul 23, 2023 · 1 comment · Fixed by #16499
Labels
bug Observed behavior contradicts documented or intended behavior os-windows standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@squeek502
Copy link
Collaborator

squeek502 commented Jul 23, 2023

Zig Version

0.11.0-dev.4059+17255bed4

Steps to Reproduce and Observed Behavior

#15501 made std.os.windows.DeleteFile use NtSetInformationFile with FILE_DISPOSITION_POSIX_SEMANTICS if builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs1).

However, this flag is only supported if the underlying filesystem is NTFS. On FAT filesystems, calling NtSetInformationFile with FILE_DISPOSITION_POSIX_SEMANTICS results in STATUS_INVALID_PARAMETER:

E:\zigstd\lib\std\os\windows.zig:975:31: 0x7ff6cd6687a1 in DeleteFile (test.exe.obj)
        .INVALID_PARAMETER => unreachable,
                              ^
E:\zigstd\lib\std\os.zig:2463:30: 0x7ff6cd53d593 in unlinkatW (test.exe.obj)
    return windows.DeleteFile(sub_path_w, .{ .dir = dirfd, .remove_dir = remove_dir });
                             ^

To avoid this, it should be possible to use NtQueryVolumeInformationFile before the NtSetInformationFile in std.os.windows.DeleteFile with FileFsAttributeInformation/FILE_FS_ATTRIBUTE_INFORMATION. The FileSystemAttributes can then be checked for FILE_SUPPORTS_POSIX_UNLINK_RENAME.

Tested to make sure the FILE_SUPPORTS_POSIX_UNLINK_RENAME flag works the expected way using the higher level Windows APIs in C:

#include <windows.h>
#include <fileapi.h>
#include <stdio.h>
#include <winnt.h>

int main() {
	HANDLE file = CreateFile("build.zig", GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);

	if (file == INVALID_HANDLE_VALUE) {
		exit(1);
	}

	DWORD flags;
	BOOL result = GetVolumeInformationByHandleW(file, NULL, 0, NULL, NULL, &flags, NULL, 0);
	if (result == 0) {
		exit(1);
	}

	printf("supports posix unlink: %d\n", (flags & FILE_SUPPORTS_POSIX_UNLINK_RENAME) != 0);

	CloseHandle(file);
}

On NTFS:

C:\Users\Ryan\Programming\Zig\tmp> volume.exe
supports posix unlink: 1

On FAT:

E:\zigtest> "C:\Users\Ryan\Programming\Zig\tmp\volume.exe"
supports posix unlink: 0

Expected Behavior

std.fs delete APIs to work on FAT filesystems on Windows

@squeek502 squeek502 added the bug Observed behavior contradicts documented or intended behavior label Jul 23, 2023
@squeek502
Copy link
Collaborator Author

It actually looks like FileDispositionInformationEx as a whole is unsupported on FAT filesystems.

Here's the nttrace of calling DeleteFileW on FAT32:

NtOpenFile( FileHandle=0xc9847ff5e8 [0x90], DesiredAccess=DELETE|0x80, ObjectAttributes=0x48:"blah.txt", IoStatusBlock=0xc9847ff518 [0/1], ShareAccess=7, OpenOptions=0x00204040 ) => 0
NtQueryInformationFile( FileHandle=0x90, IoStatusBlock=0xc9847ff518, FileInformation=0xc9847ff510, Length=8, FileInformationClass=0x23 [FileAttributeTagInformation] ) => 0xc000000d [87 'The parameter is incorrect.']
NtSetInformationFile( FileHandle=0x90, IoStatusBlock=0xc9847ff518, FileInformation=0xc9847ff5e0, Length=4, FileInformationClass=0x40 [FileDispositionInformationEx] ) => 0xc000000d [87 'The parameter is incorrect.']
NtSetInformationFile( FileHandle=0x90, IoStatusBlock=0xc9847ff518 [0/0], FileInformation=0xc9847ff5d8, Length=1, FileInformationClass=0xd [FileDispositionInformation] ) => 0
NtClose( Handle=0x90 ) => 0

That is, it tries using FileDispositionInformationEx, gets STATUS_INVALID_PARAMETER, and then falls back to FileDispositionInformation.

So, there seems to be two ways to go:

  1. Do the same thing: try FileDispositionInformationEx first, and then fall back to FileDispositionInformation if we get STATUS_INVALID_PARAMETER. This would make the non-FAT path only need 1 call to NtSetInformationFile but the FAT path to need 2.
  2. Use NtQueryVolumeInformationFile to determine whether or not to use FileDispositionInformationEx or FileDispositionInformation. This would make both the non-FAT and the FAT paths always call both NtQueryVolumeInformationFile and NtSetInformationFile

squeek502 added a commit to squeek502/zig that referenced this issue Jul 23, 2023
… fallback if not

Using FileDispositionInformationEx (and therefore flags like FILE_DISPOSITION_POSIX_SEMANTICS and FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE) is only supported on NTFS, so the comptime Windows version range check is not enough to determine whether or not the NtSetInformationFile will succeed.

This commit makes DeleteFile always try using FileDispositionInformationEx first, but if INVALID_PARAMETER is received (which is the status that occurs when the filesystem doesn't support FileDispositionInformationEx), then it will fallback and try calling NtSetInformationFile with FileDispositionInformation.

This keeps NTFS as fast as it was before, since it will do at most 1 NtSetInformationFile call, but on non-NTFS filesystems (e.g. FAT32), DeleteFile may need to do 2 NtSetInformationFile calls.

Closes ziglang#16497
@andrewrk andrewrk added standard library This issue involves writing Zig code for the standard library. os-windows labels Jul 23, 2023
@andrewrk andrewrk added this to the 0.11.0 milestone Jul 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior os-windows standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants