Skip to content

fix(gc): classify boxed string concat as poll-capable - #7896

Merged
proggeramlug merged 3 commits into
mainfrom
fix/7872-poll-capable-concat
Aug 12, 2026
Merged

fix(gc): classify boxed string concat as poll-capable#7896
proggeramlug merged 3 commits into
mainfrom
fix/7872-poll-capable-concat

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Closes #7872.

Summary

  • add js_string_concat_box to POLL_CAPABLE_RUNTIME, so every --moving-only checker arm sees the helper's path through js_dynamic_string_or_number_add to coercion/user code
  • extend the planted poll-reach self-test with that exact two-hop production edge

Reproduction

On the pre-fix main checkout:

$ python3 scripts/gc_root_dominance_check.py --audit-poll-reach
error: ALLOC_RE symbols that CALL a POLL_CAPABLE_RUNTIME symbol but are not in POLL_CAPABLE_RUNTIME:
  js_string_concat_box                             -> js_dynamic_string_or_number_add
AUDIT_EXIT=2

After this change, the same audit reports 3,791 exported symbols, 1,616 intra-runtime call edges, 384 ALLOC_RE matches, and no unlisted poll-reaching allocation wrapper.

Validation

  • python3 scripts/gc_root_dominance_check.py --self-test
  • all four static audits (--audit-alloc-re, --audit-poll-capable, --audit-poll-reach, --audit-immovable-sources)
  • python3 scripts/gc_gate_wiring_check.py
  • python3 scripts/check_test_registration.py
  • shadow corpus: 139/139 sources, 161 IR modules, 0 skips
    • root-store dominance: 0 violations; 40/40 planted violations caught
    • unrooted allocas: 0
    • stale registers: 18, within the pinned 39 budget
  • native corpus: 139/139 sources, 161 IR modules, 0 skips, 35,246 emitted statepoints / 21,290 non-empty live bundles
    • checked 35,085 safepoints and 50,900 relocations
    • 0 unrooted, 0 stale; 40/40 planted statepoint violations caught
  • exact before/after classification over the curated corpus found 27 newly poll-reaching compiled functions but 0 newly exposed stale windows
  • bash scripts/check_file_size.sh
  • git diff --check

Additional observation, not changed here: the dependency-scale Zod corpus emitted all 81 expected IR modules but its final link failed on unresolved intra-Zod symbols, identically with Perry's cache disabled. Running the checker directly over those emitted modules still met the normal floors (12,903 functions), reported 0 dominance violations, 0 unrooted allocas, and 89 stale uses within the 118 budget; the exact classification delta for this list entry was 0 newly exposed stale windows.

Summary by CodeRabbit

  • Bug Fixes

    • Improved garbage-collection safety checks for boxed string concatenation when coercion may execute user code.
    • Corrected poll-reachability analysis across indirect numeric coercion paths.
    • Added coverage for exact two-step coercion scenarios to improve audit accuracy and validation.
  • Documentation

    • Added a changelog entry describing the corrected moving-only audit behavior and verification results.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 59d513e9-7843-4e19-a25f-48667ab6f86f

📥 Commits

Reviewing files that changed from the base of the PR and between 23f09a0 and 6e55f06.

📒 Files selected for processing (1)
  • changelog.d/7896-boxed-concat-poll-reach.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • changelog.d/7896-boxed-concat-poll-reach.md

📝 Walkthrough

Walkthrough

The GC root-dominance checker classifies js_string_concat_box as poll-capable, models its coercion path through js_number_coerce, and verifies the result in its self-test. A changelog entry documents the correction.

Changes

GC poll-reach classification

Layer / File(s) Summary
Poll-capable classification and reach fixture
scripts/gc_root_dominance_check.py
Adds js_string_concat_box to POLL_CAPABLE_RUNTIME and models its two-hop path through js_dynamic_string_or_number_add to js_number_coerce.
Self-test and changelog
scripts/gc_root_dominance_check.py, changelog.d/7896-boxed-concat-poll-reach.md
Updates expected poll-reach results and documents the corrected boxed-concatenation audit classification.

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

Possibly related PRs

  • PerryTS/perry#7835: Changed js_string_concat_box to use dynamic coercion for non-string operands.
  • PerryTS/perry#7679: Expanded and validated POLL_CAPABLE_RUNTIME poll-reach classification.
  • PerryTS/perry#7284: Updated POLL_CAPABLE_RUNTIME and related checker self-tests.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: classifying boxed string concatenation as poll-capable for GC checks.
Description check ✅ Passed The description explains the change, linked issue, reproduction, validation commands, audit results, and corpus findings; missing optional template sections are non-critical.
Linked Issues check ✅ Passed The PR satisfies issue #7872 by updating poll-capable classification, adding the focused self-test, passing the audit, and reviewing corpus results.
Out of Scope Changes check ✅ Passed The changes are limited to the poll-capable runtime classification, its focused self-test, and related changelog documentation.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/7872-poll-capable-concat

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

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

@proggeramlug
proggeramlug marked this pull request as ready for review August 12, 2026 00:41
@proggeramlug

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/7896-boxed-concat-poll-reach.md`:
- Around line 1-3: Expand the changelog fragment to include concise context
referencing Issue `#7872`, plus the completed GC root-dominance moving-only audits
and corpus checks from the PR objective. Preserve the existing behavior summary
and record the validation results as history and validation notes.
🪄 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: 5306b7a8-ee26-4944-9e5d-3ac47e3e2d27

📥 Commits

Reviewing files that changed from the base of the PR and between c5a0341 and 23f09a0.

📒 Files selected for processing (2)
  • changelog.d/7896-boxed-concat-poll-reach.md
  • scripts/gc_root_dominance_check.py

Comment thread changelog.d/7896-boxed-concat-poll-reach.md
@proggeramlug
proggeramlug merged commit 08078f4 into main Aug 12, 2026
1 of 19 checks passed
@proggeramlug
proggeramlug deleted the fix/7872-poll-capable-concat branch August 12, 2026 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gc: js_string_concat_box is missing from POLL_CAPABLE_RUNTIME, blinding moving root-dominance arms

1 participant