imported functions

[MarkSwanson]

How do I configure the wasm environment so imported functions work?
If I use dlopen() and dlsym() to get the address of a function (call_test), how do I inject that function address into the wasm linear memory so when I call wasm functions they work correctly (because those wasm functions call my imported function).

Example: this Rust fn compiles down to wasm

#[no_mangle]
pub fn test(n: i32) -> i32 {
    let _a = unsafe {call_test(1)};  // how do I place the dlsym address for call_test() into the wasm environment?
    n
}

If there is no mechanism, and you have a moment, please consider writing out the steps to implement this and I'll do it.

Thanks!

[sunfishcode]

There is no mechanism yet, but I can write out the steps.

We have two main options here.

The place where you can plug a raw function address in is the relocate function. Currently it just knows how to plug in addresses of functions compiled by wasmstandalone itself (see target_func_address). So, the first option is to teach that code to add code to handle the case where r.func_index is the index of an imported function.

• the wasm function index space is defined to start with the imported functions, followed by the defined functions. compilation.functions contains just the defined functions, so the code needs to check if the index is less than the number of imports: if so, then it should look up the import, and if not, subtract the number of imports from the index before indexing into compilation.functions.
• to support lookups for imported functions, we'll need to add an import table that can be passed into compile_module (and then to relocate) to map import indices to function addresses.

The other option is to port wasmstandalone to the new SimpleJIT API. The demo is an example of how to use the API. This option would be more work, but simplejit provides more features, such as built in dlsym support and basic linking. In the future, it may offer more features like jit caching and invalidation. If you choose this way, I can write up a more detailed plan, but for now, some high-level considerations are:

• simplejit would obviate the relocate function altogether, most of the execute function, and the compilation data structures. It handles allocating memory and making it executable, as well as all relocations.
• simplejit uses a simple string symbol namespace, so if you want to automatically resolve imports in one module to exports in another, you'll need a name mangling scheme to mangle the field and module names into a single name.
• simplejit is new, so it may need some work to support everything needed by wasm.

If you have any questions or something here isn't clear, please let me know!

[MarkSwanson]

Hello Sunfishcode,

Thank you for taking the time to write this out.
I can definitely see some benefits to using SimpleJIT.
However, unless you strongly feel otherwise, I'm going to take the 'easy' way out this time. I reserve to change my mind in the morning :-)

relocate() question: I'm good up until write_unaligned(reloc_address as *mut i32, <what_goes_here>);

It's really not a reloc_delta_i32 anymore right?
Well, I mean... should I just use zero for that value? (because there is no relocation for imported functions?)
also, the reloc_address is just the address of the dlsym() pointer to the function right?
(I'm missing the reason behind the +4 bytes...)

Thanks!

[sunfishcode]

However, unless you strongly feel otherwise, I'm going to take the 'easy' way out this time. I reserve to change my mind in the morning :-)

Sounds good!

relocate() question: I'm good up until write_unaligned(reloc_address as *mut i32, <what_goes_here>);

The short answer is, if you arrange for target_func_address to hold the value you got from dlsym, the rest of that code should just work.

The write_unaligned is writing into the machine code buffer. As the TODO above that code notes, what it's currently doing here is specific to x86-64. x86-64 call instructions have a 32-bit offset that says where to jump to, and the offset is relative to the address of the end of the call instruction itself (this is often called PC-relative addressing). target_func_address is the address of the callee. reloc_delta_i32 is the offset from the end of the call instruction to target_func_address. The + 4 happens because in x86-64, the bytes for a call look like this:

   e8 xx xx xx xx
   ^  ^          ^
   a  b          c

where (a) is the beginning of the call, (b) is where the offset field begins, and (c) is the end of the call, which is the point that the offset is relative to. The relocation is requesting a patch at address (b), so we have to add 4 to compute the address of (c) in order to compute the offset from (c) to the target address. That offset is what we write into those xx bytes.

[MarkSwanson]

Thank you for explaining.
Something to consider: the native code that wasm is calling may be further away than an i32 offset. Fun fact: the binary I'm working with is > i32.

Is it possible for us to:
mov rax, dlsym() ptr
call rax

Also, iirc there were some ideas tossed around involving mmap'ing a 4GB linear array so we wouldn't have to do bounds checks. Wouldn't this ensure that the native code wasm is calling could likely be > i32 offset?

[sunfishcode]

Yes, in recent versions of cretonne-native, external functions have a colocated flag, which declares that they can use PC-relative addressing. If colocated is set to false, calls are lowered as a mov and indirect call as you describe.

Cretonne supports the "4GiB trick" for avoiding bounds checks in linear memory, however linear memory is typically not allocated in the same place as compiled code, so it typically doesn't affect code offsets.

[MarkSwanson]

colocated : glad to see that. I've walked through how colocated is set, and I believe it will always be false for imported functions (Linkage.is_definable() will always be false for an 'Import'). This looks like it means I don't have any work to do :-)
(because finalize_all() won't call finalize_function() for imported functions, so colocated won't be set to true)

