Skip to content
This repository has been archived by the owner on May 11, 2020. It is now read-only.

Warn when modules are missing before attempting disassembly #90

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gballet
Copy link
Contributor

@gballet gballet commented Oct 16, 2018

The goal is to catch the error early, otherwise it will cause a very confusing disasm: stack underflow which makes debugging very difficult.

It won't work otherwise, and the error will be very confusing.
@sbinet
Copy link
Contributor

sbinet commented Oct 16, 2018

thanks, but it seems this breaks a bunch of tests.

@gballet
Copy link
Contributor Author

gballet commented Oct 17, 2018

It seems to trigger a lot of issues that I missed out, indeed. I'm going to fix those in several commits over the next few days.


// We already read the file without trying to resolve imports,
// start over in order to do that work now.
f.Seek(0, os.SEEK_SET)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/os.SEEK_SET/io.SeekStart/

// Decode the module in order to fake its imports as exports
mod, err := wasm.DecodeModule(f)
if err != nil {
log.Fatalf("Could not read module: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Could/could/

ninternal, _ := leb128.ReadVarint32(bytes.NewReader(module.Code.Bytes))
nexternal := len(module.FunctionIndexSpace) - int(ninternal)
if nexternal != len(module.Import.Entries) {
panic(fmt.Sprintf("Attempting to disassemble a module that is missing imports %d != %d", nexternal, len(module.Import.Entries)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/fmt.Sprintf/fmt.Errorf/

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

Successfully merging this pull request may close these issues.

2 participants