Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with br labels in the JS build #915

Open
dcodeIO opened this issue Sep 23, 2018 · 3 comments
Open

Issue with br labels in the JS build #915

dcodeIO opened this issue Sep 23, 2018 · 3 comments

Comments

@dcodeIO
Copy link
Contributor

dcodeIO commented Sep 23, 2018

Somehow, a module in text format like

(module
  (type $t0 (func (param i32)))
  (type $t1 (func))
  (import "env" "log" (func $log (type $t0)))
  (func $main (type $t1)
    block $B0
      block $B1
        br $B1
        i32.const 3
        call $log
      end
      i32.const 2
      call $log
    end
    i32.const 1
    call $log)
  (start 1))

becomes

(module
  (type $t0 (func (param i32)))
  (type $t1 (func))
  (import "env" "log" (func $log (type $t0)))
  (func $main (type $t1)
    block $B0
      block $B1
        br 3228196 (; INVALID ;)
        i32.const 3
        call $log
      end
      i32.const 2
      call $log
    end
    i32.const 1
    call $log)
  (start 1))

after parsing with libwabt.js. Note the invalid pointer value instead of the label on the br. Any ideas?

See: wasmerio/vscode-wasm#14 (comment)

@binji
Copy link
Member

binji commented Sep 24, 2018

When I copy this in to the wat2wasm demo it seems to produce the correct output.

I think I see the problem, though -- if I modify wat2wasm/demo.js and comment out the call to module.resolveNames I get the behavior you're seeing:

Error: validate failed:
test.wast:4:40: error: function type variable out of range (max 2)
  (import "env" "log" (func $log (type $t0)))
                                       ^^^
test.wast:5:21: error: function type variable out of range (max 2)
  (func $main (type $t1)
                    ^^^
test.wast:8:9: error: invalid depth: 3228196 (max 2)
        br $B1
        ^^
...

So it seems like you should be able to fix by calling resolveNames on the module.

@dcodeIO
Copy link
Contributor Author

dcodeIO commented Sep 24, 2018

Thanks binji, makes sense. Linked your solution on the original issue :)

@binji
Copy link
Member

binji commented Sep 24, 2018

Now that I think of it, this should be clearer in the API as well. In C++ there is an assertion that validate/toBinary aren't called unless the names are resolved, so the JS version should probably thow an exception in that case.

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

No branches or pull requests

2 participants