morluto/rea

Fork 0

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

[PR #3516] [MERGED] fix(managed): report ILONLY-clear assemblies as mixed in boundary summaries #5796

New issue

Closed

opened 2026-10-31 01:09:30 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:09:30 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/managed-mixed-mode-summary

📝 Commits (2)

bb5091f fix(managed): report ILONLY-clear assemblies as mixed in boundary summaries
897d4da Merge remote-tracking branch 'origin/main' into fix/managed-mixed-mode-summary

📊 Changes

5 files changed (+68 additions, -31 deletions)

View changed files

📝 src/dotnet/ManagedArtifactInspector.ts (+2 -3)
📝 src/dotnet/ManagedNativeBoundaryHelpers.ts (+21 -19)
📝 src/dotnet/ManagedNativeBoundaryInspector.test.ts (+29 -0)
📝 src/dotnet/ManagedNativeBoundaryInspector.ts (+9 -9)
📝 src/dotnet/ManagedPeReader.ts (+7 -0)

📄 Description

Summary

inspect-managed-native-boundaries reported summary.mixed_mode_or_native_header: false for C++/CLI (ILONLY clear, not ReadyToRun) assemblies, even though inspect-managed-artifact classifies the same bytes as mixed-clr-native / cpp-cli-mixed. Fixes #3366.

Problem and expected behavior

nativeBoundarySummary only counted the managed native header (RVA or size) and the native entry-point flag. The artifact classifier's rule is different: crossgen2 clears ILONLY on ReadyToRun output, so a clear ILONLY flag without the ReadyToRun signature means mixed CLR/native code. The boundary summary never applied that rule, so the two tools disagreed about the same bytes.

Change and scope

managedCliIsMixedClrNative(cli) in ManagedPeReader is now the only place this rule is defined. The artifact runtime family and the boundary summary both use it.
nativeBoundarySummary and cliNative read the ManagedCliHeader directly instead of the cli_native output projection, so the summary no longer re-derives facts from display fields. emptyInspection and buildNativeBoundaryInspection pass pe.cli.
The summary still reports true for a managed native header or a native entry point. The result schema is unchanged.

Contribution scope: an ordinary fix within existing boundaries, so no maintainer scope decision is needed.

Contract and boundary impact

Semantic owner and earliest changed stage: src/dotnet/ManagedPeReader.ts (CLI header facts). Consumers: ManagedArtifactInspector, ManagedNativeBoundaryHelpers.
Contribution scope fit and linked maintainer decision: not applicable (ordinary fix).
CLI and MCP/tool-catalog contract: no schema change. mixed_mode_or_native_header is now true for ILONLY-clear, non-ReadyToRun headers.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: the boundary summary now agrees with the artifact classification.
Target selection, authority, upstream behavior, and cleanup details: unchanged.
Generated metadata: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: src/dotnet/ManagedNativeBoundaryInspector.test.ts, agrees with artifact classification for $name. Cases: C++/CLI with a native method body, ILONLY clear with unreadable metadata (the emptyInspection path), pure IL, and ReadyToRun. With the source change reverted, the two mixed cases fail (2 failed, 14 passed).
Base reproduction (the #3366 generator through the built CLI), mixed_mode_or_native_header:

fixture	artifact classification	main	this PR
mixed (`cliFlags: 0` + native method)	`mixed-clr-native` / `cpp-cli-mixed`	`false`	`true`
ILONLY clear only	`mixed-clr-native` / `cpp-cli-mixed`	`false`	`true`
pure IL	`modern-dotnet` / `cil`	`false`	`false`
ReadyToRun	`modern-dotnet` / `cil-and-ready-to-run`	`true`	`true`

Real data: inspect-managed-native-boundaries JSON for real AWSSDK.Core.dll (net8.0; 82 native implementations, 2 P/Invokes) and itextsharp.dll is byte-identical to main's (Evidence ID excluded). Both take 0.43 s, with peak RSS of 311 MB (AWSSDK.Core) and 308 vs 315 MB (iTextSharp).
Remaining proof gaps: all 259 local NuGet DLLs are ILONLY. No real C++/CLI or ReadyToRun assembly was available, so those paths are covered only by fixtures through the built CLI.
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

npm run test:local -- src/dotnet/*.test.ts (14 files): 326 passed
npm run test:focused -- tests/boundary/providers/managed/*.test.ts tests/boundary/mcp/managed*.test.ts: 101 passed
npm run check:fast: passed
Built CLI on the #3366 fixtures and on real DLLs: as listed above

Compatibility, safety, and release

Breaking changes or migration steps: none. Field shapes are unchanged; one boolean is corrected.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no change.

Author readiness

Human owner:

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/3516 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/managed-mixed-mode-summary` --- ### 📝 Commits (2) - [`bb5091f`](https://github.com/morluto/rea/commit/bb5091f2b9da939840baaca836c17f381e2c4d86) fix(managed): report ILONLY-clear assemblies as mixed in boundary summaries - [`897d4da`](https://github.com/morluto/rea/commit/897d4da39a5589404ee4fbf08f631901ab1af61f) Merge remote-tracking branch 'origin/main' into fix/managed-mixed-mode-summary ### 📊 Changes **5 files changed** (+68 additions, -31 deletions) <details> <summary>View changed files</summary> 📝 `src/dotnet/ManagedArtifactInspector.ts` (+2 -3) 📝 `src/dotnet/ManagedNativeBoundaryHelpers.ts` (+21 -19) 📝 `src/dotnet/ManagedNativeBoundaryInspector.test.ts` (+29 -0) 📝 `src/dotnet/ManagedNativeBoundaryInspector.ts` (+9 -9) 📝 `src/dotnet/ManagedPeReader.ts` (+7 -0) </details> ### 📄 Description ## Summary `inspect-managed-native-boundaries` reported `summary.mixed_mode_or_native_header: false` for C++/CLI (ILONLY clear, not ReadyToRun) assemblies, even though `inspect-managed-artifact` classifies the same bytes as `mixed-clr-native` / `cpp-cli-mixed`. Fixes #3366. ## Problem and expected behavior `nativeBoundarySummary` only counted the managed native header (RVA or size) and the native entry-point flag. The artifact classifier's rule is different: crossgen2 clears ILONLY on ReadyToRun output, so a clear ILONLY flag *without* the ReadyToRun signature means mixed CLR/native code. The boundary summary never applied that rule, so the two tools disagreed about the same bytes. ## Change and scope - `managedCliIsMixedClrNative(cli)` in `ManagedPeReader` is now the only place this rule is defined. The artifact runtime family and the boundary summary both use it. - `nativeBoundarySummary` and `cliNative` read the `ManagedCliHeader` directly instead of the `cli_native` output projection, so the summary no longer re-derives facts from display fields. `emptyInspection` and `buildNativeBoundaryInspection` pass `pe.cli`. - The summary still reports `true` for a managed native header or a native entry point. The result schema is unchanged. Contribution scope: an ordinary fix within existing boundaries, so no maintainer scope decision is needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/dotnet/ManagedPeReader.ts` (CLI header facts). Consumers: `ManagedArtifactInspector`, `ManagedNativeBoundaryHelpers`. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix). - CLI and MCP/tool-catalog contract: no schema change. `mixed_mode_or_native_header` is now `true` for ILONLY-clear, non-ReadyToRun headers. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: the boundary summary now agrees with the artifact classification. - Target selection, authority, upstream behavior, and cleanup details: unchanged. - Generated metadata: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `src/dotnet/ManagedNativeBoundaryInspector.test.ts`, `agrees with artifact classification for $name`. Cases: C++/CLI with a native method body, ILONLY clear with unreadable metadata (the `emptyInspection` path), pure IL, and ReadyToRun. With the source change reverted, the two mixed cases fail (2 failed, 14 passed). - Base reproduction (the #3366 generator through the built CLI), `mixed_mode_or_native_header`: | fixture | artifact classification | main | this PR | |---|---|---|---| | mixed (`cliFlags: 0` + native method) | `mixed-clr-native` / `cpp-cli-mixed` | `false` | `true` | | ILONLY clear only | `mixed-clr-native` / `cpp-cli-mixed` | `false` | `true` | | pure IL | `modern-dotnet` / `cil` | `false` | `false` | | ReadyToRun | `modern-dotnet` / `cil-and-ready-to-run` | `true` | `true` | - Real data: `inspect-managed-native-boundaries` JSON for real `AWSSDK.Core.dll` (net8.0; 82 native implementations, 2 P/Invokes) and `itextsharp.dll` is byte-identical to main's (Evidence ID excluded). Both take 0.43 s, with peak RSS of 311 MB (AWSSDK.Core) and 308 vs 315 MB (iTextSharp). - Remaining proof gaps: all 259 local NuGet DLLs are ILONLY. No real C++/CLI or ReadyToRun assembly was available, so those paths are covered only by fixtures through the built CLI. - [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 - `npm run test:local -- src/dotnet/*.test.ts` (14 files): 326 passed - `npm run test:focused -- tests/boundary/providers/managed/*.test.ts tests/boundary/mcp/managed*.test.ts`: 101 passed - `npm run check:fast`: passed - Built CLI on the #3366 fixtures and on real DLLs: as listed above ## Compatibility, safety, and release - Breaking changes or migration steps: none. Field shapes are unchanged; one boolean is corrected. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no change. ## Author readiness Human owner:  - [ ] 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](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-10-31 01:09:30 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns