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

Webassembly linker does not export symbols #1570

Closed
josephg opened this Issue Sep 21, 2018 · 13 comments

Comments

Projects
None yet
4 participants
@josephg

josephg commented Sep 21, 2018

Given this zig program:

export fn add(a: i32, b: i32) i32 {
    return a + b;
}

Compiled with zig build-exe --release-small --target-arch wasm32 wasm.zig, the output doesn't export the function.

Run through wasm2wat, the exported output is:

(module
  (type (;0;) (func (param i32 i32) (result i32)))
  (func $add (type 0) (param i32 i32) (result i32)
    get_local 1
    get_local 0
    i32.add)
  (table (;0;) 1 1 anyfunc)
  (memory (;0;) 0))

... Which doesn't export the add function like it should:

$ node
> m = new WebAssembly.Module(fs.readFileSync('wasm'))
Module [WebAssembly.Module] {}
> i = new WebAssembly.Instance(m, {})
Instance [WebAssembly.Instance] {}
> i.exports.add(1,2)
TypeError: i.exports.add is not a function

Linking with wasm-ld from llvm 7 works fine, and produces this:

sephsmac:zig josephg$ zig build-obj --release-small --target-arch wasm32 wasm.zig 
sephsmac:zig josephg$ wasm-ld wasm.o -o xyz -O2 --no-entry
sephsmac:zig josephg$ wasm2wat xyz
(module
  (type (;0;) (func))
  (type (;1;) (func (param i32 i32) (result i32)))
  (func $__wasm_call_ctors (type 0))
  (func $add (type 1) (param i32 i32) (result i32)
    get_local 1
    get_local 0
    i32.add)
  (table (;0;) 1 1 anyfunc)
  (memory (;0;) 2)
  (global (;0;) (mut i32) (i32.const 66560))
  (global (;1;) i32 (i32.const 66560))
  (global (;2;) i32 (i32.const 1024))
  (export "memory" (memory 0))
  (export "__heap_base" (global 1))
  (export "__data_end" (global 2))
  (export "add" (func $add)))

... Which works just fine. (Notice the export "add" at the end)

@andrewrk andrewrk added this to the 0.4.0 milestone Sep 21, 2018

@andrewrk andrewrk added the bug label Sep 21, 2018

@andrewrk

This comment has been minimized.

Member

andrewrk commented Sep 21, 2018

Thanks for the report. LLVM just now finally merged my patch to make WebAssembly a normal (non-experimental) target. But they didn't do it before llvm 7 release. So zig 0.4.0 will have WebAssembly support without any hacks, and I'll make sure this is fixed by then.

@josephg

This comment has been minimized.

josephg commented Sep 21, 2018

If anyone comes across this looking to compile zig to wasm in the meantime, here's a rough guide to getting it working at present. At least on macos:

https://gist.github.com/josephg/873a21d4558fd69aeccea19c3df96672

@andrewrk

This comment has been minimized.

Member

andrewrk commented Sep 21, 2018

Thanks for doing that, that's really handy. It's also worth mentioning that both the windows and Linux static CI builds of zig come with the webassembly target enabled.

@bnoordhuis

This comment has been minimized.

Member

bnoordhuis commented Oct 1, 2018

We could fix this in zig by not passing --relocatable to lld but then either zig or the programmer needs to add a _start() entry point.

The entry point is exported through the module's exports and that might be undesirable if it's synthesized by zig.

@andrewrk

This comment has been minimized.

Member

andrewrk commented Oct 1, 2018

zig build-exe should not pass --relocatable to the linker. We have an established pattern for putting the start code in std/special/bootstrap.zig. You can see here we have the _start symbol for ELF:

nakedcc fn _start() noreturn {

We can have the webassembly equivalent which calls the user's main function. Likewise for zig build-lib I think relocatable should be off. Zig would generate an empty _start function, I suppose, since libraries don't have an entry point. zig build-lib --static and zig build-obj can have it on.

@BarabasGitHub

This comment has been minimized.

Contributor

BarabasGitHub commented Oct 1, 2018

@andrewrk

This comment has been minimized.

Member

andrewrk commented Oct 1, 2018

Sure, if it worked. But since what --relocatable empirically does is make dynamic libraries not work, then turning it off is what makes export do what export should do.

bnoordhuis added a commit to bnoordhuis/zig that referenced this issue Oct 2, 2018

fix build-exe for --target-arch wasm32 (ziglang#1570)
Pass --no-entry instead of --relocatable to lld. Both stop a reference
to the _start() entry point from being emitted but --relocatable also
prevents public symbols from being exported when creating an executable.
@bnoordhuis

This comment has been minimized.

Member

bnoordhuis commented Oct 2, 2018

After thinking about this some more, I'd say the best way forward is to replace --relocatable with --no-entry. I've opened #1619.

Emitting a _start() symbol arguably doesn't make sense in the context of WASM because binaries are really just libraries. There is no conventional entry point like there is with COFF or ELF.

andrewrk added a commit that referenced this issue Oct 2, 2018

Merge pull request #1619 from bnoordhuis/fix1570
fix build-exe for --target-arch wasm32 (#1570)
@josephg

This comment has been minimized.

josephg commented Oct 2, 2018

Yeah it’s really not clear to me what build-exe Is supposed to mean in the context of wasm, as opposed to build-lib. There’s no entry point either way.

@bnoordhuis

This comment has been minimized.

Member

bnoordhuis commented Oct 2, 2018

#1619 was merged so I'll go ahead and close this issue. Thanks for the bug report.

@bnoordhuis bnoordhuis closed this Oct 2, 2018

@andrewrk

This comment has been minimized.

Member

andrewrk commented Oct 2, 2018

Maybe build-exe for the webassembly target should be an error and it should suggest using build-lib?

@josephg

This comment has been minimized.

josephg commented Oct 2, 2018

Yeah - either that or build-exe could just alias build-lib on wasm. (Which might be what its doing now?)

@andrewrk

This comment has been minimized.

Member

andrewrk commented Oct 2, 2018

In general the difference between build-exe and build-lib is (1) the entry point code in std/special/bootstrap.zig and (2) linker flags.

For webassembly since we're not doing (1) and there is no difference in (2) it's the same thing. So I think it makes sense to make build-exe an error so that there is a canonical way to do things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment