Skip to content

GC: js_object_assign_one's string-source arm held a raw borrow into the source StringHeader across its allocating copy loop #7215

Description

@proggeramlug

Summary

object_assign_string_source (crates/perry-runtime/src/object/alloc.rs) decoded the SOURCE string once into a raw (*const u8, len) view and held that borrow across every allocation in its copy loop. Found in CodeRabbit review of #7207, which had rooted the other three values in that same function.

str_bytes_from_jsvalue returns hdr + size_of::<StringHeader>() for any string past the SSO limit (SHORT_STRING_MAX_LEN = 5 bytes), string_storage_alloc allocates header and payload as one contiguous arena_alloc_gc block, and GC_TYPE_STRING carries movable = true. The decoder's own safety note forbids exactly this use:

Callers must not hold this pointer past a subsequent scratch modification or a GC cycle that could sweep the heap-backed StringHeader.

The loop held it across three allocation points per character (two js_string_from_bytes, plus the write funnel's key interning / keys-array growth).

Measured, not asserted

An instrumented runtime rooted both the source and the target inside the loop and counted observed relocations. On a 26 001-character source, evacuating arm, PERRY_GC_FORCE_EVACUATE=1:

[probe7214] chars=26001 src_moves=0 tgt_moves=1

Under the threshold the source is a movable nursery string, but one call then allocates too little to reliably span a collection. Four shapes (209 / 8 000 / 26 001 chars × 4 arm configurations, up to 120 cycles) all stayed clean.

So there is no runtime witness, and the shipped test says so. The exposure is real and narrow: a source in the band just under 16 KiB is both movable and long enough to allocate ~32 000 times. The safety margin rests entirely on a tunable constant.

Fixed

#7215 snapshots the characters once before the loop (s.to_string()), matching the expandos snapshot precedent a few lines down in the same file, on a path that is already O(n).

The general lesson, which is bigger than this call site

str_bytes_from_jsvalue has ~N callers and its contract is stated only in a doc comment. Any caller that allocates between the decode and the last use of the view has this bug, and no static gate reaches it: gc_root_dominance_check.py (both modes) reads emitted LLVM IR, and this is runtime Rust. #7207's --unrooted-allocas mode and #7206's --stale-registers mode are both blind to it by construction.

Worth an audit of the other callers, and worth considering whether the decoder should return an owned/rooted form on the heap-string path so the contract is enforced by the type rather than by a comment.

Refs #7207, #7200, #7154.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions