Fix MVP element-segment encoding for table64 active segments (#317)

* 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.

* fixup workflow

Author: guybedford

.github/workflows/main.yml

@@ -1,5 +1,8 @@
 name: CI
-on: [push, pull_request]
+on:
+  pull_request:
+  push:
+    branches: [main]
 
 jobs:
   test:

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,87 @@
+//! Repro for an emit bug: walrus picks the MVP element-segment form
+//! (variant `0x00`, implicit table 0, i32 offset) for active segments
+//! targeting a `table64` table, which V8 rejects with `invalid table
+//! elements limits flags`. `wasmparser` accepts the malformed binary,
+//! so this test asserts at the byte level on the segment-kind tag.
+
+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());
+
+    let func_id =
+        FunctionBuilder::new(&mut module.types, &[], &[]).finish(vec![], &mut module.funcs);
+    module.exports.add("f", func_id);
+
+    let table_id = module
+        .tables
+        .add_local(/* table64 */ true, 1, Some(1), RefType::FUNCREF);
+
+    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();
+
+    let kind = first_element_segment_kind(&wasm).expect("module must have an element section");
+    assert_ne!(
+        kind, 0x00,
+        "walrus emitted MVP element-segment form for a table64 segment; \
+         engines like V8 reject this. Use the explicit-table-index form."
+    );
+
+    let module2 = config.parse(&wasm).expect("must round-trip");
+    let table = module2.tables.iter().next().expect("should have a table");
+    assert!(table.table64, "table64 flag must survive round-trip");
+}
+
+fn first_element_segment_kind(wasm: &[u8]) -> Option<u8> {
+    let mut parser = wasmparser::Parser::new(0);
+    let mut cur = wasm;
+    loop {
+        match parser.parse(cur, true).ok()? {
+            wasmparser::Chunk::Parsed { consumed, payload } => {
+                if let wasmparser::Payload::ElementSection(reader) = &payload {
+                    let range = reader.range();
+                    let bytes = &wasm[range.start..range.end];
+                    let (_count, n) = leb128_u32(bytes);
+                    return Some(bytes[n]);
+                }
+                if matches!(payload, wasmparser::Payload::End(_)) {
+                    return None;
+                }
+                cur = &cur[consumed..];
+            }
+            _ => return None,
+        }
+    }
+}
+
+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,15 @@ 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);
+                        // Use the MVP encoding (implicit table 0) only when the
+                        // table is index 0 and not a table64, since MVP requires
+                        // an i32 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,
                         );