Skip to content

temp_root_{argument,operand}_temporaries assert the pre-#7487 FFI spelling: 10 tests fail and the 8 that pass are vacuous — the #6951 contract has no working coverage #7503

Description

@proggeramlug

Split out of #7493. These failures are not the #7370 lowering default — PERRY_RS4GC=0 moves temp_root_argument_temporaries not at all (3/7 either way). They are #7487's, and the passing half of both suites is worse than the failing half.

Symptom

crates/perry-codegen/tests/temp_root_argument_temporaries.rs — 3 passed, 4 failed
crates/perry-codegen/tests/temp_root_operand_temporaries.rs — 13 passed, 6 failed

Failing (all with the same shape, ir.contains("call i32 @js_gc_temp_root_push") is false):

  • console_argument_accumulator_is_temp_rooted
  • array_literal_roots_elements_before_an_allocating_element
  • symbol_keyed_typed_array_read_is_still_temp_rooted
  • unproven_key_typed_array_read_is_still_temp_rooted
  • map_set_key_is_rooted_across_an_allocating_value
  • concat_accumulator_is_rooted_and_written_back
  • constructor_arguments_are_rooted_across_the_instance_allocation
  • wtf8_literal_operand_is_rooted_not_merely_reused
  • the_new_instance_is_rooted_across_the_constructor_body
  • the_object_assign_accumulator_is_rooted_across_each_source

Cause

#7487 re-lowered temp roots onto pooled frame allocas: push became a store plus the same bind a named local's store emits, get a load, truncate a slot zero. js_gc_temp_root_push / _get / _set / _truncate survive only on the FFI fallback arm, which is taken when reserve_shadow_slot() returns None — and it returns Some under both lowerings. So the spelling every assertion in these two files searches for is now emitted essentially never.

The contract itself is intact. console.log("label", churn()) today:

%r2 = alloca ptr addrspace(1)                 ; the pooled temp slot
%r1 = call i64 @js_array_alloc(i32 2)
%rs4gc.s1 = inttoptr i64 %r1 to ptr addrspace(1)
store ptr addrspace(1) %rs4gc.s1, ptr %r2     ; push
%r7.rs4p = load ptr addrspace(1), ptr %r2     ; re-read before use
%r8 = call i64 @js_array_push_f64(i64 %r7, double %r6)
%rs4gc.s2 = inttoptr i64 %r8 to ptr addrspace(1)
store ptr addrspace(1) %rs4gc.s2, ptr %r2     ; write the reallocated pointer back
...
store ptr addrspace(1) null, ptr %r2          ; truncate

Every property #6951 was written to pin is there. Only the spelling changed.

The worse half

The tests that still pass in these files are the negative direction — !ir.contains("call i32 @js_gc_temp_root_push") — and they now hold for every program, rooted or not:

  • non_allocating_element_list_emits_no_rooting_calls
  • buffer_element_operand_is_not_temp_rooted
  • buffer_index_get_operand_is_not_temp_rooted
  • map_set_with_a_non_allocating_value_emits_no_rooting_calls
  • string_method_with_non_allocating_args_emits_no_rooting_calls
  • constructor_arguments_on_plain_locals_emit_no_rooting_calls
  • registered_root_operands_are_reloaded_rather_than_rooted
  • string_literal_concat_operand_is_not_re_derived_when_nothing_collects

That is CLAUDE.md hazard 4 — the gate runs, its subject does not. #6996/#6997's whole point was that rooting a value which can never be collected is pure cost on the path that exists because it was optimised, and nothing measures that any more.

So the net position: the #6951 / #6969 / #6970 / #6971 / #7114 / #7154 / #7200 temp-root contract has no working coverage in either direction. Ten tests fail, eight pass without asserting anything.

What to build

Re-point both files at the pooled form. The assertion has to name the value — "some temp root exists" is too weak, since a named local's slot looks identical:

  • find the register the fixture's producer defines (js_array_alloc, js_string_concat, js_object_assign_one, …);
  • assert it is stored into an entry alloca before the next allocating call;
  • assert the consuming call reads its operand with a load from that same alloca, not from the producer's register;
  • for _set-shaped producers (concat, Object.assign), assert the post-call value is stored back.

That is strictly stronger than ir.contains("call i32 @js_gc_temp_root_push"), which only proved a call was emitted somewhere in the module.

Both lowerings must be expressible: shadow spells the slot alloca i64 + js_shadow_slot_bind, native spells it alloca ptr addrspace(1) with the bind implicit in the RS4GC retype. A helper that normalises the two, in tests/native_proof_support/ or a sibling, would serve both files — and would let these tests run unpinned, which is the right outcome: the contract is lowering-independent and should be asserted that way.

Acceptance

  • All 10 failing tests green, asserting the pooled form.
  • All 8 vacuous negatives re-pointed and shown to be non-vacuous — sabotage each by deleting the rooting at its emission site and confirm the corresponding positive test fails.
  • At least one test still covers the FFI fallback arm (js_gc_temp_root_push is not dead code — it is what runs when reserve_shadow_slot returns None), or that arm should be deleted per CLAUDE.md's kill-policy.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugConfirmed defect or regression

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions