Skip to content

codegen: a constructor bail-out lowers the body and then discards its globals and declares (method.rs:928) #9890

Description

@proggeramlug

crates/perry-codegen/src/codegen/method.rs:917-931 — the "parent class has no
callable constructor symbol" bail-out inside constructor lowering — lowers the
method body into the module and then discards every global and declaration that
body produced
:

} else {
    // No callable ctor symbol — bail.
    stmt::lower_stmts(&mut ctx, method_body).with_context(|| {
        format!("lowering body of method '{}::{}'", class.name, method.name)
    })?;
    // Fall through to the default ret at end.
    if !ctx.block().is_terminated() { … ctx.block().ret(DOUBLE, &undef); }
    let _ = std::mem::take(&mut ctx.ic_globals);
    let _ = std::mem::take(&mut ctx.typed_parse_rodata);
    let _ = std::mem::take(&mut ctx.pending_declares);
    return Ok(());
}

The three std::mem::takes discard the values; the normal exit at
method.rs:1306-1343 does the opposite with the same three — it drains them
into llmod as inline_cache_global_definitions, raw globals and
declare_functions.

The lowered body is not discarded with them. ctx writes its blocks into
the function llmod already holds, so on this path the module keeps IR that
references:

  • every @perry_ic_N inline-cache slot the body's property accesses allocated
    (expr::inline_cache_global_name / inline_cache_global_definition),
  • every raw global pushed to typed_parse_rodata — today that is
    expr/strings.rs, expr/array_literal.rs, expr/object_literal.rs,
    expr/typed_feedback.rs, expr/instance_misc1.rs, expr/v8_interop.rs and
    concat_site_cache.rs,
  • every runtime symbol queued in pending_declares,

none of which is ever defined or declared.

How it fails

Loudly, at the in-process LLVM parse, with use of undefined value '@…' — the
same shape as #9859, where five js_segments_view_* calls were emitted with no
declare and twelve passing HIR-level unit tests could not see it. It is not a
miscompile and cannot produce a wrong answer at runtime.

There is a second, quieter consequence: this path returns without running
llmod.ic_counter = ic_end, so the site ids this body consumed are handed back
out to the next function in the module and two sites can be issued the same
@perry_ic_N / @perry_concat_site_N name → an LLVM redefinition error.
Also loud.

Why it has not fired

Presumably every constructor that reaches this arm today has a body trivial
enough to allocate no IC site, no rodata global and no pending declare. That is
a property of the corpus, not of the code, and nothing enforces it.

How I found it

Not from a failure — from reading the drain sites before adding a new one. The
regex literal-site key (perf/regex-literal-site-key) emits one
@perry_regexp_site_<prefix>__<n> per regex literal through
typed_parse_rodata, so a regex literal inside such a constructor would be the
next victim. The new lowering carries a comment pointing here so the next person
to see use of undefined value knows where to look.

Suggested fix

Drain instead of discard: hoist the ic_globals / typed_parse_rodata /
pending_declares emission (and llmod.ic_counter = ic_end) into a single exit
helper that every return from this function goes through. A regression test
wants a class whose parent has no callable constructor symbol and whose own
constructor body contains something that allocates a global — an array literal
or a property read is enough — and asserts the emitted module defines every
global its IR names.

Filed by the regex-subsystem lane of the cc-perf campaign; not blocking that
work, since the failure mode is a compile error rather than a wrong answer.

Activity

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