wasmx / wasm-chisel

Some useful utilities to transform WebAssembly binaries.
Apache License 2.0
56 stars 11 forks source link

Add support for remapstart #107

Closed jwasinger closed 5 years ago

jwasinger commented 5 years ago

Fixes https://github.com/wasmx/wasm-chisel/issues/99

jwasinger commented 5 years ago

check if a start section is present. If not, just quit.

Already implemented as described.

check if an export named main exists. If yes, start (recursively) rename so that there is no main function.

I have a comment in the code because I wasn't sure whether to expect the module to be validly formed when remapstart is run. But I see now that I will have to make a change to fail without panicking when main export is not found.

Also, I'm not sure that implementing this with recursion is much cleaner than what I am currently doing?

insert a new export called main which points to the same function index what the start section was using

Already implemented as described.

remove the start section

Already implemented as described.

axic commented 5 years ago

Lets simplify my last comment https://github.com/wasmx/wasm-chisel/pull/107#pullrequestreview-245932155:

As a way to organise I'd say it should:

  1. check if a start section is present. If not, just quit.
  2. drop the names section (not sure if exports are duplicated in names, but lets just drop it and we can fix later)
  3. check if an export named main exists. If yes, remove that entry from the export section.
  4. insert a new export called main which points to the same function index what the start section was using
  5. remove the start section
axic commented 5 years ago

@jwasinger can you squash this to a single commit?