wasm-interp: Handle refs_ correctly

[SoniEx2]

This fills in some TODOs, and cleans up some other issues, relevant for GC later on.

We tried our best to make this efficient, hence the custom wabt opcodes.

[SoniEx2]

@sbc100 (rebased, ptal)

[SoniEx2]

oh yeah, we should probably bring it up: an alternative to the funny stuff with locals and refs_ would be to mark ref locals at function start, which would be decidedly faster if slightly more annoying to set up. but then again, we weren't really going for maximum performance with this.

[SoniEx2]

we don't know how complete the interpreter GC (not to be confused with the GC proposal and its implementation) is (how do you even test something like that), but this "should" mark refs correctly now. we think. we're pretty sure, but we wouldn't mind a second opinion.

[sbc100]

Are there any tests we can now pass after this change?

[SoniEx2]

as we were saying, we're not sure if there's a (good) way to test this?

as far as we can tell, Store::Collect is never called while the interpreter is running, so it kinda just... leaks?

[sbc100]

So are you proposing landing this without tests? Or is this still a draft PR with more changes coming maybe?

[SoniEx2]

we suppose we can try making the GC run regularly and see if it blows up... (but actually enabling the GC to run on its own should be a separate change)

[SoniEx2]

running GC every instruction does indeed blow up.

[SoniEx2]

we can't currently add tests (actually this doesn't need new tests as much as it needs a way to run the existing tests with actual collection enabled), blocked on fixing a nasty issue with spectest-interp, but these changes do fix a bunch of assertion failures when running store_.Collect() for every instruction. we would indeed like to land this without tests (for now).

[SoniEx2]

any chance of getting this merged? @sbc100

[sbc100]

I suppose I'm ok with landing this as an internal refactor. Hopefully any followups can have tests?