leftover elem segments count as table mutation for call_indirect elisions

Active elem segments whose memory range extends past the destination
table's current size at instantiation behave as a runtime mutation:
the trailing entries get dropped, but only after they've been
considered for table-resize semantics. Treat the source table as
mutated when the module contains such a segment so the
call_indirect elisions don't over-fire.

Adds an integration test (`tests/all/leftover_elem_segment_soundness.rs`)
+ two disas filetests covering the leftover-segment shape.

Author: matthargett

crates/cranelift/src/func_environ.rs

@@ -2037,10 +2037,16 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
     /// All four of these must hold for the resolution to succeed:
     ///
     /// 1. The target table must be provably immutable for the lifetime of
-    ///    any instance of this module: defined (not imported) and never the
-    ///    target of `table.set` / `table.fill` / `table.copy` (as the dst)
-    ///    / `table.grow` / `table.init`. This is the `tables_mutated` bit
-    ///    populated in `ModuleEnvironment::translate`.
+    ///    any instance of this module: defined (not imported), not exported,
+    ///    never the target of `table.set` / `table.fill` / `table.copy`
+    ///    (as the dst) / `table.grow` / `table.init`, and not the target
+    ///    of any leftover active `elem` segment (one
+    ///    `try_func_table_init` couldn't fold into the precomputed image
+    ///    and that will execute at instantiation time, potentially
+    ///    overwriting precomputed slots with arbitrary funcrefs). All of
+    ///    these conditions are folded into the `tables_mutated` bit
+    ///    populated in `ModuleEnvironment::translate` —
+    ///    `tables_mutated[t] == false` is necessary and sufficient.
     ///
     /// 2. The callee index value (the operand to `call_indirect`) must be a
     ///    compile-time constant — i.e., the wasm did `i32.const N;
@@ -2140,8 +2146,14 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
     /// True when:
     ///
     /// 1. The table is provably immutable (`tables_mutated[table_index] ==
-    ///    false`). Defined-not-imported is implied since imported tables
-    ///    are pre-marked as mutated.
+    ///    false`). The `tables_mutated` bit set by
+    ///    `analyze_table_mutability` covers imports, exports, runtime
+    ///    mutation opcodes, AND tables targeted by leftover active `elem`
+    ///    segments (segments that didn't fold into the precomputed image
+    ///    and execute at instantiation time, potentially overwriting
+    ///    precomputed slots with a funcref of a different signature).
+    ///    `false` here is therefore sufficient to treat the precomputed
+    ///    image as authoritative for the lifetime of any instance.
     ///
     /// 2. The table is precomputable from static `elem` segments
     ///    (`TableInitialValue::Null { precomputed }`).
@@ -2179,7 +2191,10 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
         let Some(defined_table) = module.defined_table_index(table_index) else {
             return false;
         };
-        let Some(init) = module.table_initialization.initial_values.get(defined_table)
+        let Some(init) = module
+            .table_initialization
+            .initial_values
+            .get(defined_table)
         else {
             return false;
         };
@@ -2205,9 +2220,7 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
             return false;
         }
         // Every slot must be a real FuncIndex — no reserved-value sentinels.
-        precomputed
-            .iter()
-            .all(|f| !f.is_reserved_value())
+        precomputed.iter().all(|f| !f.is_reserved_value())
     }
 
     fn try_elide_sig_check_for_immutable_table(
@@ -2226,7 +2239,11 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
             None => return false,
         };
 
