binaryen icon indicating copy to clipboard operation
binaryen copied to clipboard

Questions Related to Preserving Data Segments in BinaryenSetMemory C API

Open mobsceneZ opened this issue 1 year ago • 2 comments

In Wasm Specification, data segments and memory are seperated. However, in Binaryen's current C API, they are deeply coupled.

This leads to some awkward use case, suppose there is a wasm module containing only data segments, and I want to add memory into it, I have to manually copy all existing data segments and pass them into BinaryenSetMemory, which is unnecessary if we can preserve original data segments.

Therefore, I wonder if we could add an additional argument to BinaryenSetMemory that indicates whether we should preserve the data segments or not. If that's reasonable, I already created a pull request #6313 and maybe you can check that out ;).

mobsceneZ avatar Feb 15 '24 13:02 mobsceneZ

suppose there is a wasm module containing only data segments

Data segments are not valid without a memory, though? Every data segment refers to a memory. Sorry, I think I'm missing something here.

kripken avatar Feb 15 '24 17:02 kripken

suppose there is a wasm module containing only data segments

Data segments are not valid without a memory, though? Every data segment refers to a memory. Sorry, I think I'm missing something here.

Hey kripken, for active data segment, the answer is yes. But for passive data segment, it's always valid even though a memory is not present (according to https://webassembly.github.io/spec/core/valid/modules.html#data-segments)

Consider following simplified testcase from WebAssembly Core Specification:

(module
  (type (;0;) (func))
  (func (;0;) (type 0)
    data.drop 1)
  (data (;0;) "")
  (data (;1;) ""))

It's valid and you can use wat2wasm from wabt to check that out.

mobsceneZ avatar Feb 16 '24 05:02 mobsceneZ

I see, thanks. Yes, it sounds like we should find a way in the C API to support such modules.

kripken avatar Feb 20 '24 19:02 kripken

I see, thanks. Yes, it sounds like we should find a way in the C API to support such modules.

@kripken Actually, Binaryen does support parsing modules which contain data segments and no memory but the C API currently does not provide a convenient way to operate data segments seperately.

Do you think it's necessary to seperate out memory operation and data segment operation? Or we could add an ad-hoc boolean argument like keepSegment to BinaryenSetMemory, and if it's set to true, we keep original data segments and the new data segments will just be appended to the internal map.

I will try to create a new PR if any of above options looks convinient for you.

mobsceneZ avatar Feb 21 '24 12:02 mobsceneZ

I think adding an ad-hoc boolean might be more confusing. Adding a new API to just add a data segment seems simplest?

kripken avatar Feb 22 '24 17:02 kripken

@kripken hey kripken, I added corresponding API in PR #6346, you can check that out :).

mobsceneZ avatar Feb 25 '24 15:02 mobsceneZ