Export WASM table by default

[overdrivenpotato]

This allows loading a rust-generated .wasm binary in a host and using the exported table much like the memory export.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[nikomatsakis]

r? @alexcrichton

[alexcrichton]

Thanks for the PR @overdrivenpotato! For these sorts of flags to LLD we tend to just want to make sure that it can be turned off as well. Is there something that can be passed via -C link-args to disable this behavior?

[overdrivenpotato]

You can use -C link-arg=--import-table to use an imported table instead of generating an exported table. However, there is no way to revert to the old behavior (a private table). With this in mind, marking the table exported should not be an issue in any WebAssembly binary as the behavior of the table stays the same. It is simply accessible from the host now. In practice, this avoids the need for a custom trampolining function to handle callbacks between Rust and the host environment, as we can instead use the native WebAssembly table feature.

[alexcrichton]

Ah ok interesting! In that case we may not want to have this turned on by default (as it can't be turned off). While true that it doesn't affect the behavior of the binary it does make it a bit larger because the table section has to get exported.

Note that it's also pretty easy to export the table section in an existing wasm binary, this is what wasm-bindgen does today to implement callbacks for rust modules. (despite the table not by default being exported it ensures that it's flagged to get exported)

[overdrivenpotato]

If the only concern is binary size, it's worth noting that the --export-table flag will only add 28 bytes to the final binary. Couldn't we then consider removing this table export in a build step to be an edge case optimization, rather than having to patch it back in for almost all use cases?

It's also worth noting that the memory export is exported by default as well and I don't believe there is a linker flag to disable it (correct me if I'm wrong). Enabling this flag by default should reduce the friction required for users, while allowing those who really need the few extra bytes to manually remove this export (and potentially the memory export as well) in a custom build step.

Test case:

#![no_std]#![feature(lang_items, panic_implementation, core_intrinsics)]#[panic_implementation]pubfnpanic_fmt(_fmt:&core::panic::PanicInfo) -> ! {unsafe{ core::intrinsics::abort()}}#[lang = "eh_personality"]pubfneh_personality(){}extern{fntest(f:externfn() -> i32);}#[lang = "start"]pubfnmain(){externfncb() -> i32{123}unsafe{test(cb);}}

With --export-table - 270 bytes:

$ RUSTFLAGS="-C link-arg=--export-table" cargo build --target wasm32-unknown-unknown --release
Compiling test_lld v0.1.0 (file:///private/tmp/test_lld)
Finished release [optimized] target(s) in 0.18s
$ ls -alh target/wasm32-unknown-unknown/release/test_lld.wasm
-rwxr-xr-x 2 marko wheel 270B 13 Aug 21:30 target/wasm32-unknown-unknown/release/test_lld.wasm

Without --export-table - 242 bytes:

$ cargo build --target wasm32-unknown-unknown --release
Compiling test_lld v0.1.0 (file:///private/tmp/test_lld)
Finished release [optimized] target(s) in 0.27s
$ ls -alh target/wasm32-unknown-unknown/release/test_lld.wasm
-rwxr-xr-x 2 marko wheel 242B 13 Aug 21:29 target/wasm32-unknown-unknown/release/test_lld.wasm

[alexcrichton]

Ah good points! While it seems small people are definitely counting bytes when looking at Rust "hello world" use cases on wasm, but this seems like a handful of bytes that may not matter too much.

@fitzgen can you think of any reason why this might cause problems? I'd be tempted to merge this myself now!

[fitzgen]

Is most of the bytes the name of the exported table?

I'm fine with this so long as the bloat is not proportional to the size of the wasm binary, which I don't expect it to be, or else things have gone really wrong :-p

[alexcrichton]

@bors: r+

Ok! Sounds good!

[bors]

📌 Commit c7a39b1 has been approved by alexcrichton

[overdrivenpotato]

LLVM exports the table as indirect_function_table, that’s most of the additional bloat. There’s also a few extra bytes to add an export entry. It will always be 28-29 bytes (possibly one extra with LEB128), no matter the size of the actual Rust project.

[bors]

⌛ Testing commit c7a39b1 with merge 0f4b498...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 0f4b498 to master...