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

wasm cannot import globals #4866

Open
fengb opened this issue Mar 30, 2020 · 7 comments
Open

wasm cannot import globals #4866

fengb opened this issue Mar 30, 2020 · 7 comments
Labels
arch-wasm 32-bit and 64-bit WebAssembly backend-llvm The LLVM backend outputs an LLVM IR Module. backend-self-hosted bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase.
Milestone

Comments

@fengb
Copy link
Contributor

fengb commented Mar 30, 2020

Wasm allows you to import global variables:

(module
  (type (;0;) (func (param i32)))
  (type (;1;) (func))
  (import "env" "frobinate" (func $frobinate (type 0)))
  (import "env" "glob" (global (;0;) i32))
  (func (;1;) (type 1)
    global.get 0
    call $frobinate))

But I can't seem to do this in Zig because lld attempts to resolve the variable symbols at link time:

extern "env" fn frobinate(num: u32) void; // this works just fine
extern "env" const glob: u32; // this doesn't work

export fn foo() void {
    frobinate(glob);
}

// $ zig build-lib extern.zig -dynamic -target wasm32-freestanding
// lld: error: ./extern.o: undefined symbol: glob
@daurnimator daurnimator added the arch-wasm 32-bit and 64-bit WebAssembly label Mar 30, 2020
@andrewrk andrewrk added this to the 0.7.0 milestone Mar 31, 2020
@kubkon
Copy link
Member

kubkon commented Jun 4, 2020

This one is really interesting! Do you have any particular example in mind? The example you pasted in in the issue description is actually not corresponding to the Zig's source since foo is not actually exported in your WAT source. I'm sure it's just a typo, but for completeness sake, I guess something like this is what we'd like to support (please verify me here @fengb):

; globals.wat
(module
  (type (;0;) (func (param i32)))
  (type (;1;) (func))
  (import "env" "frobinate" (func $frobinate (type 0)))
  (import "env" "glob" (global (;0;) i32))
  (export "foo" (func $foo))
  (func $foo (type 1)
    global.get 0
    call $frobinate
    return))

Then, we could link that with the following module at runtime:

; globals_env.wat
(module
  (type (;0;) (func (param i32)))
  (global i32 (i32.const 10)) (export "glob" (global 0))
  (export "frobinate" (func $frobinate))
  (func $frobinate (type 0) (param i32)
    return))

You can actually verify that it (at least) doesn't trap using Wasmtime like so:

wasmtime globals.wat --preload env=globals_env.wat --invoke foo

Now, to the issue at hand, I think this might be a problem with lld itself, as I'm having really hard time at getting this to not throw the exact same error in Rust and C. I'll ask around in the Wasm community what their thoughts are on this and report back.

@kubkon
Copy link
Member

kubkon commented Jun 4, 2020

Update: I’ve confirmed that it’s the limitation of LLVM. I’m not sure what the best course of action here is. We could submit an upstream proposal/request for this I guess and work with the LLVM devs at getting it implemented. Thoughts?

@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 17, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 Nov 6, 2020
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 May 19, 2021
@fengb
Copy link
Contributor Author

fengb commented Jun 9, 2021

With the latest compiler, this is no longer a compile error but instead a miscompilation:

(module
  (type (;0;) (func (param i32)))
  (type (;1;) (func))
  (import "env" "frobinate" (func $frobinate|env (type 0)))
  (func $__wasm_call_ctors (type 1))
  (func $foo (type 1)
    i32.const 0
    i32.load
    call $frobinate|env)
  (memory (;0;) 2)

Instead of an import, this function uses the (non-exposed) 0 address in the internal memory.

@Vexu Vexu added bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code. stage1 The process of building from source via WebAssembly and the C backend. labels Jun 9, 2021
@andrewrk andrewrk modified the milestones: 0.11.0, 0.9.1 Nov 27, 2021
@Luukdegram
Copy link
Member

Luukdegram commented Dec 21, 2021

This is actually working as intended.
This is why:
Top-level declarations actually live in the data section (Similar to .bss section for ELF). This is also why you're seeing instructions for loading from memory. What this means is that glob is actually being compiled to an undefined 'data' symbol in the object file. As we link statically, the linker is trying to resolve the undefined symbol during link time, which it can't resolve (hence the error that was being output previously. But since we append --allow-undefined by default, you no longer get that error). When wasm-ld performs relocations for this symbol, it writes 0 as it is undefined.

This does mean above use case is not supported (in no other language such as Rust, C, etc really). While above behavior is correct, it means the user must be aware of the semantics of how declarations live in the linear memory rather than wasm's globals which is very unfortunate.

One possible solution would be to support something like extern "wasm" const foo: i32;, but that would only be supported by the wasm backend as there's no way to tell llvm to use a wasm global instead. Linking with other languages would yield an error but that is fine as such 'wasm' ABI isn't stable anyway so users would have to revert to status quo for that.

@zigazeljko
Copy link
Contributor

there's no way to tell llvm to use a wasm global instead

There is, actually. Since this patch got merged, you can create wasm globals by using addrspace(1) in LLVM IR.

@Luukdegram
Copy link
Member

there's no way to tell llvm to use a wasm global instead

There is, actually. Since this patch got merged, you can create wasm globals by using addrspace(1) in LLVM IR.

Thanks for that! Not sure how I missed that but glad I was proven wrong. Considering stage2 supports address spaces we could support this in both backends then.

@andrewrk andrewrk added backend-llvm The LLVM backend outputs an LLVM IR Module. backend-self-hosted enhancement Solving this issue will likely involve adding new logic or components to the codebase. and removed miscompilation The compiler reports success but produces semantically incorrect code. labels Dec 23, 2021
@andrewrk andrewrk modified the milestones: 0.9.1, 0.10.0 Dec 23, 2021
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@Vexu Vexu removed the stage1 The process of building from source via WebAssembly and the C backend. label Jan 19, 2023
@andrewrk andrewrk removed this from the 0.11.0 milestone Apr 9, 2023
@ElectroluxV2
Copy link

Hi, do you have any workaround to this issue? Or should I ignore it and just use bunch of extern constant functions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm 32-bit and 64-bit WebAssembly backend-llvm The LLVM backend outputs an LLVM IR Module. backend-self-hosted bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase.
Projects
None yet
Development

No branches or pull requests

8 participants