fix(runtime): root string/regex receivers across the ToString argument coercion (#6949 shape a)

[proggeramlug]

The six replace/replaceAll entry points root their raw receiver across the argument coercion (#6949 shape a).

js_string_coerce used as a plain ToString argument coercion allocates for every shape except an already-heap STRING_TAG value (builtins::string_coerce_is_inert): an SSO short string materialises onto the heap, a number/bool/null/BigInt builds its stringification, and a POINTER_TAG object runs a user toString/valueOf. Any of those can collect and evacuate. Rust evaluates arguments left to right, so the raw *const StringHeader receiver is copied before the coercion runs — the copy survives, the pointee moves, and the callee dereferences the stale one.

Fixed at all six sites in regex/replace_fn.rs, using the same RuntimeHandleScope idiom #6943 established for the property-key half of this family:

function	rooted
js_string_replace_string_dyn	s, pattern
js_string_replace_all_string_dyn	s, pattern
js_string_replace_search_dyn	s
js_string_replace_all_search_dyn	s
js_string_replace_regex_dyn	s, re
js_string_replace_all_regex_dyn	s, re

The two regex entry points matter beyond byte-reading: re is a RegExpHeader, so a stale one is consulted for its compiled pattern, not merely for characters.

Stated plainly: I could not produce a failing witness. A 200k-iteration fixture driving all six entry points with non-string replacements (so the coercion always allocates) produces node-identical output on both arms, including under PERRY_GC_ZEAL=1 with PERRY_GC_PROTECT_FROMSPACE=1 at depth 200 — and that run is not vacuous: the quarantine retired 27 page-sets and protected 179 MB, so evacuation genuinely happened. The unfixed baseline survives it 3/3 as well.

That is characteristic of this class rather than evidence against it — the window needs the pointee to move during that specific coercion, and #7154's family is documented as invisible to every runtime probe at the moment of collection. The justification here is structural and identical to the one #6943 shipped on: a raw heap pointer held across a call that can allocate is a defect by the repo's own rooting invariant, whether or not today's allocator layout happens to expose it.

Scope is deliberately shape (a) of the three the issue enumerates. Shape (b) (constructors holding a fresh obj across a later coercion, in messaging.rs, disposable.rs, boxed_primitives.rs, construct.rs) and the third shape (raw JSValues parked in Rust Vecs across allocations, in groupby.rs / define_properties.rs) are untouched — the third in particular is a different mechanism, since no GC scanner can see a Vec<f64> at all, and the issue itself flags it as needing its own decision.

Verified: cargo test -p perry-runtime --lib 2051 passed / 0 failed; test_gap_string 5/5, test_gap_regex 3/3; fmt, file-size and the addr-class audit all clean.

Summary by CodeRabbit

• Bug Fixes

  • Improved replace and replaceAll reliability when converting replacement values or search patterns.
  • Prevented potential failures caused by memory cleanup during string and regular expression replacement operations.
  • Applied fixes across both string-based and regular-expression replacement paths.

• Documentation

  • Added a changelog entry describing the replacement stability improvements.

[coderabbitai]

📝 Walkthrough

Walkthrough

The six dynamic replace and replaceAll entry points now root raw subjects, patterns, and RegExp values across potentially allocating coercions. The dispatchers pass relocated-safe pointers to downstream replacement functions. The changelog records the scope and validation results.

Changes

Regex replacement GC safety

Layer / File(s)	Summary
Root operands before coercion crates/perry-runtime/src/regex/replace_fn.rs, changelog.d/7811-string-coerce-operand-rooting.md	String and regex replace and replaceAll dispatchers root operands across replacement and search-value coercion, then pass current pointers to downstream replacement functions. The changelog documents the affected paths and validation results.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• runtime: a fourth unrooted-operand family — js_string_coerce as a plain ToString argument coercion (string built-ins, constructors) #6949 — Covers the same family of stale-pointer fixes for string built-ins, including rooting receivers, patterns, regexes, and coercion operands.

Possibly related PRs

• PerryTS/perry#7227 — Related GC-rooting fixes for coercion paths and RegExp receiver handling.
• PerryTS/perry#7240 — Applies related receiver-rooting fixes to replace and replaceAll dispatchers.
• PerryTS/perry#6975 — Fixes GC-rooting gaps around coercion-capable runtime operations.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly and concisely describes the main receiver-rooting fix in string and regex replace operations.
Description check	✅ Passed	The description clearly covers the change, affected entry points, related issue, scope, risks, and verification results, although it omits the template checklist.

✨ 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/6949a-string-coerce-operand-rooting

---

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]

🧹 Nitpick comments (1)

changelog.d/7811-string-coerce-operand-rooting.md (1)

1-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Condense this fragment into one release-note entry.

Remove the internal coercion analysis, test diagnostics, issue-shape scope, and baseline results. State the shipped behavior in one concise entry.

Proposed release-note text

-**The six `replace`/`replaceAll` entry points root their raw receiver across the argument coercion** (`#6949` shape a).
-
-`js_string_coerce` used as a plain **ToString argument** coercion allocates for every shape except an already-heap `STRING_TAG` value (`builtins::string_coerce_is_inert`): an SSO short string materialises onto the heap, a number/bool/null/BigInt builds its stringification, and a `POINTER_TAG` object runs a user `toString`/`valueOf`. Any of those can collect and evacuate. Rust evaluates arguments left to right, so the raw `*const StringHeader` receiver is copied *before* the coercion runs — the copy survives, the pointee moves, and the callee dereferences the stale one.
-
-Fixed at all six sites in `regex/replace_fn.rs`, using the same `RuntimeHandleScope` idiom `#6943` established for the property-key half of this family:
-
-| function | rooted |
-|---|---|
-| `js_string_replace_string_dyn` | `s`, `pattern` |
-| `js_string_replace_all_string_dyn` | `s`, `pattern` |
-| `js_string_replace_search_dyn` | `s` |
-| `js_string_replace_all_search_dyn` | `s` |
-| `js_string_replace_regex_dyn` | `s`, `re` |
-| `js_string_replace_all_regex_dyn` | `s`, `re` |
-
-The two regex entry points matter beyond byte-reading: `re` is a `RegExpHeader`, so a stale one is consulted for its compiled pattern, not merely for characters.
-
-**Stated plainly: I could not produce a failing witness.** A 200k-iteration fixture driving all six entry points with non-string replacements (so the coercion always allocates) produces node-identical output on both arms, including under `PERRY_GC_ZEAL=1` with `PERRY_GC_PROTECT_FROMSPACE=1` at depth 200 — and that run is not vacuous: the quarantine retired **27 page-sets and protected 179 MB**, so evacuation genuinely happened. The unfixed baseline survives it 3/3 as well.
-
-That is characteristic of this class rather than evidence against it — the window needs the pointee to move *during* that specific coercion, and `#7154`'s family is documented as invisible to every runtime probe at the moment of collection. The justification here is structural and identical to the one `#6943` shipped on: a raw heap pointer held across a call that can allocate is a defect by the repo's own rooting invariant, whether or not today's allocator layout happens to expose it.
-
-Scope is deliberately shape (a) of the three the issue enumerates. Shape (b) (constructors holding a fresh `obj` across a later coercion, in `messaging.rs`, `disposable.rs`, `boxed_primitives.rs`, `construct.rs`) and the third shape (raw `JSValue`s parked in Rust `Vec`s across allocations, in `groupby.rs` / `define_properties.rs`) are untouched — the third in particular is a different mechanism, since no GC scanner can see a `Vec<f64>` at all, and the issue itself flags it as needing its own decision.
-
-Verified: `cargo test -p perry-runtime --lib` 2051 passed / 0 failed; `test_gap_string` 5/5, `test_gap_regex` 3/3; fmt, file-size and the addr-class audit all clean.
+Fix GC safety in dynamic `String.prototype.replace` and `String.prototype.replaceAll` when argument coercion can relocate string or RegExp operands.

Based on learnings: changelog fragments must describe final shipped behavior as one coherent release-note entry.

🤖 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/7811-string-coerce-operand-rooting.md` around lines 1 - 24,
Condense the changelog fragment into one concise release-note entry describing
that all six string and regex replace/replaceAll entry points now root their
receivers and relevant arguments across coercions, preventing stale heap
references during garbage collection. Remove the internal analysis, test
results, issue-shape discussion, and implementation diagnostics.

Source: Learnings

🤖 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.

Nitpick comments:
In `@changelog.d/7811-string-coerce-operand-rooting.md`:
- Around line 1-24: Condense the changelog fragment into one concise
release-note entry describing that all six string and regex replace/replaceAll
entry points now root their receivers and relevant arguments across coercions,
preventing stale heap references during garbage collection. Remove the internal
analysis, test results, issue-shape discussion, and implementation diagnostics.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f82b2398-286a-402d-a82d-301a74871b9c

📥 Commits

Reviewing files that changed from the base of the PR and between 1804991 and 706473d.

📒 Files selected for processing (2)

• changelog.d/7811-string-coerce-operand-rooting.md
• crates/perry-runtime/src/regex/replace_fn.rs