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

Activity

1. proggeramlug commented on Aug 9, 2026

   

   proggeramlug

   on Aug 9, 2026

   ContributorAuthor

   More actions

   The new.rs half is fixed in #7699. builtin.rs is not, and I am leaving this issue open for it — see the inventory at the bottom.

   What landed

   The three branches this issue names all early-return before the main class loop ever adopts into the enclosing scope, so lower_new_impl's open_rooted_group was open around them and empty — lexical containment is not protection when nothing is adopted, and RootedGroup::release truncates from the group's own lowest slot, so an empty group emits nothing at all.

   • new Readline(…) — output sat in a bare SSA register across options' lowering and across every trailing argument's;
   • new <importedFn>(…) — func_double across every argument, and argument i across the arguments after it;
   • new Function(…) with a dynamic body — the same loop.

   All three now adopt into the group as each operand is produced and re-read at the call. Interleaved, not appended: this issue's own trap ("rooting the finished list publishes an already-dangling argument 0 to the scanner, which turns a silent DIFF into a SIGSEGV") is restated on the shared adopt_constructor_args helper so the next edit reads it.

   Two details worth recording:

   • the helper keeps lower_constructor_arg rather than RootedGroup::lower, because it clears ctx.discard_expr_value for the operand — fix(codegen): a typed-array element store used as an expression evaluates to 0 — discard_expr_value leaks into operand position #7590: that flag means "this STATEMENT's value is discarded", is not cleared on recursion, and lowering an operand under it can evaluate a typed-array store to 0;
   • the undefined fillers for absent readline arguments stay literals, so new Readline() still costs no slot.

   lower_js_args_array is confirmed to be no rescue and is untouched: alloca_entry_array emits a bare LLVM alloca with no js_shadow_slot_bind and no shadow-slot reservation (the opt-in is expr/scalar_slot_root.rs's root_entry_alloca, which nothing on these paths calls). It copies whatever bits it is handed, stale or not, so the repair has to happen before it runs — which is where it now is.

   Still open: lower_call/builtin.rs

   lower_builtin_new(ctx, class_name, args) takes no rooting context at all, and is called from lower_new_impl_inner before that function's group is populated — so these are structurally unreachable from a new.rs-level fix. Each needs its own rooting::with_operands_rooted over the argument list.

   The arms that lower 2+ argument expressions sequentially and then use the earlier register(s):

   constructor	lines	callee
   Uint8Array (≥2 args)	141–165	js_uint8array_view
   typed-array views (Int8Array…Float16Array, ≥2 args)	171–195	js_typed_array_view
   DataView	200–228	js_data_view_new
   RegExp	229–252	js_regexp_construct
   Event	374–396	js_event_new
   CustomEvent	397–419	js_custom_event_new
   DOMException	420–441	js_dom_exception_new
   Console (2-arg form)	442–464	js_console_new2
   DatabaseSync	612–630	js_node_sqlite_database_sync_new
   StatementSync	631–649	js_node_sqlite_statement_sync_new
   Session	650–668	js_node_sqlite_session_new
   CronJob	739–768	js_cron_job_new
   AsyncResource	803–821	js_async_resource_new
   SuppressedError	848–870	js_suppressed_error_new
   Response (init fields)	914–1029	js_response_new
   Blob	1038–1064	js_blob_new
   File	1066–1106	js_file_new
   Headers (per-key loop, accumulator + pair)	1108–1137	js_headers_set
   Request (init fields)	1147–1276	js_request_new / _from_init
   ReadableStream	1282–1337	js_readable_stream_new_with_strategy_and_source_type
   WritableStream	1339–1412	js_writable_stream_new_with_sink_type
   TransformStream	1414–1477	js_transform_stream_new
   TextDecoderStream	1489–1509	js_stream_web_text_decoder_stream_new

   Ruled out on inspection, so they are not missing from the list: EventEmitter, EventEmitterAsyncResource, BlockList, SocketAddress, X509Certificate, PerformanceObserver, StringDecoder, Decimal, CountQueuingStrategy, ByteLengthQueuingStrategy, CompressionStream, ReadableStreamBYOBReader, WeakMap, WeakSet — each lowers its extra arguments with let _ = lower_expr(...), so nothing is retained across the loop to go stale. Same for new.rs's own Crypto | CryptoKey | SubtleCrypto arm and the empty-placeholder fallback: they emit plenty of IR, but no value survives to be consumed later, which is a different thing from a zero-emission false positive.

   CronJob is the one that needs care rather than a mechanical wrap: its first operand is a raw i64 from get_raw_string_ptr (which is lower_expr plus a real js_get_string_pointer_unified call), so it is #7280 taxonomy (a) and wants the derivation moved below the later operands, not a slot.
2. proggeramlug

   added 4 commits that reference this issue on Aug 9, 2026

   fix(codegen): root new's non-class ctor args and the computed-key win…

   ...

   f9b4ce7

   fix(codegen): the lowering-window cluster — push order, member update…

   ...

   9965eb3

   fix(gc): root builtin.rs's constructor arguments across each other's …

   ...

   d72e354

   fix(gc): root builtin.rs's constructor arguments across each other's …

   ...

   72ef47a
3. proggeramlug

   closed this as completedin #7719on Aug 9, 2026
4. proggeramlug

   added 2 commits that reference this issue on Aug 9, 2026

   gc: fix the phi-edge checker false positives and the static-dispatch …

   ...

   edd8f45

   gc: fix root-dominance phi false positives + 3 of 5 real hits, lower …

   ...

   c156f8a