Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wat2wasm: data section memory index #2518

Open
dy opened this issue Dec 6, 2024 · 5 comments
Open

wat2wasm: data section memory index #2518

dy opened this issue Dec 6, 2024 · 5 comments

Comments

@dy
Copy link

dy commented Dec 6, 2024

Take the following code:

(module
  (memory $n 1)
  (memory $m 1)
  (data (memory $m) (i32.const 0))
)

Compile.

The data section has bytes:

; data segment header 0
0000012: 02                                        ; segment flags
0000013: 41                                        ; i32.const
0000014: 00                                        ; i32 literal
0000015: 0b                                        ; end

As you can see from spec, flag == 2 expects memory index u32, but wabt omits it.

If you try to compile that code in JS, you'll see the error:

CompileError: WebAssembly.Module(): invalid memory index 65 for data section (having 2 memories) @+18

Adding memory index after segment flag solves the issue (watr has that fixed).

@keithw
Copy link
Member

keithw commented Dec 6, 2024

Hmm, I can't speak to the web demo, but this seems to work fine in the actual main branch of the codebase. Are you able to bisect and identify when this was fixed in the code? We might just need to update the demo.

@dy
Copy link
Author

dy commented Dec 6, 2024

It seems have been fixed for at least 2 years - I could not compile past 1.0.32. I guess demo only needs to update the libwabt.js.

@dy
Copy link
Author

dy commented Dec 7, 2024

@keithw that's weird: I've rebuilt the libwabt.js (see PR), but it still has same issue! Whereas bin/wat2wasm compiles just fine. It makes me wonder if issue is in some js include or something like that.

@sbc100
Copy link
Member

sbc100 commented Dec 7, 2024

I assume you enabled the multi-memory feature (which is still off by default)?

@dy
Copy link
Author

dy commented Dec 8, 2024

Without that feature wabt throws error on validation stage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants