[PR #6254] [MERGED] fix(nativeaot): preserve COPY bytes when hydration output grows #7229

Closed
opened 2026-11-01 21:46:19 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6254
Author: @kaoru0822-kitauji
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/5882-nativeaot-copy-growth


📝 Commits (1)

  • a24e297 fix(nativeaot): preserve COPY bytes when hydration output grows

📊 Changes

3 files changed (+276 additions, -9 deletions)

View changed files

➕ src/domain/native/nativeAotPe.hydration.test.ts (+149 -0)
📝 src/domain/native/nativeAotPe.ts (+11 -9)
📝 tests/fixtures/nativeaotPe.ts (+116 -0)

📄 Description

What broke: A NativeAOT DEHYDRATED_DATA COPY that grew the hydration buffer wrote its payload into the old buffer. The overlay then hashed uninitialized bytes, and the report still said status: complete.
Fix: Reserving output now returns a view into the buffer that is kept. Every hydration writer (COPY, zero fill, relative/absolute fixups) writes only through that view, so no writer can hold a buffer that growth has replaced.
Risk: Low. The change is local to hydrate in src/domain/native/nativeAotPe.ts. Growth, limits and budget accounting are unchanged, and the microbenchmark showed no measurable cost.

Summary

Fixes #5882

The built-in NativeAOT PE parser (parseNativeAotPe) builds the read-only derived overlay by running the RTR9.1 hydration command stream. Before this change, chunk.copy(output, ensureOutput(chunk.length)) read output before ensureOutput replaced it with a larger buffer. Any COPY that triggered growth therefore lost its bytes. derived_overlay.size_bytes was still correct, but sha256 identified wrong (uninitialized) contents, and there was no diagnostic. With this change the overlay holds the bytes the command stream defines.

Problem and expected behavior

  • Smallest trigger: one COPY whose output passes the initial 64 KiB allocation (for example 65,537 bytes), or a later COPY that crosses it.
  • Violated invariant: the documented overlay descriptor (address, size, SHA-256) must identify the actual hydration result. Source bytes stay immutable.
  • New behavior: the overlay digest equals the digest of the bytes the stream defines, across one or more growths and up to the mapped-section limit.

Change and scope

  • The growth helper ensureOutput(length): number became reserveOutput(length): Buffer. It grows the buffer when needed and returns output.subarray(start, start + length) of the buffer that is kept. All four writers write into the returned view, and none of them references output directly any more. This removes the evaluation-order hazard for every opcode, not just COPY. A comment on the helper records the remaining invariant: writers must fill the whole view, because growth uses allocUnsafe.
  • Unchanged: growth policy (doubling, capped at the mapped-section/work/memory limit), working-memory reservation and release, error messages, and partial-report behavior on failure.
  • Not applicable: this format has no back-references. COPY copies from the command stream, never from earlier output, so there are no overlapping-copy cases.
  • Contribution scope: an ordinary fix within existing boundaries. No maintainer scope decision is needed.

Contract and boundary impact

  • Semantic owner and earliest changed stage: hydrate in src/domain/native/nativeAotPe.ts (domain parser), which is the overlay producer.
  • Contribution scope fit and linked maintainer decision: an ordinary bug fix; not applicable.
  • CLI and MCP/tool-catalog contract: none. Schemas and fields are unchanged. inspect-native-load-image / inspect_native_load_image reach this parser through the Ghidra snapshot owner (established from source).
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: derived_overlay.sha256 now identifies the hydrated bytes for streams larger than 64 KiB. Address and size semantics are unchanged.
  • Target selection, authority, upstream behavior, and cleanup details: not applicable.
  • Generated metadata, package, or installation impact: none.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated:
    • src/domain/native/nativeAotPe.hydration.test.ts: 11 success cases compare the full overlay descriptor (address, size, SHA-256) with independently authored expected bytes, and check that the snapshot digest is unchanged. Cases:
      • COPY ending exactly at the initial capacity
      • COPY that grows the buffer
      • COPY that grows a buffer already holding output
      • two growths
      • empty COPYs around a growth
      • zero fill, relative fixup, pointer fixup, inline relative pointers and inline pointers, each straddling the growth boundary
      • growth up to exactly the mapped-section end (two growths, the second clamped to the limit)
    • Four failure cases check status: partial, derived_overlay: null and the specific diagnostic:
      • COPY one byte past the section
      • pointer fixup past the section after growth
      • truncated COPY payload after growth
      • fixup index outside captured sections after growth
    • tests/fixtures/nativeaotPe.ts: nativeAotPeHydrationFixture authors an RTR9.1 command stream inside an expanded .rdata section and computes the expected overlay from writer semantics.
  • Base reproduction (executed): the issue's test on unmodified origin/main (c537c52c9) failed 3 of 4. [65537], [32768,32769] and [65537,65537] were complete with the right size but a wrong digest (for example, the parser reported abc20979… where a3253bfa… was expected). The new suite with the fix reverted fails the 5 cases where a COPY triggers growth and passes the others. The non-COPY writers were already correct; their cases guard the new reservation contract.
  • User-visible CLI/MCP output: none changed beyond the corrected digest.
  • Remaining proof gaps: no real Ghidra, CLI or MCP run (Ghidra is not installed here). Coverage is at the parser production boundary with synthetic PE fixtures. Effects on downstream MethodTable or frozen-string discovery in real binaries were not measured.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npx vitest run --project domain src/domain/native/nativeAotPe.copyGrowth.test.ts --maxWorkers 1 (issue repro, temporary file): on base, 3 failed / 1 passed; with the fix, 4 passed.
  • npx vitest run --project domain src/domain/native/nativeAotPe.hydration.test.ts --maxWorkers 1: 15 passed. With the source fix reverted: 5 failed / 10 passed.
  • npx vitest run src/domain/native src/ghidra/GhidraProvider.behavior.test.ts src/ghidra/GhidraLoadImageAttest.test.ts src/ghidra/extensions/GhidraExtensions.test.ts --maxWorkers 2: 16 files, 334 tests passed.
  • npm run check:fast: passed (typecheck, lint).
  • npm run format:check: passed.
  • Performance (local microbenchmark, warm, 40 parses of a fixture with 16,000 inline fixups plus a 40 KB COPY): 8.5–9.4 ms per parse before and 8.8 ms after. The difference is within run-to-run noise. Token usage and memory were not measured; the change does not alter response size or allocation policy.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: none. macOS arm64 unit/integration tests only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: parser-only; no new I/O, processes or authority. Uninitialized growth capacity is still never exposed, because every reserved byte is written before the overlay is hashed.

Author readiness

Human owner: @kaoru0822-kitauji

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6254 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/5882-nativeaot-copy-growth` --- ### 📝 Commits (1) - [`a24e297`](https://github.com/morluto/rea/commit/a24e297c7e2c7cbbc772a6d779e0337a5392d372) fix(nativeaot): preserve COPY bytes when hydration output grows ### 📊 Changes **3 files changed** (+276 additions, -9 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/native/nativeAotPe.hydration.test.ts` (+149 -0) 📝 `src/domain/native/nativeAotPe.ts` (+11 -9) 📝 `tests/fixtures/nativeaotPe.ts` (+116 -0) </details> ### 📄 Description **What broke:** A NativeAOT DEHYDRATED_DATA COPY that grew the hydration buffer wrote its payload into the old buffer. The overlay then hashed uninitialized bytes, and the report still said `status: complete`. **Fix:** Reserving output now returns a view into the buffer that is kept. Every hydration writer (COPY, zero fill, relative/absolute fixups) writes only through that view, so no writer can hold a buffer that growth has replaced. **Risk:** Low. The change is local to `hydrate` in `src/domain/native/nativeAotPe.ts`. Growth, limits and budget accounting are unchanged, and the microbenchmark showed no measurable cost. ## Summary Fixes #5882 The built-in NativeAOT PE parser (`parseNativeAotPe`) builds the read-only derived overlay by running the RTR9.1 hydration command stream. Before this change, `chunk.copy(output, ensureOutput(chunk.length))` read `output` before `ensureOutput` replaced it with a larger buffer. Any COPY that triggered growth therefore lost its bytes. `derived_overlay.size_bytes` was still correct, but `sha256` identified wrong (uninitialized) contents, and there was no diagnostic. With this change the overlay holds the bytes the command stream defines. ## Problem and expected behavior - Smallest trigger: one COPY whose output passes the initial 64 KiB allocation (for example 65,537 bytes), or a later COPY that crosses it. - Violated invariant: the documented overlay descriptor (address, size, SHA-256) must identify the actual hydration result. Source bytes stay immutable. - New behavior: the overlay digest equals the digest of the bytes the stream defines, across one or more growths and up to the mapped-section limit. ## Change and scope - The growth helper `ensureOutput(length): number` became `reserveOutput(length): Buffer`. It grows the buffer when needed and returns `output.subarray(start, start + length)` of the buffer that is kept. All four writers write into the returned view, and none of them references `output` directly any more. This removes the evaluation-order hazard for every opcode, not just COPY. A comment on the helper records the remaining invariant: writers must fill the whole view, because growth uses `allocUnsafe`. - Unchanged: growth policy (doubling, capped at the mapped-section/work/memory limit), working-memory reservation and release, error messages, and partial-report behavior on failure. - Not applicable: this format has no back-references. COPY copies from the command stream, never from earlier output, so there are no overlapping-copy cases. - Contribution scope: an ordinary fix within existing boundaries. No maintainer scope decision is needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `hydrate` in `src/domain/native/nativeAotPe.ts` (domain parser), which is the overlay producer. - Contribution scope fit and linked maintainer decision: an ordinary bug fix; not applicable. - CLI and MCP/tool-catalog contract: none. Schemas and fields are unchanged. `inspect-native-load-image` / `inspect_native_load_image` reach this parser through the Ghidra snapshot owner (established from source). - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: `derived_overlay.sha256` now identifies the hydrated bytes for streams larger than 64 KiB. Address and size semantics are unchanged. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: - `src/domain/native/nativeAotPe.hydration.test.ts`: 11 success cases compare the full overlay descriptor (address, size, SHA-256) with independently authored expected bytes, and check that the snapshot digest is unchanged. Cases: - COPY ending exactly at the initial capacity - COPY that grows the buffer - COPY that grows a buffer already holding output - two growths - empty COPYs around a growth - zero fill, relative fixup, pointer fixup, inline relative pointers and inline pointers, each straddling the growth boundary - growth up to exactly the mapped-section end (two growths, the second clamped to the limit) - Four failure cases check `status: partial`, `derived_overlay: null` and the specific diagnostic: - COPY one byte past the section - pointer fixup past the section after growth - truncated COPY payload after growth - fixup index outside captured sections after growth - `tests/fixtures/nativeaotPe.ts`: `nativeAotPeHydrationFixture` authors an RTR9.1 command stream inside an expanded `.rdata` section and computes the expected overlay from writer semantics. - Base reproduction (executed): the issue's test on unmodified `origin/main` (c537c52c9) failed 3 of 4. `[65537]`, `[32768,32769]` and `[65537,65537]` were `complete` with the right size but a wrong digest (for example, the parser reported `abc20979…` where `a3253bfa…` was expected). The new suite with the fix reverted fails the 5 cases where a COPY triggers growth and passes the others. The non-COPY writers were already correct; their cases guard the new reservation contract. - User-visible CLI/MCP output: none changed beyond the corrected digest. - Remaining proof gaps: no real Ghidra, CLI or MCP run (Ghidra is not installed here). Coverage is at the parser production boundary with synthetic PE fixtures. Effects on downstream MethodTable or frozen-string discovery in real binaries were not measured. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npx vitest run --project domain src/domain/native/nativeAotPe.copyGrowth.test.ts --maxWorkers 1` (issue repro, temporary file): on base, 3 failed / 1 passed; with the fix, 4 passed. - `npx vitest run --project domain src/domain/native/nativeAotPe.hydration.test.ts --maxWorkers 1`: 15 passed. With the source fix reverted: 5 failed / 10 passed. - `npx vitest run src/domain/native src/ghidra/GhidraProvider.behavior.test.ts src/ghidra/GhidraLoadImageAttest.test.ts src/ghidra/extensions/GhidraExtensions.test.ts --maxWorkers 2`: 16 files, 334 tests passed. - `npm run check:fast`: passed (typecheck, lint). - `npm run format:check`: passed. - Performance (local microbenchmark, warm, 40 parses of a fixture with 16,000 inline fixups plus a 40 KB COPY): 8.5–9.4 ms per parse before and 8.8 ms after. The difference is within run-to-run noise. Token usage and memory were not measured; the change does not alter response size or allocation policy. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: none. macOS arm64 unit/integration tests only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: parser-only; no new I/O, processes or authority. Uninitialized growth capacity is still never exposed, because every reserved byte is written before the overlay is hashed. ## Author readiness Human owner: @kaoru0822-kitauji - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:46:19 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#7229
No description provided.