-
Notifications
You must be signed in to change notification settings - Fork 148
Fix 64-bit file offsets on Windows #348
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,16 @@ import Android | |
| #error("Unsupported Platform") | ||
| #endif | ||
|
|
||
| #if os(Windows) | ||
| // On Windows, C `off_t` is `long`, i.e. 32-bit, so it cannot represent file | ||
| // offsets or sizes at or beyond 2 GiB. Use a 64-bit offset type so that | ||
| // seeking, positioned I/O, and resizing work on large files, matching the | ||
| // 64-bit `off_t` on Linux and Darwin. The Windows syscall adapters route | ||
| // this through the 64-bit `_lseeki64` / `OVERLAPPED` / `LARGE_INTEGER` APIs. | ||
| internal typealias _COffT = Int64 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I would slightly prefer |
||
| #else | ||
| internal typealias _COffT = off_t | ||
| #endif | ||
|
|
||
| // MARK: syscalls and variables | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,13 +121,6 @@ internal func write( | |
| Int(_write(fd, buf, numericCast(nbyte))) | ||
| } | ||
|
|
||
| @inline(__always) | ||
| internal func lseek( | ||
| _ fd: Int32, _ off: off_t, _ whence: Int32 | ||
| ) -> off_t { | ||
| _lseek(fd, off, whence) | ||
| } | ||
|
|
||
| @inline(__always) | ||
| internal func dup(_ fd: Int32) -> Int32 { | ||
| _dup(fd) | ||
|
|
@@ -144,7 +137,7 @@ internal func dup2(_ fd: Int32, _ fd2: Int32) -> Int32 { | |
|
|
||
| @inline(__always) | ||
| internal func pread( | ||
| _ fd: Int32, _ buf: UnsafeMutableRawPointer!, _ nbyte: Int, _ offset: off_t | ||
| _ fd: Int32, _ buf: UnsafeMutableRawPointer!, _ nbyte: Int, _ offset: Int64 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These can then use the same spelling |
||
| ) -> Int { | ||
| let handle: intptr_t = _get_osfhandle(fd) | ||
| if handle == /* INVALID_HANDLE_VALUE */ -1 { ucrt._set_errno(EBADF); return -1 } | ||
|
|
@@ -159,8 +152,10 @@ internal func pread( | |
| let hFile: HANDLE = HANDLE(bitPattern: handle)! | ||
|
|
||
| var ovlOverlapped: OVERLAPPED = OVERLAPPED() | ||
| ovlOverlapped.OffsetHigh = DWORD(UInt32(offset >> 32) & 0xffffffff) | ||
| ovlOverlapped.Offset = DWORD(UInt32(offset >> 0) & 0xffffffff) | ||
| // Split the 64-bit offset into high/low DWORDs. Use truncating conversions: | ||
| // `DWORD(UInt32(offset))` would trap for any offset >= 4 GiB. | ||
|
Comment on lines
+155
to
+156
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment isn't necessary. The truncating API is wordy for a reason! |
||
| ovlOverlapped.OffsetHigh = DWORD(truncatingIfNeeded: offset >> 32) | ||
| ovlOverlapped.Offset = DWORD(truncatingIfNeeded: offset) | ||
|
|
||
| var nNumberOfBytesRead: DWORD = 0 | ||
| if !ReadFile(hFile, buf, DWORD(nbyte), &nNumberOfBytesRead, &ovlOverlapped) { | ||
|
|
@@ -172,7 +167,7 @@ internal func pread( | |
|
|
||
| @inline(__always) | ||
| internal func pwrite( | ||
| _ fd: Int32, _ buf: UnsafeRawPointer!, _ nbyte: Int, _ offset: off_t | ||
| _ fd: Int32, _ buf: UnsafeRawPointer!, _ nbyte: Int, _ offset: Int64 | ||
| ) -> Int { | ||
| let handle: intptr_t = _get_osfhandle(fd) | ||
| if handle == /* INVALID_HANDLE_VALUE */ -1 { ucrt._set_errno(EBADF); return -1 } | ||
|
|
@@ -187,8 +182,10 @@ internal func pwrite( | |
| let hFile: HANDLE = HANDLE(bitPattern: handle)! | ||
|
|
||
| var ovlOverlapped: OVERLAPPED = OVERLAPPED() | ||
| ovlOverlapped.OffsetHigh = DWORD(UInt32(offset >> 32) & 0xffffffff) | ||
| ovlOverlapped.Offset = DWORD(UInt32(offset >> 0) & 0xffffffff) | ||
| // Split the 64-bit offset into high/low DWORDs. Use truncating conversions: | ||
| // `DWORD(UInt32(offset))` would trap for any offset >= 4 GiB. | ||
|
Comment on lines
+185
to
+186
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above |
||
| ovlOverlapped.OffsetHigh = DWORD(truncatingIfNeeded: offset >> 32) | ||
| ovlOverlapped.Offset = DWORD(truncatingIfNeeded: offset) | ||
|
|
||
| var nNumberOfBytesWritten: DWORD = 0 | ||
| if !WriteFile(hFile, buf, DWORD(nbyte), &nNumberOfBytesWritten, | ||
|
|
@@ -214,7 +211,7 @@ internal func csystem_posix_pipe2( | |
| } | ||
|
|
||
| @inline(__always) | ||
| internal func ftruncate(_ fd: Int32, _ length: off_t) -> Int32 { | ||
| internal func ftruncate(_ fd: Int32, _ length: Int64) -> Int32 { | ||
| let handle: intptr_t = _get_osfhandle(fd) | ||
| if handle == /* INVALID_HANDLE_VALUE */ -1 { ucrt._set_errno(EBADF); return -1 } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -328,6 +328,58 @@ final class FileOperationsTestWindows: XCTestCase { | |||||||||||
| } | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /// Regression test for 64-bit file offsets on Windows. | ||||||||||||
| /// | ||||||||||||
| /// C `off_t` is 32-bit on Windows, so before the offset type was widened, | ||||||||||||
| /// seeking or performing positioned I/O at or beyond 2 GiB trapped at | ||||||||||||
| /// runtime, and the `OVERLAPPED` high dword was always zero so offsets past | ||||||||||||
| /// 4 GiB were unreachable. | ||||||||||||
| func testLargeFileOffsets() throws { | ||||||||||||
| try withTemporaryFilePath(basename: "testLargeFileOffsets") { path in | ||||||||||||
| let fd = try FileDescriptor.open( | ||||||||||||
| path.appending("large.bin"), .readWrite, | ||||||||||||
| options: [.create, .truncate], | ||||||||||||
| permissions: .ownerReadWrite | ||||||||||||
| ) | ||||||||||||
| defer { try? fd.close() } | ||||||||||||
|
|
||||||||||||
| // 2 GiB is > Int32.max; 5 GiB is > UInt32.max. Both trapped before the fix. | ||||||||||||
| let twoGiB: Int64 = 1 << 31 | ||||||||||||
| let fiveGiB: Int64 = 5 << 30 | ||||||||||||
|
|
||||||||||||
| // Seeking allocates no storage; it exercises `_lseeki64` and must | ||||||||||||
| // round-trip the full 64-bit position rather than trapping or truncating. | ||||||||||||
| XCTAssertEqual(try fd.seek(offset: twoGiB, from: .start), twoGiB) | ||||||||||||
| XCTAssertEqual(try fd.seek(offset: fiveGiB, from: .start), fiveGiB) | ||||||||||||
|
|
||||||||||||
| // Positioned read/write beyond 4 GiB exercises both dwords of the | ||||||||||||
| // `OVERLAPPED` offset. Mark the file sparse first so the test does not | ||||||||||||
| // allocate several gigabytes of real storage. | ||||||||||||
| let handle = try XCTUnwrap(HANDLE(bitPattern: _get_osfhandle(fd.rawValue))) | ||||||||||||
| var bytesReturned: DWORD = 0 | ||||||||||||
| let FSCTL_SET_SPARSE: DWORD = 0x000900C4 | ||||||||||||
| try XCTSkipUnless( | ||||||||||||
| DeviceIoControl(handle, FSCTL_SET_SPARSE, nil, 0, nil, 0, | ||||||||||||
| &bytesReturned, nil), | ||||||||||||
|
Comment on lines
+363
to
+364
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our coding style is to have all the parameters on one line, or to have one line per parameter.
Suggested change
|
||||||||||||
| "filesystem does not support sparse files" | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| let marker = Array("swift-system".utf8) | ||||||||||||
| let offset = fiveGiB + 123 | ||||||||||||
| let written = try marker.withUnsafeBytes { | ||||||||||||
| try fd.write(toAbsoluteOffset: offset, $0) | ||||||||||||
| } | ||||||||||||
| XCTAssertEqual(written, marker.count) | ||||||||||||
|
|
||||||||||||
| var readBack = [UInt8](repeating: 0, count: marker.count) | ||||||||||||
| let read = try readBack.withUnsafeMutableBytes { | ||||||||||||
| try fd.read(fromAbsoluteOffset: offset, into: $0) | ||||||||||||
| } | ||||||||||||
| XCTAssertEqual(read, marker.count) | ||||||||||||
| XCTAssertEqual(readBack, marker) | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| #endif // os(Windows) | ||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you make this comment less wordy? It's probably enough to say that we use a 64-bit value because
off_tis a 32-bit type on Windows.