Corruption of WasmerEnv when using call indirect to host fn from Wasm with Singlepass

[thedavidmeister]

Describe the bug

If a host function is being called from different crates in the guest then on the host side the env points to garbage, which returns None for memory and can simply panic for other fields.

Steps to reproduce

I made a repo - https://github.com/thedavidmeister/wasmer-env-memory

Expected behavior

Host consistently finds memory and other env fields.

Actual behavior

Host finds None and/or panics due to other env fields pointing to garbage.

Additional context

There is a workaround/fix which is adding #[inline(always)] to any guest function that:

• calls a host function
• expects to be used by other crates

https://github.com/thedavidmeister/wasmer-env-memory/blob/master/other/lib.rs#L1

[thedavidmeister]

cc @MarkMcCaskey

[MarkMcCaskey]

Thanks for the bug report! I did a deep dive on this and found out the cause. It seems to be a bug only affecting singlepass.

The WAT that causes the failure:

(module
  (type (;0;) (func (param i32) (result i32)))
  (type (;1;) (func))
  (type (;2;) (func (param i32 i32) (result i32)))
  (import "env" "__read_memory" (func $__read_memory (type 0)))
  (func $read_memory (type 1)
    (drop
      (call $_ZN5other8dispatch17h053cb34ef5d0d7b0E
        (i32.const 1)
        (i32.const 2)))
    (drop
      (call $__read_memory
        (i32.const 1))))
  (func $_ZN5other8dispatch17h053cb34ef5d0d7b0E (type 2) (param i32 i32) (result i32)
    (call_indirect (type 0)
      (local.get 1)
      (local.get 0)))
  (table (;0;) 2 2 funcref)
  (memory (;0;) 16)
  (global (;0;) (mut i32) (i32.const 1048576))
  (global (;1;) i32 (i32.const 1048576))
  (global (;2;) i32 (i32.const 1048576))
  (export "memory" (memory 0))
  (export "read_memory" (func $read_memory))
  (export "__data_end" (global 1))
  (export "__heap_base" (global 2))
  (elem (;0;) (i32.const 1) func $__read_memory))

The version with inlining that fixes this

(module
  (type (;0;) (func (param i32) (result i32)))
  (type (;1;) (func))
  (import "env" "__read_memory" (func $__read_memory (type 0)))
  (func $read_memory (type 1)
    (drop
      (call $__read_memory
        (i32.const 2)))
    (drop
      (call $__read_memory
        (i32.const 1))))
  (table (;0;) 1 1 funcref)
  (memory (;0;) 16)
  (global (;0;) (mut i32) (i32.const 1048576))
  (global (;1;) i32 (i32.const 1048576))
  (global (;2;) i32 (i32.const 1048576))
  (export "memory" (memory 0))
  (export "read_memory" (func $read_memory))
  (export "__data_end" (global 1))
  (export "__heap_base" (global 2)))

Additionally, running the Wasm through wasm-opt will inline the call and side-step this bug (I imagine compiling with full LTO also avoids it). Just as a side note, it's always good to ensure your Wasm is optimized before giving it to Wasmer, though in this case I'm glad you didn't so we could find this bug 😆 .

[olonho]

Can be fixed along those lines:

diff --git a/lib/compiler-singlepass/src/codegen_x64.rs b/lib/compiler-singlepass/src/codegen_x64.rs
index bd02ddc2a..0444fcf82 100644
--- a/lib/compiler-singlepass/src/codegen_x64.rs
+++ b/lib/compiler-singlepass/src/codegen_x64.rs
@@ -5388,6 +5388,8 @@ impl<'a> FuncGen<'a> {
 
                 let vmcaller_checked_anyfunc_func_ptr =
                     self.vmoffsets.vmcaller_checked_anyfunc_func_ptr() as usize;
+                let vmcaller_checked_anyfunc_vmctx =
+                    self.vmoffsets.vmcaller_checked_anyfunc_vmctx() as usize;
 
                 self.emit_call_sysv(
                     |this| {
@@ -5403,6 +5405,11 @@ impl<'a> FuncGen<'a> {
                             this.trap_table
                                 .offset_to_code
                                 .insert(offset, TrapCode::StackOverflow);
+                            // Prepare context pointer, it isn't always vmctx.
+                            this.assembler.emit_mov(
+                                Size::S64,
+                                Location::Memory(GPR::RAX, vmcaller_checked_anyfunc_vmctx as i32),
+                                Machine::get_param_location(0));
                             this.assembler.emit_call_location(Location::Memory(
                                 GPR::RAX,
                                 vmcaller_checked_anyfunc_func_ptr as i32,

[syrusakbary]

The fix looks correct. I've opened a PR to test it a bit more throughly

[syrusakbary]

This is now fixed in master: #2494. Closing the issue