Skip to content

[1/2] call_indirect implementation - #1251

Merged
bitwalker merged 42 commits into
nextfrom
call-indirect-frontend
Aug 7, 2026
Merged

[1/2] call_indirect implementation#1251
bitwalker merged 42 commits into
nextfrom
call-indirect-frontend

Conversation

@greenhat

@greenhat greenhat commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Close #133

How it works

  • Each dispatched-through funcref table is lowered to a new module-level builtin.function_table op holding one builtin.function_table_entry per statically-initialized slot (from the table's active element segments).
  • The linker allocates a word-aligned region for each table — one word (16 bytes) per slot — in the page following the globals table, and moves the dynamic heap base past it.
  • The component init procedure fills each initialized slot with the callee's MAST root using procref;
  • A call site lowers through the new hir.exec_indirect op to a bounds check, slot address computation, and a dynexec;

Design notes

  • Tables are lowered lazily, on first use by a call_indirect: wit-bindgen and rustc routinely emit funcref tables (empty, or holding only artifacts like cabi_realloc) in modules that never make an indirect call, and lowering those would burden every compiled program with a useless table and its initialization code. With lazy lowering, programs without indirect calls compile byte-identically to before this change.
  • The Wasm-mandated runtime type-signature check is intentionally omitted (see the known-limitations appendix): safe Rust cannot produce a signature-mismatched call — reaching one requires code that is already undefined behavior — so the check would only add runtime cost to every indirect call. dynexec resolves the slot's word as a MAST root digest, so a corrupted or mismatched slot either matches a procedure already compiled into the program or fails execution; code injection is impossible.
  • Table-referenced functions are promoted from private to internal visibility, since the assembler rejects cross-module procref of private procedures.

Memory-layout hardening

Reviewing the safety of the new table region surfaced a pre-existing miscompile, fixed in the second commit: zero-initialized statics (.bss) occupy linear memory after the data segments without appearing in the wasm module, and the linker allowed only a single page of headroom for them — a larger .bss region silently overlapped the compiler-managed memory laid out above it (globals, function tables, heap).

The wasm module's declared minimum memory (previously parsed and discarded) is now recorded as a language-agnostic reserved_memory attribute on builtin.Module, expressed in bytes so no wasm units leak below the frontend, and the linker uses it as an additional floor for all compiler-managed regions. The one-page heuristic and the 17-page default reservation remain as fallbacks for HIR that carries no attribute (e.g. not produced by the Wasm frontend).

@greenhat greenhat changed the title call_indirect implementation [1/2] call_indirect implementation Jul 6, 2026
@greenhat
greenhat force-pushed the call-indirect-frontend branch from 5d2d0de to 6248a60 Compare July 7, 2026 13:50
@greenhat
greenhat requested a review from bitwalker July 7, 2026 14:12
@greenhat
greenhat force-pushed the call-indirect-frontend branch from 6248a60 to 5b776b7 Compare July 13, 2026 13:41

@bitwalker bitwalker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't quite ready to merge (beyond the obvious need to rebase on next, which will necessarily have conflicts) see my comments for more info.


The following limitations remain:

- The Wasm-mandated runtime type-signature check is intentionally omitted, with no plans to add

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree with the premise here - there are serious correctness issues with executing a call with a signature mismatch:

  • We can't lower the call correctly unless we know the signature/ABI
  • Not trapping will allow execution to proceed in potentially very dangerous ways - it would be a gaping huge security hole that anyone could leverage in an attack if the control flow path that leads to it is in any way caller-controlled. At the very least it would make diagnosing signature mismatch issues (e.g. due to a bug in codegen) difficult and annoying; at worst, it could allow a valid program to be used in malicious ways, so long as a caller can cause the valid program to call a caller-controlled function reference.

The question becomes: how do we enforce this? The simple answer is to use Wasm's approach of type tags - every function table reference has an associated type tag, and the runtime checks the type tag before executing the call (which is how it knows how to trap). The actual checking can't be done by dynexec/dyncall today, because there is no VM-native notion of function tables but we could emit a table that pairs every function digest with a type tag, and our lowering of dynexec/dyncall can check the tag associated with the function reference against the tag expected by the callsite before the actual invoke occurs. Yes, this incurs some overhead - but it isn't extreme, and it provides safety that is well worth that overhead IMO.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 3c36645

Comment on lines +390 to +397
fn resolve(&self) -> Option<SymbolRef> {
None
}

fn resolve_in_symbol_table(&self, _symbols: &dyn SymbolTable) -> Option<SymbolRef> {
None
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an issue here that allows the results of indirect calls to bypass unconstrained-advice analysis - because these callbacks always return None, the analysis today treats the call as external, then marks unresolved call results clean. So a private table callee can return raw advice without the finding produced for an equivalent direct call.

We should probably ensure that the analysis either joins the statically known table callees or conservatively taints unknown indirect results.

To be clear, the issue is really with the analysis, but since this PR is implementing indirect calls, we should probably also ensure that the analysis properly handles them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 55d3abe

Comment thread codegen/masm/src/linker.rs Outdated
/// Get the address of the first page boundary past all statically-allocated memory (global
/// variables and function tables), or the end of reserved memory if larger; this is where
/// the dynamic heap starts when the program is executed.
pub fn heap_base(&self) -> u32 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked layout still panics near 4GiB. This function remains infallible. A valid 65,535-page memory plus one 16-byte table links successfully, then panics while rounding 0xffff0010 to the next page. I'd compute and store the heap base within the fallible linker path and return LayoutOverflow.

It looks like the related global layout expect paths need the same fix.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a7e00d9

Comment thread codegen/masm/src/lower/component.rs Outdated
{
let mut callee = callee.borrow_mut();
if callee.visibility() == Visibility::Private {
callee.set_visibility(Visibility::Internal);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things here:

  1. There is no Internal visibility in MASM at this point - these procedures will be made public
  2. Making the procedures public in a non-public module is fine, because those now-public procedures won't be part of the public interface of the resulting package - but we have to explicitly make sure that the module hierarchy is structured that way. If we make the submodule public, then any public procedure of that submodule, or its public children, will become part of the public interface of the package - and that could very well be undesirable.

For Rust-compiled code, we know that we basically end up with the following structure:

  • A root module for the component namespace, which carries the init procedure, and the component-level exports
  • A submodule tree reflecting the structure of modules in that component. These submodule declarations are currently marked Public, though we should probably try and preserve the fact that the core Wasm module is private to the component. The fact that the initial visibility of the top-level submodule is Public is hardcoded, but should probably be Private instead (as public procedures of private submodules are visible to the parent which declares those submodules).

Anyway, I would suggest we try to resolve this such that the symbols are only visible within the package, not part of its public interface.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in fff0509

// the zero MAST root of a null slot, matching Wasm's uninitialized-element trap
let image = collect_table_image(table_index, defined_idx, module, diagnostics)?;

// The table symbol is internal to the compiler, so use a hygienic generated name; a Wasm

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not hygenic - in order to be hygenic the generated symbol here needs to be uniqued, such that it doesn't conflict with user-defined symbols. I was able to trigger an assert by defining a function named __indirect_function_table_0.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 0b91cf7

Comment thread codegen/masm/src/legalization.rs Outdated
.add_legal_op::<builtin::Function>()
.add_legal_op::<builtin::GlobalVariable>()
.add_legal_op::<builtin::Segment>()
.add_legal_op::<builtin::FunctionTable>()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will accept IR that later panics:

  • FunctionTable is marked unconditionally legal, even outside a module where the linker can never discover it
  • ExecIndirect lacks legality checks for its 15-felt/no-extension constraints.

Both of those should be dynamic legality checks or verifier-enforced.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c6e7a39

| CallableFunction::Intrinsic { function_ref, .. } => self
.module_builder
.append_function_table_entry(table, index, function_ref, span),
CallableFunction::Instruction { .. } => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're erasing operation-lowered intrinsic stubs before table construction, so we end up rejecting them here for lacking a procedure body. We need to somehow handle taking a reference against a stub that would be lowered to an operation, probably with some kind of synthetic wrapper just to make the function reference valid

@greenhat greenhat Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The erasure ordering is not the root cause. Building tables before pass 1 would not help — the entry's symbol use would dangle after the erase.

On reachability: this arm cannot be hit through the public SDK surface. The raw externs are private and never address-taken; every path from safe Rust to a funcref slot (fn pointer, closure, vtable) goes through a real Rust function (public wrapper), and rustc materializes those even when they are #[inline(always)]. I verified with the exact SDK shape (weak extern + #[inline(always)] wrapper + separately-compiled unreachable stub):

(elem (i32.const 1) func $wrapper_add $wrapper_mul) ;; wrappers, never the stub
(func $wrapper_add (param i32 i32) (result i32)
  local.get 0
  local.get 1
  call $intrinsics::felt::add)                      ;; body = call to the stub

Direct calls elsewhere still inline fully. The materialized wrapper is an ordinary Function-kind callable, lands in the table fine, and its body's call to the stub gets intrinsic-inlined during translation — so rustc already generates exactly the synthetic wrapper for us in every reachable case. The only way to hit the rejection is to hand-declare an extern against the stub symbols (intrinsics::felt::*) and take its address directly.

Given it is unreachable today, I would keep the diagnostic (rejection) as is.

greenhat added 17 commits August 4, 2026 10:53
Rust code using function pointers or trait objects previously hit a todo!() panic in the wasm frontend, since call_indirect had no lowering.

Lower each dispatched-through funcref table to a new builtin.function_table op holding one entry per statically-initialized slot. The linker allocates a word-aligned region for it (one word per slot) in the page following the globals table, moving the dynamic heap base past it; the component init procedure fills each slot with the callee's MAST root via procref + mem_storew_le; and call sites lower through the new hir.exec_indirect op to a table-index bounds check, slot address computation, and dynexec.

Tables are lowered lazily, on first use by a call_indirect: wit-bindgen and rustc routinely emit funcref tables (empty, or holding only artifacts like cabi_realloc) in modules that never perform an indirect call, so eager lowering would burden every compiled program with a useless table and its initialization code. Table-referenced functions are promoted to internal visibility so the init module's procref can resolve them, and the init procedure now attaches its collected invoked set, without which the assembler's linker treats procref-only callees as unreachable.

The wasm-mandated runtime type-signature check is deferred and documented in the known-limitations appendix; only the bounds check traps deterministically, and null slots fail inside dynexec. Covered by a differential test dispatching through a static array of function pointers across two indirect call sites, and a frontend expect-test snapshotting the lowered HIR.
Zero-initialized statics occupy linear memory after the data segments without appearing in the wasm module, and the linker's only allowance for them was a single page of headroom past the last segment. A larger .bss region therefore silently overlapped the compiler-managed memory laid out above it — global variables, function tables, and the dynamic heap — corrupting, for example, the MAST roots the function tables hold.

Record the declared minimum size of the module's linear memory (previously parsed and discarded) as a language-agnostic reserved_memory attribute on builtin.Module, expressed in bytes so no wasm units leak below the frontend, and use it in the linker as an additional floor for the global-table offset; function tables and the heap base are laid out above the globals, so the one attribute lifts all compiler-managed regions past everything the module's producer placed. The one-page heuristic and the 17-page default reservation remain as fallbacks for modules that carry no attribute, such as HIR not produced by the wasm frontend.

Covered by the static_bss differential case: a 136 KiB zero-initialized static filled across the window where a reservation-blind layout would place the function table, dispatching indirect calls through it afterwards.
The known-limitations entry listed the missing Wasm runtime type-signature check as if it were pending work. It is a deliberate design decision: safe Rust cannot produce a signature-mismatched indirect call — reaching one requires code that is already undefined behavior — so the check would only add runtime cost to every indirect call.

Also note the containment property that makes this acceptable: dynexec resolves the table slot as a MAST root digest, so a corrupted or mismatched slot either matches a procedure already compiled into the program or fails execution.
Table entries were collected as an append-only list with ref.null element entries skipped, which is only correct for slots that were already null: a whole-table (ref.func ..) default, or an earlier segment write followed by a later ref.null, left the earlier function in place, so an in-bounds call_indirect could dispatch to a stale function instead of trapping on a null slot. The TableInitialValue::Null { precomputed } arm had the inverse latent bug, passing reserved (null) indices through to a panicking map lookup.

Compute a final per-slot image instead: apply the initial value and the active element segments in order, with later writes - including explicit ref.null entries - replacing earlier ones, and emit one table entry per final non-null slot. This also collapses duplicate slot writes into a single startup store and removes the write-ordering subtlety the init code previously relied on.

Also reject pathological tables (more than 2^20 slots) up front instead of exhausting memory materializing per-slot IR, replace the remaining todo!()/bare-Report paths in the same file with proper diagnostics (core function imports, inlined-intrinsic table elements, now named by function), and enable the reference-types feature so expression-style element segments - the encoding ref.null clears arrive in - validate; every construct it admits is either handled or rejected with a clean error. Covered by a module translation test where a ref.null entry clears a previously initialized slot, and one rejecting an oversized table.
The reserved_memory module attribute is the input that decides where the linker places compiler-managed memory, but builtin.module's custom printer and parser handled only visibility, name, and body: printed HIR silently dropped the attribute, so re-ingesting a dump (HIR files are a first-class compiler input) reinstated the heuristic layout floor - the exact silent-corruption bug the attribute exists to prevent - and no dump ever showed the reservation. Similarly, hir.exec_indirect printed a custom form with no matching parser, making any IR containing an indirect call one-way.

Print and parse an optional attributes dictionary on builtin.module (attribute values round-trip fully typed, e.g. #builtin.u64<..>), and add an OpParser for hir.exec_indirect mirroring hir.exec (table symbol, bracketed u32 index operand, argument list, signature), along with the attribute-dictionary tail its printer was missing. Covered by a print/parse round-trip test asserting the typed attribute and table survive, and a lit test round-tripping the full new textual surface.

Also hardens the new table IR against non-frontend producers: ModuleBuilder::append_function_table_entry now rejects out-of-bounds slot indices (instead of deferring to a codegen panic), creates the entries block on demand for tables not built by define_function_table, and documents that later duplicate-slot entries win; the FunctionTable size attribute is renamed to num_slots to make its unit unambiguous; the stale commented-out CallIndirect skeleton - which embodied the obsolete design this branch replaced - is dropped; and builtin.module's rustdoc now lists FunctionTable among the entities a module can contain.
Several failure paths in the new indirect-call machinery aborted the compiler instead of producing a diagnostic, and two were reachable from validator-accepted Wasm: a module declaring the wasm32 maximum memory (65536 pages) panicked converting the reserved-memory floor to u32, and a sufficiently large declared reservation survived the cast only to silently corrupt every global variable offset through the wrapping i32 casts in update_global_table_offset. The linker now returns a LayoutOverflow error for reservations, data-segment ends, and function tables that leave no room in the 32-bit address space, and the offset-rebasing arithmetic is done in 64 bits with checked conversions. Function-table init emission likewise returns diagnostics instead of panicking on malformed entries or unresolvable callees.

The private-to-internal visibility promotion of table-referenced callees moves from the wasm frontend into the init emission: the promotion exists only because codegen places init in the root component module and takes MAST roots via cross-module procref, so the constraint is now enforced where it originates, and other producers of function-table IR get it for free.

Also folds the slot geometry into FunctionTableLayout (SLOT_SIZE_BYTES/SLOT_SIZE_ELEMENTS constants and an element_addr_of helper that centralizes the byte-to-element conversion and word-alignment check), resolves the exec_indirect lowering's table through nearest_symbol_table like hir.exec instead of assuming a builtin.module ancestor, renames the bounds-check trap message to the frontend-agnostic "indirect call: function table index out of bounds", and replaces raw FunctionTableRef::from_raw uses with the as_function_table_ref helper.
The bounds check is the most safety-relevant code the indirect-call lowering emits, yet no test executed it: the differential case masks its index, and a wrong comparison direction or off-by-one would have passed the whole suite. Out-of-bounds and null-slot dispatch also cannot be differential cases, because both are undefined behavior when the same program runs natively.

Add an emitter unit test pinning the exact instruction sequence and stack effect of exec_indirect (bounds check, in-place index-to-address rewrite, frame-traced dynexec), and an execution test that transmutes an input into a function pointer - at the Wasm level a function pointer is its table index - and asserts that an in-bounds slot dispatches successfully, an out-of-bounds index traps with the documented bounds-check message, and the null slot fails on its zero MAST root.
The appendix docs predated the memory-based dynexec and the call_indirect implementation on this branch: the 'Dynamic procedure invocation' section still claimed dynexec support was unimplemented and described the obsolete hash-on-stack design with per-callee stubs, and calling-conventions.md derived a 12-element argument limit from that design, contradicting the actual 15-plus-address limit and wrongly claiming indirect-call arguments spill.

Rewrite the dynamic-invocation section around the implemented mechanism (same-context dynexec dispatch through a memory-resident MAST root, no stubs needed; dyncall still pending on cross-context invocation), fix the argument-limit and spill claims, and align the smaller details: the function-call-indirection section now states that table lowering is lazy and lists the empty-table compile-time rejection, the stale milestone reference is dropped, the wasm frontend README gains the table support/limitation matrix, and the println_expr comment no longer cites call_indirect as the reason for avoiding core::fmt (it is supported now; the remaining reason is VM cost).
…names

A call_indirect through a table with no statically-initialized entries was rejected at compile time, although the runtime representation already handles null slots (a zero word that dynexec fails on), so valid trap-at-runtime Wasm failed to compile. Lower such tables with their declared slot count and no entries instead; every dispatch through them now fails at runtime like any other null slot, matching Wasm's uninitialized-element trap. Lazy lowering still ignores tables that no call_indirect dispatches through.

Also stop reusing a table's Wasm export name as its HIR symbol: export names are arbitrary strings independent of the module's internal namespace, so a colliding name (e.g. a table exported as 'add' alongside a function @add) rejected valid modules. Every lowered table now gets the generated __indirect_function_table_{index} name; the symbol is compiler-internal, so nothing user-visible changes.
The layout-overflow hardening left three spots that could still wrap silently in release builds for layouts approaching the 32-bit address limit: global variable placement (alignment and size accumulation in GlobalVariableLayout::insert) and the page rounding in next_page_boundary and heap_base. All three now use checked arithmetic that fails loudly with a descriptive message, consistent with the offset-rebasing fix.

Also make builtin.function_table_entry dynamically legal only inside a builtin.function_table, so a misplaced entry is reported as a legalization diagnostic instead of reaching the MASM module builder's panic, and clean up a duplicated doc summary plus two inline TODOs that are really design notes.
The Wasm-mandated call_indirect type check was deliberately omitted: a signature mismatch requires code whose behavior is already undefined, and the check costs cycles on every dispatch. Review discussion on #1251 overturned that trade-off: on every other Wasm target this class of UB is bounded to a deterministic trap, while here a type-confused dispatch silently reinterprets the caller's operand stack and still proves as a valid execution, so the trap is worth its cost.

Each table slot now pairs the callee's MAST root with a tag identifying its signature - structurally interned, so equal signatures share a tag, with 0 reserved for null slots. The component init procedure stores the tag next to the digest, and dispatch asserts the slot's tag against the tag the call site expects before dynexec, five extra instructions per indirect call. The check doubles as the Wasm uninitialized-element trap, replacing the bare VM failure on the zero MAST root of a null slot with a deterministic, source-spanned assertion.
The unconstrained-advice analysis treated hir.exec_indirect as an external call, because the op resolves to no single callee, and external-call results are trusted clean - so a table callee returning or storing raw advice escaped the findings an equivalent direct call would produce.

The set of functions an indirect call can reach is statically known, though: exactly the table entries whose signature tag matches the call site, since dispatch to any other slot traps on the runtime signature check before the callee runs. A new CallOpInterface::possible_callees hook (defaulting to the single resolved callee) exposes that set, hir.exec_indirect enumerates it (last entry per slot; unknown if anything fails to resolve), and the dead-code analysis registers the call site with every analyzable target. The existing predecessor machinery then carries arguments into the callees and joins their returns and memory effects back into the call site, for every interprocedural analysis on the solver; the external-call classification of both forward strategies moves into one shared predicate.

Calls that still cannot be enumerated are now modeled conservatively instead of trusted clean: their results are treated as unconstrained advice, and, because a same-context callee can write caller memory, so is every memory load after the call.
A valid 65,535-page module memory plus one function table linked successfully, then panicked in LinkInfo::heap_base while rounding the table end to the next page: link() checked its own layout arithmetic, but the heap base was computed lazily in an infallible accessor. The same expect-based rounding and rebasing survived in the global-variable layout, reachable from link() with valid input, since a large declared memory reservation can push globals toward the top of the address space.

The heap base is now computed and validated as a linker step and stored in LinkInfo, so static memory that leaves no representable heap base is a LayoutOverflow error instead of a panic; the global-layout insertion, rebasing, and page-rounding paths report LayoutOverflow the same way. The raw page bookkeeping on LinkInfo and the table end-offset accessor only served the removed computation, and are dropped.
The v0.25 migration replaced the frame trace instructions with event emission and reworked the test executor to take packages directly; the dynexec framing in exec_indirect, its emitter unit test, and the indirect-call trap test now use the migrated forms.
The assembler derives a library package's public surface from the modules reachable from the root through public submodule declarations, but codegen declared every submodule public - a hardcoded placeholder left by the v0.25 migration, with the intended visibility mapping commented out beside it. Every masm-public procedure of every core module therefore leaked into the package's interface, including procedures that are only public so cross-module procref and exec can reach them, such as the function table callees promoted for indirect-call slot initialization.

Enable the mapping: a component's core modules are private in HIR, so they become private submodules, whose public procedures remain resolvable within the package (a private submodule is visible to its parent and siblings, which covers init's procref of table callees and interface calls into core functions) without being part of its interface. The synthetic wrapper the compiler creates around a bare core module is not a real component boundary, though: the wrapped module is the artifact's own interface, and the generated executable main module lives outside the wrapper's module tree, so its submodules stay public, as does everything lowered without a component id.

With internals no longer export roots, the assembler's export-rooted retention strips unreachable code from real component packages: the basic-wallet account drops 44% (14277 to 7982 stripped MAST bytes), the p2id note 36%, and the swapp note 13%. A new unit test pins the surface rule - a public procedure of a private module assembles and is callable from the root, but is not exported - and the one fixture whose module is its component's whole interface now declares it public.
Every symbol name in a Wasm module is a producer-controlled string, so the fixed generated name for a lowered function table was not hygienic: a user function named __indirect_function_table_0 collided with it and tripped an assert when the table symbol was defined.

Probe the module symbol table and bump a counter until the generated name is free. The collision set is complete at that point, because tables are built lazily during body translation and all functions and global variables are declared before any body is translated; the choice is deterministic, and nothing depends on the spelling, since entries and call sites hold symbol references and the lazy-table map is keyed by table index. Covered by a frontend test whose colliding user function is itself an entry of the table that had to avoid its name.
Legalization accepted operations whose codegen contracts were only enforced by panics downstream: a function table anywhere but a module body is invisible to the linker's layout scan, so a dispatch through it panicked at lowering on the missing layout entry, and hir.exec_indirect reached the emitter with constraints only the wasm frontend checked - arguments must be extension-free (the transient slot address holds the stack top while they are consumed), and the arguments plus the table index must fit the addressable operand stack window.

Make the function table dynamically legal only inside a module, mirroring the existing entry-scope rule, and give hir.exec_indirect a legality check for both constraints, so any producer's IR fails legalization with a reasoned diagnostic instead of panicking in codegen. The stack window size is sourced from the VM's canonical MIN_STACK_DEPTH in both the check and the wasm frontend's translation-time diagnostic, rather than being restated as a literal.
@greenhat
greenhat force-pushed the call-indirect-frontend branch from 5b776b7 to c6e7a39 Compare August 4, 2026 13:10
@greenhat

greenhat commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@bitwalker Thank you! I addressed all the comments. Please do another round.

@greenhat
greenhat requested a review from bitwalker August 4, 2026 13:14
`parse_anchored_source` parses into a synthetic anchor `World` and detaches
the parsed world from it, because a world nested in a world is invalid IR.
Verification ran inside `finalize`, i.e. while the parsed world was still
nested — and symbol resolution is rooted at the root operation, so during
that window an absolute path like `::@m::@tbl` names a grandchild of the
root rather than a child. Any verifier that resolves one saw valid IR as
invalid; `hir.exec_indirect`'s table verifier is the first that does, which
made `midenc compile --emit hir` output impossible to feed back to `midenc`
for any program containing a `call_indirect`.

Detach first, then verify the root that is about to be returned. `finalize`
still runs attached — it resolves deferred locations and records symbol uses
by walking down from the anchor — so it grows a `finalize_without_verifying`
variant rather than moving wholesale. Failures still surface as a
`ParserError::Report` carrying the source, exactly as before.
…ts traversal

The verifier's "last entry per slot wins" rule and its rejection of the
reserved tag 0 were both untested: every fixture had a single entry, so
replacing the `BTreeMap` with an accumulating `Vec` — or deleting the tag-0
branch outright — kept the suite green. Add a fixture per direction of the
overwrite rule (a dead mismatched entry must be ignored; an entry that
overwrote a matching one must be checked) and one for tag 0.

The verifier and `possible_callees` were also computing the dispatchable set
twice, by the same traversal over the same field. That duplication is what
the security guarantee rests on: if one grew a filter the other lacked, the
verifier would silently stop covering entries codegen still dispatches to.
Extract `live_table_slots`, used by both, with `possible_callees` dropping
the fields it does not need.

Finally, the malformed-body diagnostic named neither the op nor the source
of the complaint — it read as if the table were at fault, when the failing
verifier is a call site. Name both.
The table's rule is last-entry-per-slot-wins, and until now three places
implemented it independently: the `hir.exec_indirect` verifier, the
`possible_callees` analysis, and code generation — which never implemented
it at all, and emitted a store for every entry in the body.

That was harmless while all the stores went into one `init` body in
textual order, but grouping them by the callee's defining module broke
it: fragments run in path order, so a dead entry naming a callee in a
later-sorting module now lands in the slot *after* the live one. Its
signature was never compared against any call site - the verifier only
looks at the entry that wins the slot - so the runtime tag check passes
and `dynexec` transfers control with a mismatched stack contract.

Promote the rule to `builtin::FunctionTable::live_entries` and route all
three consumers through it, so there is one definition to disagree with.
The entries come back unresolved: the verifier resolves only the ones
whose tag matches its call site, which on a table with `1 << 20` slots is
not all of them, while codegen resolves every one. Resolving in the
shared method made each pay the other's cost, and made the verifier, 
which runs per pass/per call site, quadratic in the table.
A table entry may name a function in a declaration-only world sibling.
`classify_siblings` skips such a module and never lowers it, but the
entry still resolves and the function still has a signature, so both the
`hir.exec_indirect` verifier and legalization accept the IR - and the
first thing the producer saw was a panic from the slot-filling code
looking for a module that is not there. Report it as invalid input,
naming the callee and its module, like every other invalid-IR path in
that function.

This also attaches the invocations recorded for `init` to the `init`
procedure. Nothing ever did, so the fragment roots registered just above
it, and the global-variable initializers before them, were written into
a set that was then dropped. The linker resolves an `exec` from the
instruction too, which is why the omission never showed, but the set is
what it builds its call graph from and `init` was the one procedure in
the component declaring none of its callees.

Also record why a private nested module whose parent defines no table
callee is a fragment root `init` cannot reach, next to where roots are
computed - it fails loudly at assembly time and nothing produces it, but
it costs a paragraph to not have to rediscover that.
…r marker

Three loose ends the branch left, none of which changes behavior.

Verification moved out of `OperationParser::finalize` and into
`parse_anchored_source`, and nothing verified that it still runs: the tests
that exercise invalid IR call `recursively_verify` themselves, and the
one deliberately malformed fixture is parsed with verification off, so
deleting the call would have kept them all green. Feed a source that is
well-formed as text and ill-formed as IR through the ordinary `parse_any`
path, and assert both that it fails and that the same source parses with
verification turned off - the second half is what makes the first about
verification rather than about the text.

Initializing a function table trades a private callee off the package's
export manifest for a compiler-generated `__init_function_table` onto it.
The test asserted the two memberships it was named for and nothing else;
assert the whole sorted set, so the trade is written down and a third
symbol arriving on the public surface is a failure rather than a
discovery.

Give `builtin::Component` a `mark_synthetic_wrapper`, beside the
`is_synthetic_wrapper` that reads it, and route the frontend and the two
test harnesses standing in for it through that instead of each spelling
out the same attribute plumbing.
Pre-existing on the branch, and the one thing standing between
`cargo clippy --workspace --all-targets` and a clean run.
An entry naming a function in a declaration-only sibling would panic, 
which everything upstream accepts and `classify_siblings` never lowers. 
Verified against the pre-fix code, where this fixture panics with 
"a table callee's module must have been lowered".

@bitwalker bitwalker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good now. I've addressed a handful of related issues I found directly so we can get this landed, since it holds up other work of yours that is waiting on it - but largely the focus of those changes was around bolstering verification, and closing various edge cases that was better addressed here than separately.

@bitwalker
bitwalker merged commit 1028af4 into next Aug 7, 2026
19 of 22 checks passed
@bitwalker
bitwalker deleted the call-indirect-frontend branch August 7, 2026 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wasm tables and call_indirect op translation

2 participants