[PR #3344] [CLOSED] fix(dotnet): accept legacy small EH sections that omit the header size #5719

Closed
opened 2026-10-31 01:08:54 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3344
Author: @Anurag-5ingh
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/managed-small-eh-datasize


📝 Commits (1)

  • 3176762 fix(dotnet): accept legacy small EH sections that omit the header size

📊 Changes

2 files changed (+100 additions, -6 deletions)

View changed files

📝 src/dotnet/ManagedCilDecoding.test.ts (+87 -0)
📝 src/dotnet/ManagedMemberInstructionDecoder.ts (+13 -6)

📄 Description

Summary

Fixes #3279.

inspect-managed-members reported every try/catch/finally method compiled by legacy VB.NET or MC++ as malformed ("Exception section size does not contain whole clauses"), with empty exception_regions and normalized_il_sha256: null. That also blocked import_managed_reconstruction for those methods because no normalized-IL lock could be computed. Those compilers write the small EH section DataSize as the clause bytes only, excluding the 4-byte section header. The CLR accepts that layout, so REA now decodes it like the conformant one.

Problem and expected behavior

Smallest trigger: a fat method body with MoreSects whose small EH section is 01 0c 00 00 followed by one 12-byte clause (DataSize = 12). The conformant layout of the same section (01 10 00 00 …, DataSize = 16) is already accepted.

CoreCLR documents the quirk in COR_ILMETHOD_SECT::DataSize() (src/coreclr/inc/corhlpr.h, VSWhidbey #99031): for small EH sections the clause count is DataSize / sizeof(IMAGE_COR_ILMETHOD_SECT_EH_CLAUSE_SMALL), and the section spans the header plus those clauses. The decoder required (DataSize - 4) % 12 == 0, which every legacy section fails.

Expected: both layouts produce the same clauses, the same body status, and the same normalized-IL digest. Sizes that are neither form, such as a partial clause, stay malformed.

Change and scope

parseExceptionRegions in src/dotnet/ManagedMemberInstructionDecoder.ts is the only reader of method data sections. It now derives the section's byte span once:

  • A small section whose DataSize is a whole non-zero multiple of 12 spans DataSize + 4 bytes (legacy layout).
  • Every other section spans DataSize bytes, as before.

The bounds check, clause count, and offset of the next chained section all use that span. A conformant small size (12n + 4) is never a multiple of 12, so the two layouts cannot be confused, and every previously accepted section decodes unchanged.

Not included:

  • Fat sections remain header-inclusive. The VB/MC++ quirk and CoreCLR's compatibility path apply only to small sections.
  • Other non-whole-clause sizes are not rounded down the way CoreCLR's general floor does. They stay reported as malformed rather than being silently reinterpreted.
  • The body schema does not gain a field naming the layout, because the decoded clauses are identical.

Follow the contribution scope: this is an ordinary parser fix within the existing static managed-analysis boundary, so a maintainer scope decision is not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: parseExceptionRegions (managed static method-body decoding), the single owner of EH section layout.
  • Contribution scope fit and linked maintainer decision: in scope (static format parsing); not applicable.
  • CLI and MCP/tool-catalog contract: no schema or catalog change. Affected methods now report status: "present", their exception_regions, and normalized_il_sha256 instead of malformed.
  • Provider, bridge, target-format, or platform compatibility: managed static provider only; platform-independent.
  • Evidence, artifact, provenance, or reconstruction contract: no shape change. Legacy-compiled methods now get a normalized-IL digest, so reconstruction import is no longer blocked for them.
  • 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 in src/dotnet/ManagedCilDecoding.test.ts. They exercise the production inspector through byte-built PE fixtures:
    • A legacy small section (DataSize = 12) yields a body strictly equal to the conformant section's (DataSize = 16), including status: "present" and a non-null normalized_il_sha256.
    • A chained legacy section advances by its full 16-byte span, so the next section is still read.
    • A fat section whose size is a whole multiple of 24 stays malformed (the quirk applies only to small sections).
    • At the end of the file-backed PE section, a legacy section is accepted when its 16-byte span fits, and reports "Exception section size leaves artifact" when only its 12 declared bytes fit.
  • Base reproduction (source-derived, not executed separately): on the base revision, the legacy-layout tests would fail with "Exception section size does not contain whole clauses", the bug from the issue. The existing partial-clause, unsupported-kind, and out-of-artifact cases are unchanged.
  • User-visible CLI/MCP output: the affected inspect-managed-members bodies change from malformed to present with decoded exception_regions.
  • Remaining proof gaps: verified with synthetic byte-built fixtures that match the issue's byte layout; not verified against a binary produced by an actual legacy VB 2005 / MC++ compiler.

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

  • CI workflow (ci.yml, full qualification via workflow_dispatch) on the fork branch:
    • Static checks (check:ci: typecheck, lint, module boundaries, jscpd, format:check; plus verify:test-discovery) — passed
    • Build package, Docs Generation, Source-built conformance fixtures — passed
    • Complete portable suite with coverage, all six shards, and Portable tests — passed
  • oxfmt --check on the changed files — passed
  • browser / real-browser failed in that run: a Chrome scenario capture (capture-browser-scenario, service-worker block policy) returned execution_failure. This change does not touch browser code; a rerun of that job was still queued when this PR was opened.
  • Every other lane in the run passed, including reverse-tools coverage and mutation, the real Apple artifact E2E, the native package baselines, and the Windows capability lane.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Previously accepted sections decode identically; only previously rejected legacy small sections change from malformed to present.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable (managed static parsing).
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: read-only parsing. The new span is bounds-checked against the method extent and the artifact before any clause is read, and the transient decode budget is still charged per clause.

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.

🔄 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/3344 **Author:** [@Anurag-5ingh](https://github.com/Anurag-5ingh) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/managed-small-eh-datasize` --- ### 📝 Commits (1) - [`3176762`](https://github.com/morluto/rea/commit/31767625af4775c7256e9ed89253c4bf872f7877) fix(dotnet): accept legacy small EH sections that omit the header size ### 📊 Changes **2 files changed** (+100 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `src/dotnet/ManagedCilDecoding.test.ts` (+87 -0) 📝 `src/dotnet/ManagedMemberInstructionDecoder.ts` (+13 -6) </details> ### 📄 Description ## Summary Fixes #3279. `inspect-managed-members` reported every try/catch/finally method compiled by legacy VB.NET or MC++ as `malformed` ("Exception section size does not contain whole clauses"), with empty `exception_regions` and `normalized_il_sha256: null`. That also blocked `import_managed_reconstruction` for those methods because no normalized-IL lock could be computed. Those compilers write the small EH section `DataSize` as the clause bytes only, excluding the 4-byte section header. The CLR accepts that layout, so REA now decodes it like the conformant one. ## Problem and expected behavior Smallest trigger: a fat method body with `MoreSects` whose small EH section is `01 0c 00 00` followed by one 12-byte clause (`DataSize = 12`). The conformant layout of the same section (`01 10 00 00 …`, `DataSize = 16`) is already accepted. CoreCLR documents the quirk in `COR_ILMETHOD_SECT::DataSize()` (`src/coreclr/inc/corhlpr.h`, VSWhidbey #99031): for small EH sections the clause count is `DataSize / sizeof(IMAGE_COR_ILMETHOD_SECT_EH_CLAUSE_SMALL)`, and the section spans the header plus those clauses. The decoder required `(DataSize - 4) % 12 == 0`, which every legacy section fails. Expected: both layouts produce the same clauses, the same body status, and the same normalized-IL digest. Sizes that are neither form, such as a partial clause, stay malformed. ## Change and scope `parseExceptionRegions` in `src/dotnet/ManagedMemberInstructionDecoder.ts` is the only reader of method data sections. It now derives the section's byte span once: - A small section whose `DataSize` is a whole non-zero multiple of 12 spans `DataSize + 4` bytes (legacy layout). - Every other section spans `DataSize` bytes, as before. The bounds check, clause count, and offset of the next chained section all use that span. A conformant small size (`12n + 4`) is never a multiple of 12, so the two layouts cannot be confused, and every previously accepted section decodes unchanged. Not included: - Fat sections remain header-inclusive. The VB/MC++ quirk and CoreCLR's compatibility path apply only to small sections. - Other non-whole-clause sizes are not rounded down the way CoreCLR's general floor does. They stay reported as malformed rather than being silently reinterpreted. - The body schema does not gain a field naming the layout, because the decoded clauses are identical. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope): this is an ordinary parser fix within the existing static managed-analysis boundary, so a maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `parseExceptionRegions` (managed static method-body decoding), the single owner of EH section layout. - Contribution scope fit and linked maintainer decision: in scope (static format parsing); not applicable. - CLI and MCP/tool-catalog contract: no schema or catalog change. Affected methods now report `status: "present"`, their `exception_regions`, and `normalized_il_sha256` instead of `malformed`. - Provider, bridge, target-format, or platform compatibility: managed static provider only; platform-independent. - Evidence, artifact, provenance, or reconstruction contract: no shape change. Legacy-compiled methods now get a normalized-IL digest, so reconstruction import is no longer blocked for them. - 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 in `src/dotnet/ManagedCilDecoding.test.ts`. They exercise the production inspector through byte-built PE fixtures: - A legacy small section (`DataSize = 12`) yields a body strictly equal to the conformant section's (`DataSize = 16`), including `status: "present"` and a non-null `normalized_il_sha256`. - A chained legacy section advances by its full 16-byte span, so the next section is still read. - A fat section whose size is a whole multiple of 24 stays malformed (the quirk applies only to small sections). - At the end of the file-backed PE section, a legacy section is accepted when its 16-byte span fits, and reports "Exception section size leaves artifact" when only its 12 declared bytes fit. - Base reproduction (source-derived, not executed separately): on the base revision, the legacy-layout tests would fail with "Exception section size does not contain whole clauses", the bug from the issue. The existing partial-clause, unsupported-kind, and out-of-artifact cases are unchanged. - User-visible CLI/MCP output: the affected `inspect-managed-members` bodies change from `malformed` to `present` with decoded `exception_regions`. - Remaining proof gaps: verified with synthetic byte-built fixtures that match the issue's byte layout; not verified against a binary produced by an actual legacy VB 2005 / MC++ compiler. 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 - CI workflow (`ci.yml`, full qualification via `workflow_dispatch`) on the fork branch: - Static checks (`check:ci`: typecheck, lint, module boundaries, jscpd, `format:check`; plus `verify:test-discovery`) — passed - Build package, Docs Generation, Source-built conformance fixtures — passed - Complete portable suite with coverage, all six shards, and Portable tests — passed - `oxfmt --check` on the changed files — passed - `browser / real-browser` failed in that run: a Chrome scenario capture (`capture-browser-scenario`, service-worker block policy) returned `execution_failure`. This change does not touch browser code; a rerun of that job was still queued when this PR was opened. - Every other lane in the run passed, including reverse-tools coverage and mutation, the real Apple artifact E2E, the native package baselines, and the Windows capability lane. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Previously accepted sections decode identically; only previously rejected legacy small sections change from malformed to present. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (managed static parsing). - Package or release metadata impact: none. - Security, privacy, process, or containment review: read-only parsing. The new span is bounds-checked against the method extent and the artifact before any clause is read, and the transient decode budget is still charged per clause. ## 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:08:54 +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#5719
No description provided.