Minor nit:
--- a/lib/module/src/module.rs
+++ b/lib/module/src/module.rs
@@ -574,7 +574,7 @@ where
let info = &self.contents.functions[func];
debug_assert!(
info.decl.linkage.is_definable(),

•            "imported data cannot be finalized"
  

•            "imported function cannot be finalized"
         );
         self.backend.finalize_function(
             info.compiled.as_ref().expect(
  

Also, is it true that lib/module/src/module.rs/define_function() is never called for imported functions?

I'm guessing that declare_func_in_func() is used for imported fns...

Wrt the 4GiB trick, I quite like it :-)

[sunfishcode]

Yes, that diff looks right.

And yes, define_function is not called for imported functions. You declare an import with declare_function with a Linkage of Import.

declare_func_in_func is for translating a module-level FuncId into a FuncRef that you can use for calls in in_func. (This is admittedly a little non-obvious; the reason we have module-level IDs vs function-level IDs is to allow functions to be compiled in parallel, however I'd ideally like to find a way to streamline this.)

[sunfishcode]

I should also mention that cretonne-module, the code in lib/module isn't currently used by wasmstandalone. cretonne-module is one of the higher-level API layers, and it's what SimpleJIT uses, while wasmstandalone is currently just using the lower-level APIs and doing the "module" bookkeeping itself.

[MarkSwanson]

Ah, good to know - thanks.

[MarkSwanson]

I seem to be stuck on this bug: bytecodealliance/cranelift#328
I have println!(...) in the modified relocate fn but they never get called...

Maybe there is still some issue with cretonne-wasm ?
(Or perhaps I misunderstood your comment when you referenced the demo?)

My fork/branch containing the new relocate() fn:
MarkSwanson@92551c3

Thanks!

[MarkSwanson]

I'll copy my test code into that branch too, so you can see how I'm setting up the environment, compiling, and executing...
(this evening or tomorrow morning)

[sunfishcode]

I'm not aware of any problems in cretonne-wasm that would cause trouble here. In any case, I've now added a patch to master to update wasmstandalone to the latest cretonne version, so you may want to add that patch to your branch, just to avoid any possible issues in the older version.

I looked at your wasmstandalone branch, and it looks like you're on the right track. It's not immediately obvious to me why it's not working for you. I'll take another look once you add the test code.

[MarkSwanson]

I've applied your patch to my branch. Thanks for helping with that.
I still get the same error.
Hmm... strange, I merged my test code into my wasmstandalone branch and I get further along.
Fyi you should be able to run via:
$ cd sip-test
$ ./make

I've pushed to github, but you might not even need to look at it because the stack trace may explain enough:

thread 'main' panicked at 'this variable is used but its type has not been declared', libcore/option.rs:914:5
It would be nice to know what the name of 'this variable' is :-)

...
   9: <core::option::Option<T>>::expect
             at /checkout/src/libcore/option.rs:302
  10: <cretonne_frontend::frontend::FunctionBuilder<'a, Variable>>::use_var
             at /home/mswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/cretonne-frontend-0.8.0/src/frontend.rs:319
  11: cretonne_wasm::code_translator::translate_operator
             at /home/mswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/cretonne-wasm-0.8.0/src/code_translator.rs:60
  12: cretonne_wasm::func_translator::parse_function_body
             at /home/mswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/cretonne-wasm-0.8.0/src/func_translator.rs:200
  13: cretonne_wasm::func_translator::FuncTranslator::translate_from_reader
             at /home/mswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/cretonne-wasm-0.8.0/src/func_translator.rs:99
  14: wasmstandalone_runtime::ModuleTranslation::compile
             at lib/runtime/src/lib.rs:580
  15: wasmstandalone_execute::compile_module
             at lib/execute/src/lib.rs:29
  16: sip::compile_and_execute
             at sip-bins/src/bin/sip.rs:148
...

I'm guessing I need to call declare_function() first?

[MarkSwanson]

Hey there, were you able to look at my branch updates and try my make?
Any thoughts on calling declare_function?

[sunfishcode]

This doesn't look like a declare_function issue. It looks like a declare_var issue, however cretonne-wasm should be calling declare_var itself, as needed, so this may be a bug there. I'm investigating.

[sunfishcode]

The problem is this code in wasmstandalone. It's looking up the signature for a function by its index, self.module.signatures[self.module.functions[func_index]], but the problem is that self.module.functions only contains defined functions, so it's incorrect if the wasm module contains any imports. As a result, it was getting the wrong signature for the function, which meant cretonne-wasm wasn't declaring the correct number of parameter variables.

I created a patch here which seems to fix the issue. With this, the testcase here now fails with "Wasm file './tmp/sip_test.wasm' failed to compile with error: "Wasm did not define function: call_test", but that may be a different issue.

[MarkSwanson]

The compile error was my fault.
My test code was attempting to make a native call into a wasm imported function. :-)

