fix(gc): float arithmetic proves a value is not a NaN-boxed pointer — main is red on 9 false positives

[proggeramlug]

Fixed

• gc-root-dominance-statepoints was red on main, on nine false positives (fix(gc): float arithmetic proves a value is not a NaN-boxed pointer — main is red on 9 false positives #7738). gc: reload js_closure_get_capture_bits results across a collection point (#7725) #7732 drove the native lowering's unrooted count to 0 and — correctly, per gate(gc): make the root-dominance corpus floor a two-sided ratchet #7706's precedent — deleted --max-unrooted rather than setting it to 0. With no budget, any spurious hit is a red build, and nine arrived.

  Every one had the same shape: a load double from a module global (@perry_global_…), reported as a stale register because the load crosses a statepoint, whose only use is float arithmetic:

  source (global): %r226 = load double, ptr @perry_global_…_ts__1, align 8
  stale use       : %r309 = fadd double %r226, %r308
  

  The global is let churnAcc = 0 in test_gap_repsel_gc_stress — a number. Perry represents every JS value as a NaN-boxed double, so "it is a double" says nothing about whether it is a pointer; but float arithmetic does. A NaN-box carries its tag in the exponent/mantissa bits and fadd destroys it, so codegen emits one only where it has already proven the operand numeric. The instruction is the proof. A stale copy of a number is just a number: nothing to rewrite, nothing to dereference.

  fcmp gets exactly the ordered predicates (oeq ogt oge olt ole one ord). An ordered comparison is false unless both operands are non-NaN, and every NaN-boxed reference is a NaN — so an ordered predicate on a boxed pointer is a constant false that codegen has no reason to emit. The unordered ones are excluded deliberately: fcmp uno is precisely how a NaN-box tag check is written, and treating it as numeric proof would blind the checker to the pointer case it exists for.

  The filter is per-use, not per-source. A register with one arithmetic use and one dereference is still reported for the dereference — the case that matters, and the one a source-level filter would have silently dropped.

  Verified in both directions rather than assumed. --self-test still passes, so the checker can still fail. And the predicate was exercised against ten instruction shapes: fadd / ordered fcmp / fmul fast are filtered; js_object_get_field_by_name_f64, bitcast … to i64, store, fcmp uno, fcmp une, js_nanbox_get_pointer and inttoptr are all still reported. Corpus: 131/131 sources compiled, 0 skipped → within budget: unrooted 0 <= 0.

Summary by CodeRabbit

• Bug Fixes

  • Reduced false-positive stale-pointer reports for values used solely in floating-point arithmetic or ordered comparisons.
  • Continued reporting relevant pointer-related uses, including dereferences, conversions, stores, unordered comparisons, and NaN-box checks.
  • Improved analysis reliability across multiple instruction patterns and verified results against the test corpus.

• Documentation

  • Added release notes describing the analysis improvements and validation coverage.

• Chores

  • Updated the application version to 0.5.1430.

[coderabbitai]

📝 Walkthrough

Walkthrough

The GC root dominance checker now treats floating-point arithmetic and ordered comparisons as numeric proofs. It excludes these uses from stale-pointer reports, retains pointer-related reporting, documents the fix, and updates the version to 0.5.1430.

Changes

Numeric proof filtering

Layer / File(s)	Summary
Numeric-use detection scripts/gc_root_dominance_check.py	Adds recognition for floating-point arithmetic and ordered comparisons whose operands are proven raw numbers.
Hazard scan integration and validation scripts/gc_root_dominance_check.py, changelog.d/7738-dominance-numeric-proof.md	Skips recognized numeric-proof uses during statepoint hazard scanning. The changelog records retained pointer-use reporting and verification results.
Release version updates CLAUDE.md, Cargo.toml	Updates the documented and workspace package versions from 0.5.1429 to 0.5.1430.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• PerryTS/perry#7289: Extends the same root-dominance checker.
• PerryTS/perry#7724: Modifies the same checker to address different false positives.

Suggested labels: bug

Suggested reviewers: jdalton

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the fix and verification, but it omits the required template sections, checkboxes, and explicit test commands.	Use the repository template, add all required sections and test-plan checkboxes, and address the prohibited Cargo.toml and CLAUDE.md metadata edits.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the numeric-proof fix and its purpose of removing nine false positives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/7738-dominance-numeric-proof

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@changelog.d/7738-dominance-numeric-proof.md`:
- Around line 3-5: In the changelog entry, replace the ambiguous phrase “and
nine arrived” with wording that explicitly states that nine false positives
remained, while preserving the surrounding context and meaning.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 17cd076e-8427-4759-9393-f3e51f9d3c08

📥 Commits

Reviewing files that changed from the base of the PR and between 3fde6a4 and 63a3a17.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (4)

• CLAUDE.md
• Cargo.toml
• changelog.d/7738-dominance-numeric-proof.md
• scripts/gc_root_dominance_check.py

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Clarify what “nine” refers to.

The phrase “and nine arrived” is ambiguous in a release note. Replace it with “and nine false positives remained” so the sentence identifies the affected findings.

Proposed wording

-  With no budget, any spurious hit is a red build, and nine arrived.
+  With no budget, any spurious hit is a red build, and nine false positives remained.

The supplied LanguageTool finding points to this wording.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	- **`gc-root-dominance-statepoints` was red on `main`, on nine false positives (#7738).** #7732 drove the native lowering's unrooted count to 0 and — correctly, per #7706's precedent — **deleted** `--max-unrooted` rather than setting it to 0. With no budget, any spurious hit is a red build, and nine arrived.
	Every one had the same shape: a `load double` from a module global (`@perry_global_…`), reported as a stale register because the load crosses a statepoint, whose **only use is float arithmetic**:
	- **`gc-root-dominance-statepoints` was red on `main`, on nine false positives (`#7738`).** `#7732` drove the native lowering's unrooted count to 0 and — correctly, per `#7706`'s precedent — **deleted** `--max-unrooted` rather than setting it to 0. With no budget, any spurious hit is a red build, and nine false positives remained.
	Every one had the same shape: a `load double` from a module global (`@perry_global_…`), reported as a stale register because the load crosses a statepoint, whose **only use is float arithmetic**:

🧰 Tools

🪛 LanguageTool

[grammar] ~5-~5: Ensure spelling is correct
Context: ...it is a red build, and nine arrived. Every one had the same shape: a load double fro...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@changelog.d/7738-dominance-numeric-proof.md` around lines 3 - 5, In the
changelog entry, replace the ambiguous phrase “and nine arrived” with wording
that explicitly states that nine false positives remained, while preserving the
surrounding context and meaning.

Source: Linters/SAST tools