morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4528] [CLOSED] fix(dotnet): reject unfinished CIL prefix chain at end of method body #6288

New issue

Closed

opened 2026-10-31 01:13:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:13:19 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4528
Author: @kevinjosh10
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/issue-4370-dangling-cil-prefix

📝 Commits (1)

f9bc785 fix(dotnet): reject unfinished CIL prefix chain at end of method body

📊 Changes

2 files changed (+54 additions, -0 deletions)

View changed files

📝 src/dotnet/ManagedCilDecoding.test.ts (+51 -0)
📝 src/dotnet/ManagedMemberInstructionDecoder.ts (+3 -0)

📄 Description

Summary

Fixes #4370.

When decoding CIL instructions in decodeInstructions (src/dotnet/ManagedMemberInstructionDecoder.ts), an unfinished prefix chain at the end of a method body (e.g. volatile. or tail.) was parsed without error, leaving issue: null. Consequently, methodBody marked the method body as status: "present", emitted a normalized_il_sha256 hash, and reported complete coverage, even though the trailing prefix lacked a following opcode to modify.

This PR checks whether a pending prefixStart remains after the instruction decode loop completes. If an unfinished prefix chain is detected without a prior decode failure, it records issue: "Unfinished CIL prefix chain at IL offset ${String(prefixStart)} leaves method body", correctly transitioning the method body to status: "malformed", nulling normalized_il_sha256, and reporting partial coverage while retaining parsed instructions.

Problem and expected behavior

Observed behavior: A method body ending with a CIL prefix opcode (0xFE 0x13, volatile.) was reported as present with issue: null, complete scan coverage, and a normalized IL SHA-256 fingerprint.
Expected behavior: In ECMA-335, prefix opcodes must precede a target opcode. A terminal prefix chain must report status: "malformed", a non-null diagnostic issue identifying the unfinished prefix chain, normalized_il_sha256: null, and partial coverage.

Change and scope

Added a check in src/dotnet/ManagedMemberInstructionDecoder.ts after the instruction decode loop:

if (issue === null && prefixStart !== null) {
  issue = `Unfinished CIL prefix chain at IL offset ${String(prefixStart)} leaves method body`;
}

Added test coverage in src/dotnet/ManagedCilDecoding.test.ts verifying that terminal prefixes (volatile., tail., readonly., unaligned., no., constrained., chained prefixes, and preceding valid instructions) in tiny and fat method headers produce malformed bodies without normalized hashes.

Contract and boundary impact

Semantic owner and earliest changed stage: CIL instruction decoder (ManagedMemberInstructionDecoder.ts)
Contribution scope fit and linked maintainer decision: Bug fix within existing managed inspection contracts
CLI and MCP/tool-catalog contract: none
Provider, bridge, target-format, or platform compatibility: all platforms
Evidence, artifact, provenance, or reconstruction contract: none
Target selection, authority, upstream behavior, and cleanup details (if applicable): none
Generated metadata impact: none
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/dotnet/ManagedCilDecoding.test.ts
Base reproduction or other evidence: Verified reproduction script against base revision demonstrating present with normalized IL hash, and confirmed malformed with normalized_il_sha256: null after fix.
User-visible CLI/MCP output (if applicable): none
Remaining proof gaps: none

Validation performed

