[PR #2437] [MERGED] fix(managed): preserve negative zero in CIL operands #5229

New issue

Closed

opened 2026-10-31 01:04:58 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:04:58 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2437
Author: @fisHarly0
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/managed-negative-zero

📝 Commits (1)

6e17d06 fix(managed): preserve negative zero in CIL operands

📊 Changes

2 files changed (+66 additions, -2 deletions)

View changed files

📝 src/dotnet/ManagedMemberInstructionDecoder.ts (+6 -2)
📝 tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts (+60 -0)

📄 Description

Summary

Preserve negative zero in managed CIL floating-point operands so that changing a method from returning +0 to returning -0 is reported as a CIL change.

Problem and expected behavior

Both ldc.r4 and ldc.r8 used String(value), which turns -0 into "0". Consequently the instruction anchors and normalized IL hashes were identical for different constants, and member comparison reported unchanged with an exact-il-signature match. Signed zero must remain observable in the decoded constant and comparisons.

No linked issue: this was reproduced directly against base commit 4773c152b while reviewing the static managed decoder.

Change and scope

Use one floating-point operand formatter at the decoder boundary. It emits "-0" for negative zero and preserves the existing formatting for other values. Anchors, normalized hashes, and member comparison consume that representation without downstream special cases.

Add PE-fixture regressions for both operand widths covering inspection, changed comparisons, distinct normalized hashes, and unchanged comparisons of repeated negative-zero methods.

Contract and boundary impact

Semantic owner and earliest changed stage: ManagedMemberInstructionDecoder floating-point operand decoding.
Contribution scope fit and linked maintainer decision: ordinary static-parser bug fix; maintainer scope decision not applicable.
CLI and MCP/tool-catalog contract: no schema, argument, or catalog changes. A negative-zero constant is now "-0", and a sign-only change is reported as changed with dimension cil.
Provider, bridge, target-format, or platform compatibility: existing managed PE/CLI provider; no engine or bridge changes.
Evidence, artifact, provenance, or reconstruction contract: normalized hashes now distinguish negative zero; raw artifact/IL hashes and provenance are preserved. Previously retained observations need reinspection to obtain the corrected operand and normalized hash.
Target selection, authority, upstream behavior, and cleanup: unchanged read-only analysis of caller-selected bytes; no new resource owner.
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: parameterized ldc.r4 and ldc.r8 regressions in managedMemberComparisonFromMetadata.test.ts, using actual PE bytes and the production inspector/comparator.
Base reproduction: both new tests failed before the fix, receiving status: "unchanged", dimensions: [], and basis: "exact-il-signature".
Executed compiled CLI output, for both widths (selected fields):

{"operand":"-0","status":"changed","dimensions":["cil"],"sameInputStatus":"unchanged"}

Remaining proof gaps: the complete suite/coverage and a stdio MCP session were not run. The compiled CLI and production inspection/comparison boundaries were exercised on Windows.
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

Windows, Node 24.18.0, npm 11.16.0, locked dependencies:

npm run check — passed (typecheck, build, format, lint/module boundaries, knip, metadata, architecture guards; lint reports 0 errors and 986 warnings).
npm run test:focused -- src/dotnet/ManagedCilDecoding.test.ts tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts tests/boundary/providers/managed/managedMemberComparison.test.ts tests/boundary/providers/managed/managedMemberComparisonPartialBodies.test.ts — 58 tests passed across 4 files.
npm run verify:test-discovery — all 1004 test files discovered exactly once.
Compiled node scripts/rea.mjs inspect-managed-members <negative-zero.dll> --json and compare-managed-members <positive-zero.dll> <negative-zero.dll> --json — passed for both widths; same-input negative-zero controls also remained unchanged.
git diff --check — passed.

Compatibility, safety, and release

Breaking changes or migration steps: no API/schema migration; reinspection corrects previously retained negative-zero observations.
Real Hopper/Ghidra, browser, or OS coverage: Windows built-in managed static analysis verified; optional engines and other OSes not run, as no engine/platform-specific code changed.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new execution, network, mutation, or target authority.

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/2437 **Author:** [@fisHarly0](https://github.com/fisHarly0) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/managed-negative-zero` --- ### 📝 Commits (1) - [`6e17d06`](https://github.com/morluto/rea/commit/6e17d06c3de8cfd0ac09310f3d9029e114337d62) fix(managed): preserve negative zero in CIL operands ### 📊 Changes **2 files changed** (+66 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/dotnet/ManagedMemberInstructionDecoder.ts` (+6 -2) 📝 `tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts` (+60 -0) </details> ### 📄 Description ## Summary Preserve negative zero in managed CIL floating-point operands so that changing a method from returning `+0` to returning `-0` is reported as a CIL change. ## Problem and expected behavior Both `ldc.r4` and `ldc.r8` used `String(value)`, which turns `-0` into `"0"`. Consequently the instruction anchors and normalized IL hashes were identical for different constants, and member comparison reported `unchanged` with an `exact-il-signature` match. Signed zero must remain observable in the decoded constant and comparisons. No linked issue: this was reproduced directly against base commit `4773c152b` while reviewing the static managed decoder. ## Change and scope Use one floating-point operand formatter at the decoder boundary. It emits `"-0"` for negative zero and preserves the existing formatting for other values. Anchors, normalized hashes, and member comparison consume that representation without downstream special cases. Add PE-fixture regressions for both operand widths covering inspection, changed comparisons, distinct normalized hashes, and unchanged comparisons of repeated negative-zero methods. ## Contract and boundary impact - Semantic owner and earliest changed stage: `ManagedMemberInstructionDecoder` floating-point operand decoding. - Contribution scope fit and linked maintainer decision: ordinary static-parser bug fix; maintainer scope decision not applicable. - CLI and MCP/tool-catalog contract: no schema, argument, or catalog changes. A negative-zero constant is now `"-0"`, and a sign-only change is reported as `changed` with dimension `cil`. - Provider, bridge, target-format, or platform compatibility: existing managed PE/CLI provider; no engine or bridge changes. - Evidence, artifact, provenance, or reconstruction contract: normalized hashes now distinguish negative zero; raw artifact/IL hashes and provenance are preserved. Previously retained observations need reinspection to obtain the corrected operand and normalized hash. - Target selection, authority, upstream behavior, and cleanup: unchanged read-only analysis of caller-selected bytes; no new resource owner. - 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: parameterized `ldc.r4` and `ldc.r8` regressions in `managedMemberComparisonFromMetadata.test.ts`, using actual PE bytes and the production inspector/comparator. - Base reproduction: both new tests failed before the fix, receiving `status: "unchanged"`, `dimensions: []`, and `basis: "exact-il-signature"`. - Executed compiled CLI output, for both widths (selected fields): ```json {"operand":"-0","status":"changed","dimensions":["cil"],"sameInputStatus":"unchanged"} ``` - Remaining proof gaps: the complete suite/coverage and a stdio MCP session were not run. The compiled CLI and production inspection/comparison boundaries were exercised on Windows. - [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 Windows, Node 24.18.0, npm 11.16.0, locked dependencies: - `npm run check` — passed (typecheck, build, format, lint/module boundaries, knip, metadata, architecture guards; lint reports 0 errors and 986 warnings). - `npm run test:focused -- src/dotnet/ManagedCilDecoding.test.ts tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts tests/boundary/providers/managed/managedMemberComparison.test.ts tests/boundary/providers/managed/managedMemberComparisonPartialBodies.test.ts` — 58 tests passed across 4 files. - `npm run verify:test-discovery` — all 1004 test files discovered exactly once. - Compiled `node scripts/rea.mjs inspect-managed-members <negative-zero.dll> --json` and `compare-managed-members <positive-zero.dll> <negative-zero.dll> --json` — passed for both widths; same-input negative-zero controls also remained unchanged. - `git diff --check` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: no API/schema migration; reinspection corrects previously retained negative-zero observations. - Real Hopper/Ghidra, browser, or OS coverage: Windows built-in managed static analysis verified; optional engines and other OSes not run, as no engine/platform-specific code changed. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new execution, network, mutation, or target authority. ## 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:04:58 +00:00