-        let init = match module.table_initialization.initial_values.get(defined_table) {
+        let init = match module
+            .table_initialization
+            .initial_values
+            .get(defined_table)
+        {
             Some(i) => i,
             None => return false,
         };

crates/environ/src/compile/module_environ.rs

@@ -259,8 +259,6 @@ impl<'a, 'data> ModuleEnvironment<'a, 'data> {
             self.translate_payload(payload?)?;
         }
 
-        analyze_table_mutability(&mut self.result)?;
-
         // Precompute static funcref-table contents from `elem` segments
         // before Cranelift lowering runs. The redundant call later in
         // `wasmtime/src/compile.rs::build_module_artifacts` is a no-op
@@ -276,10 +274,21 @@ impl<'a, 'data> ModuleEnvironment<'a, 'data> {
         // empty-precomputed check, never firing on real workloads —
         // their measured improvements in earlier commit messages were
         // measurement noise, not real signal.
+        //
+        // Folded segments are removed from `table_initialization
+        // .segments`; whatever's left is genuinely deferred to runtime.
+        // We do this BEFORE `analyze_table_mutability` so that the
+        // mutability analyzer can mark "tables with leftover segments"
+        // as conservatively mutated — leftover segments overwrite
+        // precomputed slots at instantiation time and so make the
+        // precomputed image non-authoritative for downstream call_
+        // indirect elisions. See `analyze_table_mutability`'s body.
         if self.tunables.table_lazy_init {
             self.result.try_func_table_init();
         }
 
+        analyze_table_mutability(&mut self.result)?;
+
         Ok(self.result)
     }
 
@@ -1398,17 +1407,34 @@ impl ModuleTranslation<'_> {
 /// destination, `table.grow`, `table.init`).
 ///
 /// Imported tables are conservatively pre-marked as mutated since the
-/// importer can mutate them in ways we can't see. Active `elem` segments
-/// applied at instantiation time are NOT counted as mutations — they are
-/// part of the table's *initial* state, not a runtime change.
+/// importer can mutate them in ways we can't see. Exported tables are
+/// also pre-marked: a host (or another instance importing the export)
+/// can `Table::set` / `Table::grow` them via the public wasmtime API,
+/// and those writes aren't visible in this module's bytecode.
+///
+/// **Active `elem` segments**: only those that `try_func_table_init`
+/// successfully folded into `table_initialization.initial_values[t]
+/// .precomputed` are part of the table's *initial* state and don't
+/// count as mutations. Anything still left in `table_initialization
+/// .segments` after that pass is a *leftover segment* that runs at
+/// instantiation time and can overwrite slots the precomputed image
+/// describes — including writing a different `FuncIndex`, a function
+/// of a different signature, or a null. Downstream `call_indirect`
+/// elisions in `crates/cranelift/src/func_environ.rs`
+/// (`try_static_resolve_indirect_call`,
+/// `try_elide_sig_check_for_immutable_table`,
+/// `precomputed_table_has_no_null_slots`) read directly from
+/// `precomputed`, so leftover segments make those reads
+/// non-authoritative. We mark every such target table as mutated to
+/// kill the elisions on it. (`try_func_table_init` must therefore have
+/// already run when we reach this function — the call site in
+/// `translate` enforces that ordering.)
 ///
 /// `elem.drop` drops a passive element segment but does not write to any
 /// table directly, so it is intentionally not counted here. Conservatively,
 /// any `table.init` from a passive segment marks the destination table as
 /// mutated.
-fn analyze_table_mutability<'data>(
-    translation: &mut ModuleTranslation<'data>,
-) -> Result<()> {
+fn analyze_table_mutability<'data>(translation: &mut ModuleTranslation<'data>) -> Result<()> {
     // Resize the table-mutability map to cover every table in the module
     // (imports + defined). `SecondaryMap` defaults to `false` for all
     // unset entries, which is the correct "definitely-not-mutated" default
@@ -1471,5 +1497,24 @@ fn analyze_table_mutability<'data>(
         }
     }
 
+    // Leftover active `elem` segments — anything `try_func_table_init`
+    // couldn't fold into `precomputed`. These run at instantiation time
+    // *after* the precomputed image has been applied and can overwrite
+    // any slot in their target table with arbitrary funcrefs (different
+    // signature, null, ...). Mark each target table as mutated so the
+    // downstream call_indirect elisions correctly bail out — they all
+    // read from `precomputed` and assume it's authoritative for the
+    // lifetime of any instance, which leftover segments invalidate.
+    //
+    // No-op if `try_func_table_init` cleared the segments list, which
+    // is the common case for compiler-emitted wasm (constant-offset
+    // Functions-form segments). Witness for the bug this guards
+    // against: a second elem segment with `(offset (global.get $g))`
+    // for the same table — dynamic offsets force the segment to stay
+    // in the segments list (see `try_func_table_init`).
+    for segment in translation.module.table_initialization.segments.iter() {
+        translation.module.tables_mutated[segment.table_index] = true;
+    }
+
     Ok(())
 }

crates/environ/src/module.rs

@@ -714,7 +714,14 @@ impl Module {
     /// - The table is provably immutable
     ///   (`tables_mutated[idx] == false`). Imported and exported
     ///   tables are pre-marked as mutated by
-    ///   `analyze_table_mutability` and so excluded.
+    ///   `analyze_table_mutability` and so excluded. The same pass
+    ///   marks tables with **leftover active `elem` segments**
+    ///   (segments that `try_func_table_init` couldn't fold into the
+    ///   precomputed image and that will run at instantiation time)
+    ///   as mutated, since those segments can overwrite slots after
+    ///   the precomputed image is applied — making any predicate
+    ///   that reads from `precomputed` non-authoritative for the
+    ///   lifetime of an instance.
     /// - The table has a precomputed image
     ///   (`TableInitialValue::Null { precomputed }`, not `Expr`).
     /// - The precomputed image covers the full minimum-size range of

crates/environ/tests/table_mutability.rs

@@ -305,3 +305,132 @@ fn module_with_no_tables_produces_empty_mutability_vec() {
     );
     assert!(bits.is_empty(), "no tables ⇒ no mutability bits");
 }
