disasm: allow to disassemble code without validating it

[dennwc]

This CL splits Disassemble function into two separate functions: DisassebleRaw and Disasseble. The first one decodes WASM instructions without checking for validity. And the second function validates the list of instructions from the first function.

This was done to bypass any bugs in the validation code like #69 and be able to manipulate assembly code.

[codecov-io]

Codecov Report

Merging #81 into master will increase coverage by 0.18%.
The diff coverage is 68.75%.

@@            Coverage Diff            @@
##           master     #81      +/-   ##
=========================================
+ Coverage   67.52%   67.7%   +0.18%     
=========================================
  Files          32      32              
  Lines        2987    3010      +23     
=========================================
+ Hits         2017    2038      +21     
- Misses        736     737       +1     
- Partials      234     235       +1

Impacted Files	Coverage Δ
disasm/asm.go	100% <100%> (ø)	⬆️
cmd/wasm-dump/main.go	44.3% <100%> (ø)	⬆️
exec/vm.go	89.25% <100%> (ø)	⬆️
disasm/disasm.go	81.78% <67.53%> (+0.85%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 876a973...eb3e410. Read the comment docs.

[sbinet]

thanks.

I think I'd rather go the other way around:

• make Disassemble not decode any wasm instruction
• introduce DisassembleValidate (?) that validates the wasm instructions.

perhaps have a Validate method on the Disassembly type instead of this ugly DisassembleValidate function?

Alternatively, we could perhaps introduce a new type, akin to the encoding/json.Decoder (e.g. Disassembler) that has a Disassemble method (that would only disassemble, w/o any validation)
that new type could have an exposed field, Validate bool, to enable the validation while disassembling.

perhaps the less invasive for now would be something along the lines of:

// Disassemble disassembles a given function body into a set of instructions.
// It won't check operations for validity.
func Disassemble(code []byte) ([]Instr, error) {
    ...
}

type Disassembly struct { ... }

// Disassemble disassembles the given function. It also takes the function's
// parent module as an argument for locating any other functions referenced by
// fn.
func (d *Disassembly) Disassemble(fn wasm.Function, module *wasm.Module) error {
    instrs, err := Disassemble(fn.Body.Code)
    if err != nil { ... }
    // ... what used to be the body of Disassemble ...
   return err
}

what do you think?

[dennwc]

I think the Disassemble+Validate is the best option for now.

I like an idea of defining a Disassembler and the Validate flag, but it will be a bit harder to use because the user will have to set the function signature and the module pointer as well. With Validate they will be impossible to miss since they will be listed as arguments.

Populating a Disassembly struct is also not very intuitive.

[sbinet]

sorry... I have been distracted (by apache/go-arrow)

I am not sure I completely get what you're advocating for (sorry!).

point noted about populating Dissambly not being too intuitive.

what about:

// Disassemble disassembles a given function body into a set of instructions.
// It won't check operations for validity.
func Disassemble(code []byte) ([]Instr, error) {
    ...
}

// NewDisassembly disassembles the given function. It also takes the function's
// parent module as an argument for locating any other functions referenced by
// fn.
func NewDisassembly(fn wasm.Function, module *wasm.Module) (*Disassembly, error) {
    instrs, err := Disassemble(fn.Body.Code)
    if err != nil { ... }
    // ... what used to be the body of Disassemble ...
   return err
}

[dennwc]

@sbinet Changed according to your last suggestion. PTAL

[sbinet]

LGTM (modulo that last one nitpick.)

[sbinet]

Please add a copyright header.

[sbinet]

Meh... I can add the header later :)