From 6d74c0d1b41b52bca3d66092a48660f2ee4ae4f8 Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Wed, 8 Mar 2023 00:57:39 -0800 Subject: [PATCH 1/2] Add comments explaining BUFFER_OVERFLOW during NtQueryInformationFile calls --- lib/std/fs/file.zig | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/std/fs/file.zig b/lib/std/fs/file.zig index bf93a61239..5306909aea 100644 --- a/lib/std/fs/file.zig +++ b/lib/std/fs/file.zig @@ -379,6 +379,9 @@ pub const File = struct { const rc = windows.ntdll.NtQueryInformationFile(self.handle, &io_status_block, &info, @sizeOf(windows.FILE_ALL_INFORMATION), .FileAllInformation); switch (rc) { .SUCCESS => {}, + // Buffer overflow here indicates that there is more information available than was able to be stored in the buffer + // size provided. This is treated as success because the type of variable-length information that this would be relevant for + // (name, volume name, etc) we don't care about. .BUFFER_OVERFLOW => {}, .INVALID_PARAMETER => unreachable, .ACCESS_DENIED => return error.AccessDenied, @@ -830,6 +833,9 @@ pub const File = struct { const rc = windows.ntdll.NtQueryInformationFile(self.handle, &io_status_block, &info, @sizeOf(windows.FILE_ALL_INFORMATION), .FileAllInformation); switch (rc) { .SUCCESS => {}, + // Buffer overflow here indicates that there is more information available than was able to be stored in the buffer + // size provided. This is treated as success because the type of variable-length information that this would be relevant for + // (name, volume name, etc) we don't care about. .BUFFER_OVERFLOW => {}, .INVALID_PARAMETER => unreachable, .ACCESS_DENIED => return error.AccessDenied, From 93b35c69998398e962bff84f3e5006afa122fbde Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Wed, 8 Mar 2023 01:18:33 -0800 Subject: [PATCH 2/2] os.isCygwinPty: Fix a bug, replace kernel32 call, and optimize - Fixes the first few code units of the name being omitted (it was using `@sizeOf(FILE_NAME_INFO)` as the start of the name bytes, but that includes the length of the dummy [1]u16 field and padding; instead the start should be the offset of the dummy [1]u16 field) - Replaces kernel32.GetFileInformationByHandleEx call with ntdll.NtQueryInformationFile + Contributes towards #1840 - Checks that the handle is a named pipe first before querying and checking the name, which is a much faster call than NtQueryInformationFile (this was about a 10x speedup in my probably-not-so-good/take-it-with-a-grain-of-salt benchmarking) --- lib/std/os.zig | 48 +++++++++++++++++++++++++++--------- lib/std/os/windows.zig | 23 +++++++++++++++++ lib/std/os/windows/ntdll.zig | 9 +++++++ 3 files changed, 69 insertions(+), 11 deletions(-) diff --git a/lib/std/os.zig b/lib/std/os.zig index 821c544cc8..f69532cc49 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -3217,22 +3217,48 @@ pub fn isatty(handle: fd_t) bool { pub fn isCygwinPty(handle: fd_t) bool { if (builtin.os.tag != .windows) return false; - const size = @sizeOf(windows.FILE_NAME_INFO); - var name_info_bytes align(@alignOf(windows.FILE_NAME_INFO)) = [_]u8{0} ** (size + windows.MAX_PATH); + // If this is a MSYS2/cygwin pty, then it will be a named pipe with a name in one of these formats: + // msys-[...]-ptyN-[...] + // cygwin-[...]-ptyN-[...] + // + // Example: msys-1888ae32e00d56aa-pty0-to-master - if (windows.kernel32.GetFileInformationByHandleEx( - handle, - windows.FileNameInfo, - @ptrCast(*anyopaque, &name_info_bytes), - name_info_bytes.len, - ) == 0) { - return false; + // First, just check that the handle is a named pipe. + // This allows us to avoid the more costly NtQueryInformationFile call + // for handles that aren't named pipes. + { + var io_status: windows.IO_STATUS_BLOCK = undefined; + var device_info: windows.FILE_FS_DEVICE_INFORMATION = undefined; + const rc = windows.ntdll.NtQueryVolumeInformationFile(handle, &io_status, &device_info, @sizeOf(windows.FILE_FS_DEVICE_INFORMATION), .FileFsDeviceInformation); + switch (rc) { + .SUCCESS => {}, + else => return false, + } + if (device_info.DeviceType != windows.FILE_DEVICE_NAMED_PIPE) return false; + } + + const name_bytes_offset = @offsetOf(windows.FILE_NAME_INFO, "FileName"); + // `NAME_MAX` UTF-16 code units (2 bytes each) + // Note: This buffer may not be long enough to handle *all* possible paths (PATH_MAX_WIDE would be necessary for that), + // but because we only care about certain paths and we know they must be within a reasonable length, + // we can use this smaller buffer and just return false on any error from NtQueryInformationFile. + const num_name_bytes = windows.MAX_PATH * 2; + var name_info_bytes align(@alignOf(windows.FILE_NAME_INFO)) = [_]u8{0} ** (name_bytes_offset + num_name_bytes); + + var io_status_block: windows.IO_STATUS_BLOCK = undefined; + const rc = windows.ntdll.NtQueryInformationFile(handle, &io_status_block, &name_info_bytes, @intCast(u32, name_info_bytes.len), .FileNameInformation); + switch (rc) { + .SUCCESS => {}, + .INVALID_PARAMETER => unreachable, + else => return false, } const name_info = @ptrCast(*const windows.FILE_NAME_INFO, &name_info_bytes[0]); - const name_bytes = name_info_bytes[size .. size + @as(usize, name_info.FileNameLength)]; + const name_bytes = name_info_bytes[name_bytes_offset .. name_bytes_offset + @as(usize, name_info.FileNameLength)]; const name_wide = mem.bytesAsSlice(u16, name_bytes); - return mem.indexOf(u16, name_wide, &[_]u16{ 'm', 's', 'y', 's', '-' }) != null or + // Note: The name we get from NtQueryInformationFile will be prefixed with a '\', e.g. \msys-1888ae32e00d56aa-pty0-to-master + return (mem.startsWith(u16, name_wide, &[_]u16{ '\\', 'm', 's', 'y', 's', '-' }) or + mem.startsWith(u16, name_wide, &[_]u16{ '\\', 'c', 'y', 'g', 'w', 'i', 'n', '-' })) and mem.indexOf(u16, name_wide, &[_]u16{ '-', 'p', 't', 'y' }) != null; } diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index b63fdb9f92..23aa378e9e 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -2472,6 +2472,29 @@ pub const FILE_INFORMATION_CLASS = enum(c_int) { FileMaximumInformation, }; +pub const FILE_FS_DEVICE_INFORMATION = extern struct { + DeviceType: DEVICE_TYPE, + Characteristics: ULONG, +}; + +pub const FS_INFORMATION_CLASS = enum(c_int) { + FileFsVolumeInformation = 1, + FileFsLabelInformation, + FileFsSizeInformation, + FileFsDeviceInformation, + FileFsAttributeInformation, + FileFsControlInformation, + FileFsFullSizeInformation, + FileFsObjectIdInformation, + FileFsDriverPathInformation, + FileFsVolumeFlagsInformation, + FileFsSectorSizeInformation, + FileFsDataCopyInformation, + FileFsMetadataSizeInformation, + FileFsFullSizeInformationEx, + FileFsMaximumInformation, +}; + pub const OVERLAPPED = extern struct { Internal: ULONG_PTR, InternalHigh: ULONG_PTR, diff --git a/lib/std/os/windows/ntdll.zig b/lib/std/os/windows/ntdll.zig index 8c5260c1bc..58cba356a2 100644 --- a/lib/std/os/windows/ntdll.zig +++ b/lib/std/os/windows/ntdll.zig @@ -18,6 +18,7 @@ const IO_STATUS_BLOCK = windows.IO_STATUS_BLOCK; const LARGE_INTEGER = windows.LARGE_INTEGER; const OBJECT_INFORMATION_CLASS = windows.OBJECT_INFORMATION_CLASS; const FILE_INFORMATION_CLASS = windows.FILE_INFORMATION_CLASS; +const FS_INFORMATION_CLASS = windows.FS_INFORMATION_CLASS; const UNICODE_STRING = windows.UNICODE_STRING; const RTL_OSVERSIONINFOW = windows.RTL_OSVERSIONINFOW; const FILE_BASIC_INFORMATION = windows.FILE_BASIC_INFORMATION; @@ -232,6 +233,14 @@ pub extern "ntdll" fn NtQueryObject( ReturnLength: ?*ULONG, ) callconv(WINAPI) NTSTATUS; +pub extern "ntdll" fn NtQueryVolumeInformationFile( + FileHandle: HANDLE, + IoStatusBlock: *IO_STATUS_BLOCK, + FsInformation: *anyopaque, + Length: ULONG, + FsInformationClass: FS_INFORMATION_CLASS, +) callconv(WINAPI) NTSTATUS; + pub extern "ntdll" fn RtlWakeAddressAll( Address: ?*const anyopaque, ) callconv(WINAPI) void;