Stream: git-wasmtime

Topic: wasmtime / Issue #1532 Cranelift WASM FuncTranslator::tra...

Wasmtime GitHub notifications bot (Apr 17 2020 at 14:43):

bkolobara opened Issue #1532:

Hi everyone,

I need help understanding a Cranelift panic when translating a WASM module with FuncTranslator::translate:

(module
  (func $one (result i32)
      i32.const 42)
  (func $add (param $p1 i32) (param $p2 i32) (result i32)
      local.get $p1
      local.get $p2
      i32.add)
  (export "add" (func $add)))

When calling translate on this 2 functions the first one works as expected, but the second call panics with: thread 'main' panicked at 'variable Variable(0) is used but its type has not been declared', /Users/bkolobara/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-frontend-0.62.0/src/frontend.rs:283:17

I have spent a lot of time trying to figure out what's going on here, but just can't get to the bottom of it. What type declaration is missing here?

Wasmtime GitHub notifications bot (Apr 17 2020 at 16:00):

bkolobara edited Issue #1532:

Hi everyone,

I need help understanding a Cranelift panic when translating a WASM module with FuncTranslator::translate:

(module
  (func $one (result i32)
    i32.const 42)
  (func $add (param $p1 i32) (param $p2 i32) (result i32)
    (i32.add
      (get_local $p1)
      (get_local $p2)))
  (export "add" (func $add)))

When calling translate on this 2 functions the first one works as expected, but the second call panics with: thread 'main' panicked at 'variable Variable(0) is used but its type has not been declared', /Users/bkolobara/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-frontend-0.62.0/src/frontend.rs:283:17

I have spent a lot of time trying to figure out what's going on here, but just can't get to the bottom of it. What type declaration is missing here?

Wasmtime GitHub notifications bot (Apr 17 2020 at 16:31):

abrown commented on Issue #1532:

Are you using module_translator::translate_module? There is a lot of module-level stuff in there that might need to happen.

Wasmtime GitHub notifications bot (Apr 17 2020 at 17:49):

bkolobara commented on Issue #1532:

I use cranelift_wasm::translate_module() and then forward the returned ModuleTranslationState to FuncTranslator::translate().

Wasmtime GitHub notifications bot (Apr 17 2020 at 18:06):

abrown commented on Issue #1532:

:+1: Can you post a #[test] snippet so that I can try to reproduce? And maybe the full backtrace?

Wasmtime GitHub notifications bot (Apr 17 2020 at 18:50):

bkolobara commented on Issue #1532:

Thanks @abrown for helping me out. I managed to figure out what the issue was.

When calling FuncTranslator::translate() I pass a context (created with let mut context = cranelift_codegen::Context::new();) to it, but I never assigned a signature to the function in the context.

I was missing a context.func.signature = ... before the call to translate(). What really confused me is that it worked with functions that only have a return types.

Wasmtime GitHub notifications bot (Apr 17 2020 at 18:50):

bkolobara closed Issue #1532:

Hi everyone,

I need help understanding a Cranelift panic when translating a WASM module with FuncTranslator::translate:

(module
  (func $one (result i32)
    i32.const 42)
  (func $add (param $p1 i32) (param $p2 i32) (result i32)
    (i32.add
      (get_local $p1)
      (get_local $p2)))
  (export "add" (func $add)))

When calling translate on this 2 functions the first one works as expected, but the second call panics with: thread 'main' panicked at 'variable Variable(0) is used but its type has not been declared', /Users/bkolobara/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-frontend-0.62.0/src/frontend.rs:283:17

I have spent a lot of time trying to figure out what's going on here, but just can't get to the bottom of it. What type declaration is missing here?

Wasmtime GitHub notifications bot (Apr 17 2020 at 19:02):

abrown commented on Issue #1532:

Ha, yes, that is a bit tricky. I do see that translate has documentation about requiring a name and signature but it might not be clear enough. What do you think about adding an assert or debug_assert somewhere to make sure all the requirements are met and, if not, fail with a more descriptive message?

Wasmtime GitHub notifications bot (Apr 17 2020 at 19:03):

abrown edited a comment on Issue #1532:

Ha, yes, that is a bit tricky. I do see that translate has documentation about requiring a name and signature but it might not be clear enough--and easy to overlook in any case. What do you think about adding an assert or debug_assert somewhere to make sure all the requirements are met and, if not, fail with a more descriptive message?

Wasmtime GitHub notifications bot (Apr 18 2020 at 11:56):

bkolobara commented on Issue #1532:

Having an assert to check that the input requirements for translate are met would have saved me a lot of time. It would stop the error from propagating too deep, where it's becomes really hard to reason about it if you are not familiar with the internals of Cranelift.

Wasmtime GitHub notifications bot (Apr 18 2020 at 22:10):

abrown commented on Issue #1532:

I agree; do you want to add it or do you want me to?

Wasmtime GitHub notifications bot (Apr 19 2020 at 13:31):

bkolobara commented on Issue #1532:

I prefer you doing it :). Thanks a lot!

Wasmtime GitHub notifications bot (Apr 22 2020 at 20:26):

abrown commented on Issue #1532:

Took a look at this today and it is a bit trickier than I initially thought: we can't just debug_assert that the func.signature has the same number of parameters as the Wasm function because we haven't read the Wasm function yet. And an empty signature (no params, no returns) doesn't mean it is unitialized; that is a perfectly valid signature. An alternative would be to change Function::signature to an Option<Signature> but that's a whole other can of worms. I'm going to leave this as-is until I think of a better option.

Last updated: Oct 23 2024 at 20:03 UTC