Fix MVP element-segment encoding for table64 active segments

When walrus emits an active element segment whose table is table 0,
it tells `wasm-encoder` to use the MVP encoding (variant `0x00`)
which carries an implicit table index of 0 and requires the offset
constant to be `i32`. If table 0 is a `table64` table the offset
must be `i64`, so the MVP encoding is not applicable; the resulting
binary is rejected by compliant engines (V8 reports
`invalid table elements limits flags` because it tries to decode
the i64 offset bytes as an i32 const expression and then reads the
following byte as element-section limits).

Use `wasm-encoder`'s explicit-table-index form (variant `0x02`)
unconditionally for `table64` tables. Non-`table64` table-0 active
segments continue to use the backwards-compatible MVP encoding.

Test: `crates/tests/tests/table64_active_segment.rs` asserts at the
byte level that the emitted segment kind tag is not `0x00` for a
table64 active segment. The test also round-trips through walrus.

Author: guybedford

crates/tests/Cargo.toml

@@ -16,6 +16,7 @@ serde_json = { version = "1.0.40", features = ['preserve_order'] }
 tempfile = "3.1.0"
 walrus = { path = "../.." }
 walrus-tests-utils = { path = "../tests-utils" }
+wasmparser = "0.245.1"
 wasmprinter = "0.245"
 wat = "1.0.85"
 

crates/tests/tests/table64_active_segment.rs

@@ -0,0 +1,119 @@
+//! Repro for an emit bug with active element segments on `table64`
+//! tables.
+//!
+//! When an active element segment targets table 0, walrus tells
+//! `wasm-encoder` to use the MVP encoding (variant 0) which carries an
+//! implicit table index of 0 and requires the offset constant to be
+//! `i32`. If table 0 is a `table64` table, the offset must be `i64`,
+//! so the MVP encoding is not applicable; walrus must emit the
+//! explicit-table-index form (variant 2) instead.
+//!
+//! Currently walrus picks the MVP form unconditionally for table 0,
+//! producing a wasm binary that real engines like V8 reject with
+//! errors like `invalid table elements limits flags` because they
+//! decode the (i64) offset as if it were an i32 const expression and
+//! then read the next byte as the "elements limits flags". `wasmparser`
+//! happens to accept the malformed binary, so this test asserts at the
+//! byte level on the segment-kind tag instead of relying on a parser.
+//!
+//! Expected: emitted wasm uses the explicit-table-index element form
+//! (variant 2) for table64 active segments.
+
+use walrus::{
+    ConstExpr, ElementItems, ElementKind, FunctionBuilder, Module, ModuleConfig, RefType,
+};
+
+#[test]
+fn table64_active_segment_round_trips() {
+    let mut config = ModuleConfig::new();
+    config.generate_producers_section(false);
+    let mut module = Module::with_config(config.clone());
+
+    // A single function to put in the table.
+    let func_id =
+        FunctionBuilder::new(&mut module.types, &[], &[]).finish(vec![], &mut module.funcs);
+    module.exports.add("f", func_id);
+
+    // table64 funcref table starting at size 1.
+    let table_id = module
+        .tables
+        .add_local(/* table64 */ true, 1, Some(1), RefType::FUNCREF);
+
+    // Active element segment at offset 0, table index 0.
+    let elem_id = module.elements.add(
+        ElementKind::Active {
+            table: table_id,
+            offset: ConstExpr::Value(walrus::ir::Value::I64(0)),
+        },
+        ElementItems::Functions(vec![func_id]),
+    );
+    module
+        .tables
+        .get_mut(table_id)
+        .elem_segments
+        .insert(elem_id);
+
+    let wasm = module.emit_wasm();
+
+    // Locate the element section and inspect its first segment's kind
+    // byte. The MVP encoding is 0x00 (which requires an i32 offset);
+    // any of 0x02/0x06 are the explicit-table-index forms that work
+    // with table64. wasm-encoder would also use 0x04/0x05/0x07 for
+    // expression-form segments without a function vector, but we use
+    // the function-vector form here.
+    let mut p = wasmparser::Parser::new(0);
+    let mut cur = &wasm[..];
+    let mut kind_byte = None;
+    loop {
+        match p.parse(cur, true).unwrap() {
+            wasmparser::Chunk::Parsed { consumed, payload } => {
+                if let wasmparser::Payload::ElementSection(reader) = &payload {
+                    // Element section bytes start with the count LEB,
+                    // then each segment begins with the kind byte.
+                    let range = reader.range();
+                    // First byte after the count LEB:
+                    let bytes = &wasm[range.start..range.end];
+                    // Count LEB is small (1) here.
+                    let (_count, count_len) = leb128_u32(bytes);
+                    kind_byte = Some(bytes[count_len]);
+                    break;
+                }
+                cur = &cur[consumed..];
+                if matches!(payload, wasmparser::Payload::End(_)) {
+                    break;
+                }
+            }
+            _ => break,
+        }
+    }
+    let kind = kind_byte.expect("module must have an element section");
+    assert_ne!(
+        kind, 0x00,
+        "walrus emitted the MVP element-segment form (variant 0) for a \
+         table64 active segment; engines like V8 reject this. The fix is \
+         to use the explicit-table-index form (variant 2) whenever the \
+         target table is a table64 (or whenever the offset is i64)."
+    );
+
+    // Sanity check: also round-trip through walrus itself.
+    let module2 = config
+        .parse(&wasm)
+        .expect("emitted wasm must round-trip through walrus");
+    let table = module2.tables.iter().next().expect("should have a table");
+    assert!(table.table64, "table64 flag must survive round-trip");
+}
+
+fn leb128_u32(bytes: &[u8]) -> (u32, usize) {
+    let mut result = 0u32;
+    let mut shift = 0;
+    let mut i = 0;
+    loop {
+        let b = bytes[i];
+        result |= ((b & 0x7f) as u32) << shift;
+        i += 1;
+        if b & 0x80 == 0 {
+            return (result, i);
+        }
+        shift += 7;
+    }
+}

src/module/elements.rs

@@ -254,12 +254,20 @@ impl Emit for ModuleElements {
             ) {
                 match kind {
                     ElementKind::Active { table, offset } => {
-                        // When the table index is 0, set this to `None` to tell `wasm-encoder` to use
-                        // the backwards-compatible MVP encoding.
-                        let table_index =
-                            Some(cx.indices.get_table_index(*table)).filter(|&index| index != 0);
+                        // When the table index is 0 *and* the table is
+                        // a regular i32 table, set this to `None` to
+                        // tell `wasm-encoder` to use the backwards-
+                        // compatible MVP encoding. For table64 tables
+                        // the MVP encoding is not applicable (its
+                        // offset must be i32), so we always include
+                        // the explicit table index, which selects the
+                        // bulk-memory form that carries an i64 offset.
+                        let table_index = cx.indices.get_table_index(*table);
+                        let is_table64 = cx.module.tables.get(*table).table64;
+                        let encoded_table_index =
+                            Some(table_index).filter(|&idx| idx != 0 || is_table64);
                         wasm_element_section.active(
-                            table_index,
+                            encoded_table_index,
                             &offset.to_wasmencoder_type(cx),
                             els,
                         );