Fix 64-bit file offsets on Windows - #348
Conversation
On Windows, C `off_t` is `long`, i.e. 32-bit, so it cannot represent file offsets or sizes at or beyond 2 GiB. The offset typealias `_COffT` was unconditionally `off_t`, so `FileDescriptor.seek`, `read`/`write` at an absolute offset, and `resize` funneled a public `Int64` through a 32-bit conversion. Because Swift's numeric conversions are range-checked, any offset >= 2^31 trapped at runtime, and positioned I/O could never target a region past 4 GiB (the `OVERLAPPED` high dword was always zero). Make `_COffT` 64-bit on Windows and thread it through `system_lseek`, `system_pread`, `system_pwrite`, and `system_ftruncate` (unchanged on other platforms, where `_COffT` remains `off_t`). The Windows adapters already had a 64-bit `lseek` overload backed by `_lseeki64`; the now-redundant 32-bit `_lseek` overload is removed so overload resolution selects it. The `OVERLAPPED` offset split now uses truncating conversions, since `DWORD(UInt32(offset))` would itself trap once offsets exceed 4 GiB.
47c4fd6 to
cca1aac
Compare
| // 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 |
There was a problem hiding this comment.
I think I would slightly prefer _off64_t as the spelling. This is still Int64 under the hood, but more explicit and uses the libc spelling.
| @inline(__always) | ||
| internal func pread( | ||
| _ fd: Int32, _ buf: UnsafeMutableRawPointer!, _ nbyte: Int, _ offset: off_t | ||
| _ fd: Int32, _ buf: UnsafeMutableRawPointer!, _ nbyte: Int, _ offset: Int64 |
There was a problem hiding this comment.
These can then use the same spelling
There was a problem hiding this comment.
Thanks for this contribution. Could you edit the very wordy comments I reviewed as well as those in the new test to be shorter and more self-contained? There is a reference in the test comment to "the fix", which won't be a useful comment for future readers. "[something about specific values]. Both trapped before the fix." -> "Two values not representable by a 32-bit off_t."
Please target the release/1.7.0 release/1.8.x branch, as this is clearly a bug fix.
| // 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. |
There was a problem hiding this comment.
Could you make this comment less wordy? It's probably enough to say that we use a 64-bit value because off_t is a 32-bit type on Windows.
| // Split the 64-bit offset into high/low DWORDs. Use truncating conversions: | ||
| // `DWORD(UInt32(offset))` would trap for any offset >= 4 GiB. |
There was a problem hiding this comment.
This comment isn't necessary. The truncating API is wordy for a reason!
| // Split the 64-bit offset into high/low DWORDs. Use truncating conversions: | ||
| // `DWORD(UInt32(offset))` would trap for any offset >= 4 GiB. |
| DeviceIoControl(handle, FSCTL_SET_SPARSE, nil, 0, nil, 0, | ||
| &bytesReturned, nil), |
There was a problem hiding this comment.
Our coding style is to have all the parameters on one line, or to have one line per parameter.
| DeviceIoControl(handle, FSCTL_SET_SPARSE, nil, 0, nil, 0, | |
| &bytesReturned, nil), | |
| DeviceIoControl( | |
| handle, FSCTL_SET_SPARSE, nil, 0, nil, 0, &bytesReturned, nil | |
| ), |
3 participants
Summary
On Windows, C
off_tislong, i.e. 32-bit, so it cannot represent file offsets or sizes at or beyond 2 GiB. The internal offset typealias_COffTwas unconditionallyoff_t, soFileDescriptor.seek,read/writeat an absolute offset, andresizefunneled a publicInt64through a 32-bit conversion.Because Swift's numeric conversions are range-checked:
>= 2^31trapped at runtime (process crash), so seeking, positioned reads/writes, or resizing anywhere past 2 GiB was impossible.OVERLAPPEDhigh dword was computed from an already-32-bit value and was therefore always zero.The Windows adapters already contained a 64-bit
lseekoverload backed by_lseeki64and anOVERLAPPED/LARGE_INTEGERsplit, which is strong evidence the 64-bit path was intended and only the plumbing type was wrong.Fix
_COffT64-bit on Windows (Int64); it remainsoff_ton all other platforms, so there is no behavioral change off Windows._COffTthroughsystem_lseek,system_pread,system_pwrite, andsystem_ftruncate._lseekoverload so overload resolution selects the existing_lseeki64-backed one.OVERLAPPEDoffset split, sinceDWORD(UInt32(offset))would itself trap once offsets exceed 4 GiB.Testing
testLargeFileOffsets(Windows), which seeks past 2 GiB and 4 GiB and performs a positioned write/read round-trip beyond 4 GiB using a sparse file so it allocates no real storage.Int32.max) and passes with it.swift testsuite green on Windows (x86_64-unknown-windows-msvc, Swift 6.3.3); no changes to other platforms.