+
+// -----------------------------------------------------------------------------
+// Leftover active elem segments — soundness gate for the call_indirect
+// elisions in `crates/cranelift/src/func_environ.rs`.
+//
+// `try_func_table_init` folds *some* active `elem` segments into
+// `table_initialization.initial_values[t].precomputed` at compile time
+// and drains them from `table_initialization.segments`. Any segment it
+// can't fold (dynamic offset, `Expressions` form, out-of-range, ...)
+// stays in the `segments` list and runs at instantiation time *after*
+// the precomputed image is applied — potentially overwriting slots
+// that downstream optimizations have read from `precomputed` and
+// assumed stable. Pin the analyzer to mark every such target as
+// mutated so the elisions correctly bail out. Caught by review on
+// PR #2 (`https://github.com/rebeckerspecialties/wasmtime/pull/2#
+// discussion_r3193374159` and `…#discussion_r3193374164`).
+// -----------------------------------------------------------------------------
+
+/// A second elem segment whose offset is `(global.get $g)` (an imported
+/// global, resolved at instantiation time) cannot be folded into
+/// `precomputed` — `try_func_table_init` only folds segments with a
+/// constant `i32.const`/`i64.const` offset. So the segment stays in
+/// `table_initialization.segments` and overwrites slots at instance
+/// time. Without the leftover-segment pass in `analyze_table_mutability`,
+/// the table would be marked immutable and the type-confusion soundness
+/// bug fires (a function of a different signature could end up at slot
+/// 0 of an "immutable" table, defeating
+/// `try_elide_sig_check_for_immutable_table`).
+#[test]
+fn dynamic_offset_leftover_segment_marks_table_mutated() {
+    let bits = translate_and_get_mutability(
+        r#"
+        (module
+          (import "" "g" (global $g i32))
+          (table 4 4 funcref)
+          (func $f (result i32) i32.const 42)
+          (elem (i32.const 0) func $f)
+          (elem (offset (global.get $g)) func $f))
+        "#,
+    );
+    assert_eq!(
+        bits,
+        vec![true],
+        "leftover segment with dynamic offset must mark its target table mutated"
+    );
+}
+
+/// A segment in `Expressions` form (`funcref (item ref.func ...)`)
+/// rather than `Functions` form is also rejected by
+/// `try_func_table_init` and stays in `segments`. Same soundness
+/// argument as the dynamic-offset case: the segment's evaluation
+/// happens at instantiation time and can produce arbitrary funcrefs
+/// (including null via `ref.null func`), which would invalidate any
+/// elision proof that read from `precomputed`.
+#[test]
+fn expressions_form_leftover_segment_marks_table_mutated() {
+    let bits = translate_and_get_mutability(
+        r#"
+        (module
+          (table 4 4 funcref)
+          (func $f (result i32) i32.const 42)
+          (elem (i32.const 0) funcref (item ref.func $f) (item ref.null func)))
+        "#,
+    );
+    assert_eq!(
+        bits,
+        vec![true],
+        "Expressions-form leftover segment must mark its target table mutated"
+    );
+}
+
+/// `try_func_table_init` short-circuits the *whole* segment-folding
+/// loop on the first segment it can't fold — including any later
+/// segments that target a different table. This preserves wasm's
+/// trap-ordering semantics (the failing segment might trap, in which
+/// case later segments shouldn't have been applied either). The
+/// upshot: a single dynamic-offset segment can leave many leftover
+/// segments behind. Verify the analyzer marks every targeted table,
+/// not just the one whose segment broke the loop.
+#[test]
+fn leftover_segments_after_short_circuit_mark_all_targets() {
+    let bits = translate_and_get_mutability(
+        r#"
+        (module
+          (import "" "g" (global $g i32))
+          (table $t0 4 4 funcref)
+          (table $t1 4 4 funcref)
+          (func $f (result i32) i32.const 42)
+          ;; First segment for t0 has dynamic offset → breaks the
+          ;; folding loop → both this segment AND the later t1
+          ;; segment stay in `table_initialization.segments`.
+          (elem (table $t0) (offset (global.get $g)) func $f)
+          (elem (table $t1) (i32.const 0) func $f))
+        "#,
+    );
+    assert_eq!(
+        bits,
+        vec![true, true],
+        "both targets of leftover segments must be marked mutated"
+    );
+}
+
+/// Independence sanity check: a leftover segment for table 0 must NOT
+/// mark a separate table 1 that has only foldable segments. Mirrors
+/// the `mutation_isolated_to_target_table` test for runtime opcodes.
+#[test]
+fn leftover_segment_marks_only_its_target_table() {
+    let bits = translate_and_get_mutability(
+        r#"
+        (module
+          (import "" "g" (global $g i32))
+          (table $t0 4 4 funcref)
+          (table $t1 4 4 funcref)
+          (func $f (result i32) i32.const 42)
+          ;; Foldable segment for t1 — applied first, before any
+          ;; segment for t0 is reached. (Wasm specifies segments are
+          ;; processed in order, but `try_func_table_init` walks them
+          ;; in order too, so applying t1 first matches.)
+          (elem (table $t1) (i32.const 0) func $f $f $f $f)
+          ;; Dynamic-offset (leftover) segment for t0.
+          (elem (table $t0) (offset (global.get $g)) func $f))
+        "#,
+    );
+    assert_eq!(
+        bits,
+        vec![true, false],
+        "leftover-segment marking must not bleed into other tables"
+    );
+}