Repository navigation
Fix backing-file offset truncation at 4 GiB - #129
Conversation
Convert page offsets to off_t before multiplying by the page size in span reset, mesh finalization, and child-after-fork remapping. Verify that off_t can represent the arena offsets at compile time. Add boundary regression checks for both 4 KiB and 16 KiB pages.
|
@bpowers, would you take a look? Audit summary: The bug is real and corrupts data.
#131 adds an end-to-end test that meshes and scavenges spans above 4 GiB. On current master the meshed object reads back as an empty string instead of its contents. With this PR it passes. The fix is correct and complete. I checked the other places that turn page offsets into byte offsets:
The only other unwidened shift is in a CI (run 39353503541): passes on Ubuntu (CMake and Bazel) and macOS (Bazel). The new I'd merge this as is. #131 stacks the end-to-end test and a small |
|
thanks! this LGTM - I was initially concerned that moving from bit shifting to a multiply would slow things down, but codegen is the same |
Mesh stores page offsets in a 32-bit
Offset. The remapping paths shift that value before converting it tooff_t, so page offset0x100000with 4 KiB pages becomes byte offset 0 instead of0x100000000. Anmmapcall can therefore succeed while referring to the wrong part of the arena backing file.Widen both operands to
off_tbefore multiplication and use the shared conversion in span reset, mesh finalization, and child-after-fork remapping. A compile-time check verifies thatoff_tcan represent the arena's backing-file offsets. The stored offset type and arena capacity stay the same.Add a small regression to the existing unit-test target, checking immediately below, at, and above 4 GiB for both 4 KiB and 16 KiB pages. Restoring the original shift produces four expected assertion failures; the fixed conversion passes all six checks without a large allocation.
Local validation on Linux x86_64 with Bazel 7.4.1, based on public upstream master
2987f883b8695c07a80dc40da67c61f524460dfd://src:unit-teststarget: in this environment, unmodified upstream passes 49/50 tests and this patch passes 50/51, in both modes. Both fail only the existing publicForkTest.RuntimePidInitializedtest (runtime PID is 0). The additional test is the new offset regression above.//src:libmesh.so: builds successfully.git diff --check: passes.The 16 KiB checks exercise the arithmetic on Linux; macOS runtime execution was not tested.