Skip to content

GC testing: PERRY_GC_FORCE_EVACUATE is inert for gc()-driven tests (full mark-sweep + forced conservative scan) — stress claims may be unsupported #6946

Closed

Description

@proggeramlug

Summary

PERRY_GC_FORCE_EVACUATE=1 has no effect on a test that drives collection with an explicit gc() call, and such a test additionally cannot expose raw-local rooting bugs. Both properties are structural, verified on origin/main. A test written that way looks like GC stress coverage and is not.

Mechanism (verified on origin/main)

js_gc_collect → manual_gc_collect_now (crates/perry-runtime/src/gc/policy.rs):

fn manual_gc_collect_now() {
    let _scan = super::roots::ManualGcScanGuard::force_full_scan();   // (1)
    ...
    gc_collect_full_mark_sweep_with_trigger(...)                      // (2)
}
  1. force_full_scan() pins raw locals. The conservative native-stack scan is forced on, so a raw NaN-boxed word in a Rust local or an unrooted slot is treated as a root. That is exactly the class of bug the rooting work (runtime: audit dynamic_arith operand rooting — raw NaN-boxed operands held across GC-capable to_numeric coercions (pre-existing, file-wide) #6655, runtime: audit ToPropertyKey receiver/value rooting — raw receiver and stored value held across GC-capable key coercions #6935/fix(runtime): root receivers and stored values across GC-capable property-key coercions (#6935) #6941) targets — a gc()-driven test cannot observe it. Note production resolves Auto → SkipDisabled (gc/roots.rs), so this is a test-only safety net: a green gc()-driven test can be a production-mode lie.
  2. It runs a FULL mark-sweep, deliberately not the minor path (the in-code comment explains why: dead large/tenured objects must be reclaimed to match --expose-gc semantics). But PERRY_GC_FORCE_EVACUATE is read only inside the minor cycle (gc/mod.rs: let force_evacuation = gc_force_evacuate_enabled(); within GcCollectionKind::Minor, plus gc/policy.rs / gc/oldgen.rs evacuation paths). A full mark-sweep never evacuates, so nothing moves, so a stale pointer is trivially valid.

Independently measured in #6942: across 10 collections in a 12M-allocation program, PERRY_GC_DIAG=1 printed zero [gc-evac-policy] / [gc-copy-minor] lines.

Blast radius (measured, not assumed)

  • 9 gap files call gc() — test_gc_array_layout_mask, test_gc_aggressive_forced, test_gc_class_object_forced, test_issue_1425_gc_unsafe_zones, test_issue_1830_gc_in_catch_after_deep_throw, test_issue_2656_weakref_finalization_gc, test_issue_4977_gc_toplevel_locals, test_issue_callable_with_properties, test_memory_class_object_churn. Any evacuation/rooting claim resting on these under FORCE_EVACUATE is unsupported.
  • The representation-selection corpus does NOT call gc() (verified: 0 occurrences across all test_gap_repsel_* / test_gap_specabi_* files). Those rely on allocation pressure, which can drive automatic minors — where FORCE_EVACUATE does apply and the conservative scan is not forced. Whether their churn actually crosses the threshold is unverified and is the open question; gc: no reachable configuration exercises an evacuating minor with unpinned runtime locals — the #6655/#6935 bug class is untestable #6942 notes a common "20000 short-lived objects" prelude triggers no GC at all because non-escaping allocations are optimized away.

Why it matters

"Byte-exact under PERRY_GC_FORCE_EVACUATE=1" has been used as the standard validation ritual across a long series of GC and representation PRs. For gc()-driven tests that sentence asserts nothing about evacuation or rooting.

Suggested fixes

  1. Make the trap loud: if PERRY_GC_FORCE_EVACUATE=1 is set and a manual gc() runs, emit a diagnostic (or have PERRY_GC_DIAG state plainly that force-evacuate was ignored on this path).
  2. Give tests a real knob — a documented way to drive an evacuating minor deterministically (perry/gc's minor() evacuates, but per gc: no reachable configuration exercises an evacuating minor with unpinned runtime locals — the #6655/#6935 bug class is untestable #6942 it engages force_full_scan() and so re-pins raw locals; a variant without that pinning is what rooting tests actually need).
  3. Require liveness proof in GC tests: assert via PERRY_GC_DIAG that a collection (and, for evacuating arms, moved_objects > 0) actually occurred, rather than trusting the env var.

Related: #6942 (stress arms measured inert, with three proposed harness routes), #6910 (mark/rewrite parity; its tests correctly pin ConservativeStackScanMode::Disabled — the pattern to copy).

Activity

  1. proggeramlug commented on Aug 9, 2026

    @proggeramlug
    ContributorAuthor

    Half of this is now resolved, and the half that remains is the more interesting one. Updating rather than closing.

    Resolved by #7657 (v0.5.1374): the second property — "such a test additionally cannot expose raw-local rooting bugs". The mechanism this issue quotes,

    fn manual_gc_collect_now() {
        let _scan = super::roots::ManualGcScanGuard::force_full_scan();   // (1)

    no longer exists. gc() runs on precise roots; the ConservativeScanSite::ManualCollect variant was deleted rather than left unconstructible. So a gc()-driven test now does expose raw-local rooting bugs, and the detector added alongside it (explicit_gc_collects_precisely_and_a_native_stack_plant_dies) asserts exactly that: an object reachable only from a pointer-shaped word in a live native frame is swept.

    Worth recording that the forced scan turned out to be a workaround for a precise-rooting hole (#4977 → #4998) that the rooting campaign had since closed from the other end — grandfathered, then quoted for two months as a requirement.

    Still open: property one. PERRY_GC_FORCE_EVACUATE is read on the minor path while gc() runs a full mark-sweep, so the knob remains inert for gc()-driven tests. That is unchanged by #7657 and is what this issue should now be scoped to.

    The consequence is also unchanged and is the reason to keep it open: a test written as gc(); assertFreed() under PERRY_GC_FORCE_EVACUATE=1 looks like evacuation stress coverage and is not — CLAUDE.md's hazard 4, and one of the three worked examples in it.

  2. proggeramlug commented on Aug 9, 2026

    @proggeramlug
    ContributorAuthor

    The remaining half is fixed. PR #7684.

    An explicit gc() under PERRY_GC_FORCE_EVACUATE now runs an evacuating minor first, then the full mark-sweep. #7657 is what made this possible: this site used to take ManualGcScanGuard::force_full_scan, and a forced conservative scan makes the copying minor ineligible outright (CopiedMinorFallbackReason::ConservativeStack). With precise roots a copying minor here is exactly as sound as the full sweep that follows, which still runs and still reclaims dead large/tenured objects, so --expose-gc semantics are unchanged.

    One subtlety worth recording, because it is this issue's own bug in a new place: gc_collect_minor_with_trigger can escalate to a full mark-sweep via two throughput-pacing predicates (copied_minor_promotion_handoff_due, arena_growth_full_escalation_due), and a full sweep moves nothing. Left alone, the fix would have handed the caller a non-moving collection under a knob whose entire name is about relocation. There is now a FullEscalation::Refused scoped to this one caller; every automatic path keeps Allowed, so the stub reclamation those predicates exist for is untouched.

    Shown able to fail. explicit_gc_under_forced_evacuation_runs_a_moving_minor asserts both that a copying minor completed and that the survivor's address changed — the counter without the address cannot tell a copying minor that moved nothing from one that moved this object. Deleting the forced minor and re-running:

    SABOTAGED RUN rc=101
    compile errors: 0
    Running unittests lines: 1
    test result: FAILED. 0 passed; 1 failed; …
    an explicit gc() under forced evacuation must run a COPYING minor (before=0 after=0).
    

    Suggested fix 3 ("require liveness proof in GC tests") is also served: crate::gc::copying_minor_cycles() and moved_objects_total() are now public counters any test can assert on, added for #7604 in the same PR.

    CLAUDE.md's kill-policy keeps this as a precedent with both halves marked closed — the story is the argument for the policy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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