wasm2wat --generate-names produces invalid names

[fitzgen]

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:

wabt/src/wast-parser.cc

Lines 1135 to 1139 in 3c79f17

	// With MVP text format the name here was intended to refer to the table
	// that the elem segment was part of, but we never did anything with this name
	// since there was only one table anyway.
	// With bulk-memory enabled this introduces a new name for the particular
	// elem segment.

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