npm run test:local -- src/dotnet/ManagedCilDecoding.test.ts — 87 passed (100%)
npm run test:local -- src/dotnet/ManagedMemberInspector.test.ts — 131 passed (100%)
All 14 src/dotnet/*.test.ts suites — 348 passed (100%)
npx turbo run typecheck lint --force — 2 tasks successful (0 errors)
node scripts/verify-module-boundaries.mjs — Checked 1979 source files (0 errors)
npm run format:check — All 3765 files formatted properly

Compatibility, safety, and release

Breaking changes or migration steps: None
Real Hopper/Ghidra, browser, or OS coverage: Platform-independent .NET CIL byte decoding
Package or release metadata impact: None
Security, privacy, process, or containment review: Pure instruction decode validation fix

Author readiness

Human owner: @kevinjosh10

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.

Maintainer blocker resolution

This alternative duplicates #4416 for #4370. Its distinct fat-method regression has been carried into #4416 at 3ab24789d2c4fd3e7cddd562036fe4a4f69ac6b3, whose decoder suite passed 90 tests plus check:fast. #4416 retains stronger prior-error, completed-chain and public-CLI coverage. Closing this duplicate preserves one primary implementation and the unique regression; #4416 remains subject to its own checks and merge decision.

_{🔄 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/4528 **Author:** [@kevinjosh10](https://github.com/kevinjosh10) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/issue-4370-dangling-cil-prefix` --- ### 📝 Commits (1) - [`f9bc785`](https://github.com/morluto/rea/commit/f9bc785ecc8e7f301d260f061691d742f3d68674) fix(dotnet): reject unfinished CIL prefix chain at end of method body ### 📊 Changes **2 files changed** (+54 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/dotnet/ManagedCilDecoding.test.ts` (+51 -0) 📝 `src/dotnet/ManagedMemberInstructionDecoder.ts` (+3 -0) </details> ### 📄 Description ## Summary Fixes #4370. When decoding CIL instructions in `decodeInstructions` (`src/dotnet/ManagedMemberInstructionDecoder.ts`), an unfinished prefix chain at the end of a method body (e.g. `volatile.` or `tail.`) was parsed without error, leaving `issue: null`. Consequently, `methodBody` marked the method body as `status: "present"`, emitted a `normalized_il_sha256` hash, and reported complete coverage, even though the trailing prefix lacked a following opcode to modify. This PR checks whether a pending `prefixStart` remains after the instruction decode loop completes. If an unfinished prefix chain is detected without a prior decode failure, it records `issue: "Unfinished CIL prefix chain at IL offset ${String(prefixStart)} leaves method body"`, correctly transitioning the method body to `status: "malformed"`, nulling `normalized_il_sha256`, and reporting partial coverage while retaining parsed instructions. ## Problem and expected behavior - **Observed behavior**: A method body ending with a CIL prefix opcode (`0xFE 0x13`, `volatile.`) was reported as `present` with `issue: null`, complete scan coverage, and a normalized IL SHA-256 fingerprint. - **Expected behavior**: In ECMA-335, prefix opcodes must precede a target opcode. A terminal prefix chain must report `status: "malformed"`, a non-null diagnostic issue identifying the unfinished prefix chain, `normalized_il_sha256: null`, and partial coverage. ## Change and scope - Added a check in `src/dotnet/ManagedMemberInstructionDecoder.ts` after the instruction decode loop: ```ts if (issue === null && prefixStart !== null) { issue = `Unfinished CIL prefix chain at IL offset ${String(prefixStart)} leaves method body`; } ``` - Added test coverage in `src/dotnet/ManagedCilDecoding.test.ts` verifying that terminal prefixes (`volatile.`, `tail.`, `readonly.`, `unaligned.`, `no.`, `constrained.`, chained prefixes, and preceding valid instructions) in tiny and fat method headers produce malformed bodies without normalized hashes. ## Contract and boundary impact - Semantic owner and earliest changed stage: CIL instruction decoder (`ManagedMemberInstructionDecoder.ts`) - Contribution scope fit and linked maintainer decision: Bug fix within existing managed inspection contracts - CLI and MCP/tool-catalog contract: none - Provider, bridge, target-format, or platform compatibility: all platforms - Evidence, artifact, provenance, or reconstruction contract: none - Target selection, authority, upstream behavior, and cleanup details (if applicable): none - Generated metadata impact: none - [ ] 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/dotnet/ManagedCilDecoding.test.ts` - Base reproduction or other evidence: Verified reproduction script against base revision demonstrating `present` with normalized IL hash, and confirmed `malformed` with `normalized_il_sha256: null` after fix. - User-visible CLI/MCP output (if applicable): none - Remaining proof gaps: none ## Validation performed - `npm run test:local -- src/dotnet/ManagedCilDecoding.test.ts` — 87 passed (100%) - `npm run test:local -- src/dotnet/ManagedMemberInspector.test.ts` — 131 passed (100%) - All 14 `src/dotnet/*.test.ts` suites — 348 passed (100%) - `npx turbo run typecheck lint --force` — 2 tasks successful (0 errors) - `node scripts/verify-module-boundaries.mjs` — Checked 1979 source files (0 errors) - `npm run format:check` — All 3765 files formatted properly ## Compatibility, safety, and release - Breaking changes or migration steps: None - Real Hopper/Ghidra, browser, or OS coverage: Platform-independent .NET CIL byte decoding - Package or release metadata impact: None - Security, privacy, process, or containment review: Pure instruction decode validation fix ## Author readiness Human owner: @kevinjosh10 - [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.  ### Maintainer blocker resolution This alternative duplicates #4416 for #4370. Its distinct fat-method regression has been carried into #4416 at `3ab24789d2c4fd3e7cddd562036fe4a4f69ac6b3`, whose decoder suite passed 90 tests plus check:fast. #4416 retains stronger prior-error, completed-chain and public-CLI coverage. Closing this duplicate preserves one primary implementation and the unique regression; #4416 remains subject to its own checks and merge decision. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:13:19 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:11:07 +00:00

[GH-ISSUE #5654] [Tracker] Open PRs that can't merge yet: conflicts and red CI (October 2026) #3425

bsakaguchi referenced this issue

2026-11-01 21:46:33 +00:00

[PR #6288] feat(harmony): add xpanda-backed Ark bytecode analysis #7259

bsakaguchi changed title from ~~[PR #4528] [CLOSED] fix(dotnet): reject unfinished CIL prefix chain at end of method body~~ to [PR #4528] fix(dotnet): reject unfinished CIL prefix chain at end of method body

2026-11-03 13:32:35 +00:00

bsakaguchi changed title from ~~[PR #4528] fix(dotnet): reject unfinished CIL prefix chain at end of method body~~ to [PR #4528] [CLOSED] fix(dotnet): reject unfinished CIL prefix chain at end of method body

2026-11-03 14:34:20 +00:00

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#6288

No description provided.

Rows
Columns