It doesn't appear that the data section is being generated for a particular wast file when running run-gen-spec-test.py

[saambarati]

I'm compiling the memory.wast test from the spec to JS code:
https://github.com/WebAssembly/spec/blob/b055d01ea1dfdd7a5231ae779095435f836de97f/interpreter/test/memory.wast

The output I get is this:
https://gist.github.com/saambarati/74490ce6882d121ffd5933f288e9e69d

If you look at the 3rd, 4th, and 5th call to assert_invalid, assert_invalid is called with the same binary. This doesn't appear to contain the "data" section at all; compare it to the .wast file's 3rd, 4th, 5th assert_invalid which does have a data section. I think wabt is miscompiling these.

[saambarati]

It looks like the code is doing some validation on its own, and failing silently:

I think this line of code (inside binary-writer.c):
if (module->memories.size && module->data_segments.size)

should become:
if (module->data_segments.size)

That said, I'm not sure what the intention of the wabt tool. Is it to do some kind of verification? If so, then this verification should stay in (and perhaps be an error). If the purpose is to translate a (potentially invalid module), then it shouldn't do this verification. My understanding is that the tool should just translate the wast even if it's invalid.

[saambarati]

Another place that fails to translate the wast in arbitrary ways:

(assert_invalid
  (module (memory 1) (data (i32.ctz (i32.const 0))))
  "constant expression required"
)

Won't get properly compiled in this loop (because of I think the write_init_expr):

    for (i = 0; i < module->data_segments.size; ++i) {
      const WasmDataSegment* segment = module->data_segments.data[i];
      write_header(ctx, "data segment header", i);
      int memory_index =
          wasm_get_memory_index_by_var(module, &segment->memory_var);
      wasm_write_u32_leb128(&ctx->stream, memory_index, "memory index");
      write_init_expr(ctx, module, segment->offset);
      wasm_write_u32_leb128(&ctx->stream, segment->size, "data segment size");
      write_header(ctx, "data segment data", i);
      wasm_write_data(&ctx->stream, segment->data, segment->size,
                      "data segment data");
    }

[binji]

Good catch!

That said, I'm not sure what the intention of the wabt tool.

Well, the binary writer is one of the older parts of the code. It used to be that wabt was just a text -> binary converter, and it would always validate the module before writing. Now that wabt does a lot more, it makes sense to allow writing invalid modules.

[saambarati]

Thanks for the fix. Works for me.