I've changed that so my test code calls test() instead (a real wasm exported function).
And we get further until another small hiccup:

thread 'main' panicked at 'index out of bounds: the len is 3 but the index is 3', /checkout/src/libcore/slice/mod.rs:2342:10
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:463
   5: std::panicking::begin_panic_fmt
             at libstd/panicking.rs:350
   6: rust_begin_unwind
             at libstd/panicking.rs:328
   7: core::panicking::panic_fmt
             at libcore/panicking.rs:71
   8: core::panicking::panic_bounds_check
             at libcore/panicking.rs:58
   9: <usize as core::slice::SliceIndex<[T]>>::index
             at /checkout/src/libcore/slice/mod.rs:2342
  10: core::slice::<impl core::ops::index::Index<I> for [T]>::index
             at /checkout/src/libcore/slice/mod.rs:2238
  11: <alloc::vec::Vec<T> as core::ops::index::Index<I>>::index
             at /checkout/src/liballoc/vec.rs:1708
  12: sip::execute
             at sip-bins/src/bin/sip.rs:229

The sip.rs code snippet is:

    let fn_index: usize = match compilation.module.exports.get(fn_name) {
        None => return Err(format!("Wasm did not define function: {}", fn_name)),
        Some(export) => {
            match export {
                Export::Function(function_index) => {
                    *function_index
                },
                Export::Table(_table_index) => {
                    return Err(format!("Strangely, your function name: {} was actually a table.", fn_name));
                },
                Export::Memory(_memory_index) => {
                    return Err(format!("Strangely, your function name: {} was actually a memory.", fn_name));
                },
                Export::Global(_global_index) => {
                    return Err(format!("Strangely, your function name: {} was actually a global.", fn_name));
                },
            }
        },
    };
    //let start_index = compilation.module.start_func.ok_or_else(|| {
        //String::from("No start function defined, aborting execution")
    //})?;
    let code_buf = &compilation.functions[fn_index];

Thinking that maybe the indexed function may be affecting the index I tried this:

    let code_buf = &compilation.functions[fn_index - 1];

And this gives me:
./make: line 9: 23501 Segmentation fault (core dumped) RUST_BACKTRACE=1 ../target/debug/sip --wasm-file ./tmp/sip_test.wasm --proxy-lib ../target/debug/libproxylib.so --function test

I modified sip-test/src/template.rs to not call the imported function 'call_test' and it worked successfully.

I changed call_test(i32) -> i32 into call_test() -> i32.
But that didn't seem to help.

Any thoughts why calling the imported function fails?

I have my changes checked into github (includes your patch).

Thanks!

[sunfishcode]

compilation.functions only contains defined functions, while the fn_index you get from the export is the wasm function index space which includes the imports, so you need to adjust it by the number of imports.

A similar issue but unrelated to the immediate problem, the new code in relocate that does imported_functions.len() - r.func_index should be r.func_index - imported_functions.len().

(As an aside, handling function index spaces is error-prone in all wasm codebases I've worked on, not just cretonne. Perhaps we should introduce distinct types for function definition indices vs function indices, so keep the two from getting confused).

The next issue is in the relocation patching code. With the newer cretonne version emitting non-colocated calls via movabs + indirect call, the code now needs an 8-byte absolute immediate, rather than being able to assume all calls use 4-byte pc-relative offsets. I implemented this in the following patch: d162f8b

While there, I also remembered that cretonne-codegen is now folding the magic "+ 4" offset into the addend now, so we no longer need to do it explicitly in relocate, so I included that fix as well.

With those three things fixed, your testcase executes successfully, for me :).

[MarkSwanson]

Oh that's such great news!!:-D \o/
Will play with this later today.

[MarkSwanson]

Ok, it also works for me! :-)

Thanks so much for your help with this.

Wrt enhancing error-prone function indexes: +1

Fyi my fork should demo this successfully ootb.