`wasm2wat --generate-names` produces invalid names

[fitzgen]

trafficstars

Test Case

;; test.wat
(module
  (table (;0;) 102 102 funcref)
  (elem (;0;) (i32.const 1) func))

Steps to Reproduce

$ wat2wasm test.wat -o test.wasm
$ wasm2wat --generate-names test.wasm -o with-names.wat
$ wat2wasm with-names.wat -o with-names.wasm
with-names.wat:3:4: error: undefined table variable "$e0"
  (elem $e0 (i32.const 1) func))
   ^^^^ 

Expected Results

The Wasm file is round trippable through wasm2wat --generate-names without any errors.

Actual Results

wasm2wat --generate-names produces invalid element segment names (I think they are in the place where the table name belongs?)

[sbc100]

Does it make sense to name active segments? Can the name be used anywhere?

[fitzgen]

Oh another idea: WABT is in oss-fuzz right? It may make sense to add a fuzz target that does something like:

void fuzz_target(uint8_t* ptr, size_t len) {
    if !is_valid_wasm(ptr, len) {
        return;
    }

    // Do the equivalent of `wasm2wat --generate-names`.
    size_t wat_len = 0;
    char* wat = wasm2wat_with_generate_names(ptr, len, &wat_len);

    // Convert it back to Wasm.
    size_t wasm_len = 0;
    uint8_t* wasm = wat2wasm(wat, wat_len, &wasm_len);

    // The conversion back to Wasm had better have succeeded without
    // errors and produced a valid Wasm buffer.
    assert(is_wasm_valid(wasm, wasm_len));
}

This should quickly track down this whole class of bug.

[fitzgen]

Does it make sense to name active segments? Can the name be used anywhere?

AFAIK, the name cannot be used anywhere.

Maybe fixing this bug is just deleting a line of code? If so, I'm happy to make a PR if someone points out the relevant line of code and which tests might need updating :)

[sbc100]

Actually in wasm-ld / emscripten we use make use of names for active elem segments so I think for completeness its worth fixing.

[sbc100]

The issue relates to whether bulk memory is enabled. If I enable bulk memory on the failing command the problem goes away:

$ wat2wasm with-names.wat -o with-names.wasm --enable-bulk-memory

This is because of the code in the wat parser which seem to handle the meaning of the name in this position differently between MVP and bulk memory: https://github.com/WebAssembly/wabt/blob/3c79f17a7dadb396f955f0912858c3230ed8e10e/src/wast-parser.cc#L1135-L1139

I think the solution is not the write elem segment names unless bulk memory is enabled. I will work on a patch.