Skip to content

gc: constructor arguments on lower_new's non-class branches (Readline / imported ctor / new Function) and builtin.rs are still not precise roots #6986

Description

@proggeramlug

Found while fixing #6969 in #6983, and deliberately left out of that PR's scope.

#6983 roots the constructor arguments built by lower_new_impl_inner's main
class path
(crates/perry-codegen/src/lower_call/new.rs, the lowered_args
loop reached after the ctx.classes lookup). Three earlier branches of the
same function build multi-argument lists the same way and are still unrooted, as
are the multi-argument built-in constructors in lower_call/builtin.rs.

Sites

crates/perry-codegen/src/lower_call/new.rs:

  • new Readline(...) (node:readline/promises) — lowers output, then
    options, then for extra in args.iter().skip(2) { lower_expr(...) }, and
    passes output/options live into js_readline_promises_readline_new. A
    later argument's lowering can collect while the first two sit in SSA
    registers.
  • new <importedFn>(...)for a in args { lowered_args.push(lower_constructor_arg(ctx, a)?) }
    then lower_js_args_array, which is a plain alloca pack with no rooting
    (expr/helpers.rs), passed to js_new_function_construct.
  • new Function(...) with a dynamic body — the identical loop, into
    js_function_ctor_from_strings.

crates/perry-codegen/src/lower_call/builtin.rs (untouched by #6983): several
built-in constructors lower args[0] then args[1] sequentially via plain
lower_expr. lower_builtin_new is called from inside lower_new_impl_inner,
so these are on the same path. I did not audit each one; the shape is the same.

Why these are cheap to fix

#6983 already wraps the whole function in an expression-scope temp-root barrier
(temp_root_scope_begin / _end in lower_new_impl), and it is a stack cut
so these branches are already inside a scope that releases correctly on every
return path. What is missing is only the per-argument push + re-read. The
primitives are temp_root::operand_needs_root, temp_root_push_double,
temp_root_get_double, and operand_is_reloadable for operands that read a
registered root.

One trap, learned the hard way in #6983: the push must be interleaved with
the lowering loop, not appended after it. Rooting the finished list publishes an
already-dangling argument 0 to the scanner, which turns a silent DIFF into a
SIGSEGV — strictly worse than not rooting at all.

Detection

Same as the rest of the family: invisible with the conservative native-stack
scan on. Reproduces under PERRY_CONSERVATIVE_STACK_SCAN=off PERRY_GC_HEAP_LIMIT=8 with a shape like
new Function(fresh(0), "return " + churn(N)), and under the evacuating arm
from #6977 (PERRY_GC_INCREMENTAL=0 + the above).

Related: #6969, #6983, #6951, #6972, #6975, #6981, #6968.

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