runtime: unify stat identity and add bounded file-backed MAP_PRIVATE mmap (Batch 32S) - #95
runtime: unify stat identity and add bounded file-backed MAP_PRIVATE mmap (Batch 32S)#95thanks-cohn wants to merge 2 commits into
Conversation
|
Review: I am holding the merge for a focused cleanup pass, not because the Batch 32S direction is wrong. What looks good:
Before merge, please repair these items on the existing PR branch:
Keep the causal frontier unchanged: do not enlarge the backing bound or implement fixed mappings in this cleanup. After the above is green and persisted, the next run should start from the observed 950-page libcrypto private-backing capacity blocker. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e68b6ecf2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (offset > file_size) return error.FileRange; | ||
| const rounded = std.math.add(usize, length, page_size - 1) catch return error.AddressOverflow; | ||
| const mapped_length = rounded & ~(page_size - 1); | ||
| return .{ .file_offset = offset, .byte_length = @min(length, file_size - offset), .mapped_length = mapped_length, .permissions = permissions }; |
There was a problem hiding this comment.
When length extends by one or more complete pages beyond file_size - offset, this truncates byte_length but retains the full mapped_length; externalFileMmap then installs every page and leaves the excess pages readable as zeros. Linux only zero-fills the remainder of the final file-backed page—access to pages wholly beyond EOF must fault—so this can silently fabricate file contents. Since this runtime has no SIGBUS/page-fault path for these mappings, reject such ranges or leave the beyond-EOF pages unmapped.
Useful? React with 👍 / 👎.
| const permissions: Permissions = .{ .read = protection & 1 != 0, .write = protection & 2 != 0, .execute = protection & 4 != 0 }; | ||
| if (permissions.write and permissions.execute) return error.PermissionDenied; |
There was a problem hiding this comment.
For protection == 0 or write-only protection == 2, plan succeeds, but the downstream Sv39 leaf encoder rejects leaves with neither R nor X and leaves with W but not R. Consequently a syntactically accepted mmap consumes preparation work, rolls back, and reports ENOMEM rather than handling or explicitly rejecting the protection mode. Validate these combinations in the planner, reserve PROT_NONE without a leaf, or normalize Linux write permission to an encodable Sv39 policy.
Useful? React with 👍 / 👎.
| var rollback: usize = 0; | ||
| while (rollback < mapped) : (rollback += 1) | ||
| _ = batch26_builder.unmapPage(candidate + rollback * frames.PageSize, .page_4k) catch shutdown(); | ||
| external_runtime_mappings.cancelLast(candidate, plan.mapped_length); |
There was a problem hiding this comment.
If mapping succeeds for initial pages and a later mapPage fails—for example when a range crosses a 2 MiB page-table boundary with the prepared table-page owner nearly exhausted—this rollback clears only leaf entries. Builder.unmapPage does not release now-empty intermediate page-table frames, so the failed mmap permanently consumes bounded table capacity and repeated failures can make later valid mappings fail. Preflight the required table pages or roll back the intermediate allocations as part of the transaction.
AGENTS.md reference: AGENTS.md:L174-L175
Useful? React with 👍 / 👎.
thanks-cohn
left a comment
thanks-cohn
left a comment
There was a problem hiding this comment.
Holding merge. The PR head is still 2e68b6ecf2a4e90c06ae345611e93b08f0965bcc, so the requested cleanup pass has not landed on this branch yet.
There are now four concrete merge blockers:
-
CI is still red at
zig build checkbecause canonical validation evidence is stale (fixed-capacity-vector: stale source digest). Regenerate the canonical validation evidence and make the full validate workflow green; do not weaken the gate. -
The Playable Alpine one-shell regression is still explicitly unproven in the persisted Batch 32S report. Re-run and persist the full already-earned acceptance after these mmap runtime changes.
-
Codex review found a real file-backed mmap correctness bug:
plan()can accept a mapping whose requested length extends by one or more whole pages beyond EOF; the runtime then maps those wholly-past-EOF pages as readable zero pages. Linux only zero-fills the tail of the final file-backed page; later whole pages must not become fabricated readable file contents. Since Morphic has no SIGBUS/page-fault handling for this slice, reject ranges that would require wholly-past-EOF mapped pages or otherwise leave them inaccessible. Add a permanent regression. -
Codex review also found that
PROT_NONE(protection == 0) and write-only (protection == 2) are accepted by the planner but cannot be encoded by the current Sv39 leaf policy. They fall through to mapping failure and surface as ENOMEM, which is semantically wrong. Handle these protection modes deliberately before mapping: reject unsupported modes deterministically, reserve PROT_NONE without a leaf if that is the chosen bounded semantic, or normalize Linux write permission to an explicitly tested encodable policy. Add permanent tests.
Keep this cleanup bounded. Do not enlarge the 320-page backing pool or start the 950-page libcrypto capacity batch yet. Once these four items are repaired, the runtime-level ownership/failure tests are present, Playable Alpine is re-proven, and CI is green, I expect this PR to be mergeable.
1 participant
Motivation
(st_dev, st_ino)acrossfstat(open(path))andnewfstatat(path)while preserving lstat/no-follow symlink identity.MAP_PRIVATEmapping necessary to advance the observed/sbin/apk --versionfrontier without speculatively implementing a full mmap subsystem.Description
resolveFinalObjectand changed stat paths to derive inode identity from the authoritative namespacemanifest_offsetfor followed resolution, while no-follow retains the symlink object identity; updatedrecipes/run-hosted-morphic-runtime/src/bounded_namespace_lookup.zigandrecipes/run-hosted-morphic-runtime/src/freestanding_riscv64.zigaccordingly.ZIGREF_LINUX_MMAP_REJECTemission with the ordinaryLINUX_MMAP_REJECTtrace to avoid misusing theZIGREF-*diagnostic namespace.recipes/run-hosted-morphic-runtime/src/linux_rv64_file_mmap.zigimplements aplan/preparemodel that checks page alignment, file-range arithmetic, W+X rejection, rounding to pages, tail zeroing, and immutable private copying; integrated runtime mapping infreestanding_riscv64.zigviaexternalFileMmapthat resolves descriptor->resource, reserves backing, copies file bytes into private backing, maps pages, and rolls back on partial failure.linux_rv64_file_mmap.ziginto therun-hosted-morphic-runtimerecipe inbuild.zig, and added a persisted handoff reportdocs/reports/AGENTIC_SNOWBALL_BATCH_32S.mdplus a smallCOMMANDS.mdnote.Testing
python3 tools/query-reference.py agent bootstrapsucceeded and repository indexes were readable, whileagent doctorreported an inherited missing virtualenv interpreter (recorded as an environment warning).zig test recipes/run-hosted-morphic-runtime/src/bounded_namespace_lookup.zig(5/5) andzig test recipes/run-hosted-morphic-runtime/src/linux_rv64_file_mmap.zig(2/2) passed, andzig build test-recipe-run-hosted-morphic-runtimecompleted successfully.zig fmt --checkon touched files andpython3 tools/check-command-reference.py --checkpassed with no command-reference drift.python3 tools/pressure-real-rv64-alpine-minirootfs.py --artifact-onlyproduced the canonical namespace and data used for runtime testing./sbin/apk --versionartifact were exercised under the new runtime; they reliably reproduced the new behavior where the smaller libapk mapping crosses but the observed libcrypto mapping exceeds the bounded private backing (explicit capacity rejection), so/sbin/apk --versiondid not succeed and the libcrypto backing-capacity remains the next causal blocker (recorded in the persisted report).Codex Task