Rerumu / Wasynth

WebAssembly to Lua translation library and tool
https://discord.gg/sgm5YcmgyD
GNU General Public License v3.0
130 stars 18 forks source link

Add memory.copy/fill #23

Closed schmatz closed 1 year ago

schmatz commented 2 years ago

I don't have much idea of what's going on here, but I think this is a step in the right direction. Please be very vigilant for mistakes as I'm sure they are present. I hope that I can quickly learn and become a net-positive contributor. I started working on this because of this issue.

A few questions that came up when writing this:

  1. The source of the operators, wasmparser, seems to have some fields listed in the structs, but others are missing. For example, their struct for MemoryCopy only has the src and dst fields, whereas there is also a third parameter according to the spec, the amount of memory to copy n. I assumed that this was popped off the stack explicitly - I have no idea why it's like this. MemoryFill is even weirder, only containing one field in the struct, mem and having two more in the spec, value and len. Any context on this?

  2. Why are the parameters not listed in wasmparser Box<Expression> rather than u32, which I'm pretty sure they are according to the spec? Why are they visited in visit.rs?

  3. How do I test this? I try running cargo test but that seems to have a ton of failures. Not sure if it's like that normally. I inspected some of the generated files and I see code which looks approximately right but the tests still fail unfortunately. Not sure if there is a way that people usually manually test this.

Thanks for your help in taking a look at this!

Rerumu commented 2 years ago

The documentation on wasmparser seems unclear, but my experience with the memory instruction is that the memory instance they refer is what's actually encoded in the u32/u8 fields. It's something that's still work in progress within the Wasm specification so despite "supporting" it right now I don't think modules with more than 1 memory instance (index=0) are ever generated in the wild.

So it looks like in this case MemoryFill defines its memory index statically, then pops 3 values off the stack (size, byte, address) to do its operation. Interestingly, MemoryCopy looks to have 2 memory indices, and my intuition says that this is because it should be able to copy between separate instances. It then pops 3 values off the stack too (size, source address, destination address).

As for the second point, the parameters in Box<Expression>s are obtained from a stack emulation step done within factory.rs; it's a syntax tree like representation of the ordered Wasm operations that makes it easier to print more... "standard" Lua code. They're visitable because they can be sub-trees containing ordering information such as whether or not they access other variables or memory.

And finally as for tests, it's not pretty at the moment. There are a lot of features that we are lacking that are in the Wasm test suite, some which aren't even completely finished on their end. For this reason I generally conduct testing for implementing specific features by finding their test file and filtering out the specific test.

schmatz commented 2 years ago

Thanks for the info and tips! I'll iterate on this.