[PR #1822] [MERGED] fix(managed): bound transient CIL by decoded instruction count #4837

New issue

Closed

opened 2026-10-31 00:55:41 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:55:41 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1822
Author: @jhuang-tw
Created: 10/11/2026
Status: ✅ Merged
Merged: 10/11/2026
Merged by: @morluto

Base: main ← Head: fix/managed-single-method-budget

📝 Commits (1)

46104eb fix(managed): charge transient CIL by decoded instructions

📊 Changes

3 files changed (+72 additions, -5 deletions)

View changed files

📝 src/dotnet/ManagedMemberInspector.test.ts (+57 -0)
📝 src/dotnet/ManagedMemberInstructionDecoder.ts (+6 -0)
📝 src/dotnet/ManagedMethodBodyReader.ts (+9 -5)

📄 Description

Why this needs a separate follow-up

Merged #1768 correctly separates accumulated per-method decoding reservations from the retained-output budget. One remaining false rejection is still reproducible on its merged main: a single method with more than ~256 KiB of multi-byte CIL is rejected by admitTransient(il.length * 256) before its actual instruction count is known, even when its retained result is modest.

On pristine post-#1768 main f178bf1c, a source-owned 272,896-byte PE/CLI fixture with one 270,000-byte method (54,000 five-byte instructions) returned coverage: unavailable, types: [], methods: [], metadata.representation resource-limit. The failure is not a malformed target or an MCP transport loss.

Fix

Admit 256 conservative transient bytes per actually decoded instruction, after checking the opcode/operand boundaries and before growing the instruction and start-index arrays. Multi-byte operands no longer each cost a complete instruction's reservation.
Propagate ManagedReaderFailure out of the decoder so exhausted budget stays a typed resource-limit, never a misleading malformed-IL observation.
Keep the existing 64 MiB shared retained/transient admission, per-item retained-output charging, exception-section coexistence, canonical Evidence, and all default CLI/MCP result contracts unchanged. No arbitrary heap/budget increase or target execution.

Independent acceptance and regression

Source-owned well-formed CIL: 54,000 five-byte zero-displacement br instructions (each branch targets the following instruction), followed by ret. Exactly 270,001 IL bytes, stack-neutral. 54,001 instructions are reported with coverage: complete, one retained type and method.
Dense single-byte nop instructions with trailing ret still produce a typed resource-limit and a recorded positive offending IL byte offset, with no invented partial successful method.
Focused managed + budget regressions: 31/31 PASS. Expanded src/dotnet, managed provider/MCP and CLI acceptance suites: PASS. npm run check:fast, npm run build:cached, npm run docs:check, and oxfmt --check on all three changed files: PASS.
Actual published CLI command and pinned-SDK stdio MCP on the generated PE (target statically analyzed, never executed): CLI JSON 4,172,415 bytes, MCP result 9,101,708 bytes under ordinary framing, exact deep equality of their complete normalized_result, 54,001 decoded instructions, successful following SDK ping, and unchanged artifact SHA-256 ab80b511d759b6ec82b20e4ff7f6c4a83c8a5f663d0a7f0ea9049be1ae656d00. Temporary fixtures cleaned.
Zenu mutation checks: VERIFIED including affected-file identity.

Scope and risks

Follow-up to #1768, refs #1801; does not claim to close #1801 or address large signature-only budgets, independent caller-selected resource overrides, or very large retained projections. Another contributor has discussed a different per-call 1 GiB expansion budget in #1801; this intentionally preserves the original 64 MiB transient admission and charges what the decoder actually produces. The 256-byte per decoded instruction accounting remains a conservative projection, not a measured peak-process-RSS guarantee.

Verification used Windows Node 24.17.0/npm 10.9.2 rather than repository-preferred Node 24.18.0/npm 11.16.0; no real proprietary managed assembly, Ghidra or Hopper claim is made.

The change is scoped to three source/test files, with no generated artifacts, user-owned project changes, or new dependencies.

AI assistance was used to develop and verify this contribution.

_{🔄 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/1822 **Author:** [@jhuang-tw](https://github.com/jhuang-tw) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/11/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/managed-single-method-budget` --- ### 📝 Commits (1) - [`46104eb`](https://github.com/morluto/rea/commit/46104eba90cbf16a4963b4b18e0cba9760cd20b2) fix(managed): charge transient CIL by decoded instructions ### 📊 Changes **3 files changed** (+72 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/dotnet/ManagedMemberInspector.test.ts` (+57 -0) 📝 `src/dotnet/ManagedMemberInstructionDecoder.ts` (+6 -0) 📝 `src/dotnet/ManagedMethodBodyReader.ts` (+9 -5) </details> ### 📄 Description ## Why this needs a separate follow-up Merged #1768 correctly separates *accumulated* per-method decoding reservations from the retained-output budget. One remaining false rejection is still reproducible on its merged main: a **single method** with more than ~256 KiB of multi-byte CIL is rejected by `admitTransient(il.length * 256)` before its actual instruction count is known, even when its retained result is modest. On pristine post-#1768 main `f178bf1c`, a source-owned 272,896-byte PE/CLI fixture with one 270,000-byte method (54,000 five-byte instructions) returned `coverage: unavailable`, `types: []`, `methods: []`, `metadata.representation` resource-limit. The failure is not a malformed target or an MCP transport loss. ## Fix - Admit **256 conservative transient bytes per actually decoded instruction**, after checking the opcode/operand boundaries and **before** growing the instruction and start-index arrays. Multi-byte operands no longer each cost a complete instruction's reservation. - Propagate `ManagedReaderFailure` out of the decoder so exhausted budget stays a typed `resource-limit`, never a misleading malformed-IL observation. - Keep the existing **64 MiB** shared retained/transient admission, per-item retained-output charging, exception-section coexistence, canonical Evidence, and all default CLI/MCP result contracts unchanged. No arbitrary heap/budget increase or target execution. ## Independent acceptance and regression - Source-owned well-formed CIL: 54,000 five-byte zero-displacement `br` instructions (each branch targets the following instruction), followed by `ret`. Exactly 270,001 IL bytes, stack-neutral. **54,001** instructions are reported with `coverage: complete`, one retained type and method. - Dense single-byte `nop` instructions with trailing `ret` still produce a **typed resource-limit** and a recorded positive offending IL byte offset, with no invented partial successful method. - Focused managed + budget regressions: **31/31 PASS**. Expanded `src/dotnet`, managed provider/MCP and CLI acceptance suites: PASS. `npm run check:fast`, `npm run build:cached`, `npm run docs:check`, and `oxfmt --check` on all three changed files: PASS. - Actual published CLI command and pinned-SDK **stdio MCP** on the generated PE (target statically analyzed, never executed): CLI JSON **4,172,415 bytes**, MCP result **9,101,708 bytes** under ordinary framing, exact deep equality of their complete `normalized_result`, 54,001 decoded instructions, successful following SDK ping, and unchanged artifact SHA-256 `ab80b511d759b6ec82b20e4ff7f6c4a83c8a5f663d0a7f0ea9049be1ae656d00`. Temporary fixtures cleaned. - Zenu mutation checks: VERIFIED including affected-file identity. ## Scope and risks Follow-up to #1768, refs #1801; **does not claim to close #1801** or address large signature-only budgets, independent caller-selected resource overrides, or very large retained projections. Another contributor has discussed a different per-call 1 GiB expansion budget in #1801; this intentionally **preserves the original 64 MiB transient admission** and charges what the decoder actually produces. The 256-byte per decoded instruction accounting remains a conservative projection, **not a measured peak-process-RSS guarantee**. Verification used Windows Node 24.17.0/npm 10.9.2 rather than repository-preferred Node 24.18.0/npm 11.16.0; no real proprietary managed assembly, Ghidra or Hopper claim is made. The change is scoped to three source/test files, with no generated artifacts, user-owned project changes, or new dependencies. AI assistance was used to develop and verify this contribution. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 00:55:41 +00:00