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

Static variables are not exported in wasm #67453

Closed
MaikKlein opened this issue Dec 20, 2019 · 4 comments · Fixed by #67975
Closed

Static variables are not exported in wasm #67453

MaikKlein opened this issue Dec 20, 2019 · 4 comments · Fixed by #67975
Labels
A-linkage Area: linking into static, shared libraries and binaries O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@MaikKlein
Copy link
Contributor

MaikKlein commented Dec 20, 2019

Repro https://github.com/EmbarkStudios/missing-symbols rustc 1.40.0 (73528e339 2019-12-16)

// lib.rs
#[no_mangle]
#[used]
pub static FOO: u64 = 4242;

cargo build --release --target wasm32-unknown-unknown, the resulting wasm file doesn't have the symbol FOO.

(module
  (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 "__data_end" (global 1))
  (export "__heap_base" (global 2)))

But if we compile with -C link-dead-code, the symbol is correctly exported in the wasm file.

RUSTFLAGS="-C link-dead-code" cargo build --release --target wasm32-unknown-unknown

(module
  (table (;0;) 1 1 funcref)
  (memory (;0;) 17)
  (global (;0;) (mut i32) (i32.const 1048576))
  (global (;1;) i32 (i32.const 1048584))
  (global (;2;) i32 (i32.const 1048584))
  (global (;3;) i32 (i32.const 1048576))
  (export "memory" (memory 0))
  (export "__data_end" (global 1))
  (export "__heap_base" (global 2))
  (export "FOO" (global 3))
  (data (;0;) (i32.const 1048576) "\92\10\00\00\00\00\00\00"))

Expected behavior: Public static variables should get exported by default.

We believe this is a regression from 1.39.0.

rustup override set 1.39.0
cargo build --release --target wasm32-unknown-unknown

Does correctly export static variables.

@Centril Centril added A-linkage Area: linking into static, shared libraries and binaries O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 21, 2019
@Centril
Copy link
Contributor

Centril commented Dec 21, 2019

cc @japaric @alexcrichton

@alexcrichton
Copy link
Member

Symbols for wasm are exported here and it looks like no --export flag is passed for FOO, which is the bug. It looks like this may trace back to exported_symbols_provider_local not including statics, but that's just a guess. I don't personally have time to deeply investigate this right now.

@MaikKlein
Copy link
Contributor Author

After a some investigation:

  • link-dead-code disables --gc-section, which means all the symbols are exported. This is why FOO appears.
  • If --gc-section is set, symbols have to be exported --export NAME, which is not the case for FOO as @alexcrichton has stated above
  • The reason for this is SymbolExportLevel::is_below_threshold is false. FOO is a rust symbol and we export to a c lib.

Statics are explicitly marked as SymbolExportLevel::Rust.

    if is_extern && !std_internal {
        let target = &tcx.sess.target.target.llvm_target;
        // WebAssembly cannot export data symbols, so reduce their export level
        if target.contains("wasm32") || target.contains("emscripten") {
            if let Some(Node::Item(&hir::Item { kind: hir::ItemKind::Static(..), .. })) =
                tcx.hir().get_if_local(sym_def_id)
            {
                return SymbolExportLevel::Rust;
            }
        }

        SymbolExportLevel::C
    } else {
        SymbolExportLevel::Rust
    }

It is unclear to me how this should be handled. Maybe statics should be exported with SymbolExportLevel::C if the static is a number?

I'll make a PR for it, that might be a better place to talk about implementation details.

@alexcrichton
Copy link
Member

Oh good find! I'm not sure about emscripten, but wasm has changed in the meantime so that it can export statics, so the wasm case there I believe can be removed.

Centril added a commit to Centril/rust that referenced this issue Jan 9, 2020
…r=alexcrichton

Export public scalar statics in wasm

Fixes rust-lang#67453

I am not sure which export level statics should get when exporting them in wasm. This small change fixes the issue that I had, but this might not be the correct way to implement this.
Centril added a commit to Centril/rust that referenced this issue Jan 9, 2020
…r=alexcrichton

Export public scalar statics in wasm

Fixes rust-lang#67453

I am not sure which export level statics should get when exporting them in wasm. This small change fixes the issue that I had, but this might not be the correct way to implement this.
@bors bors closed this as completed in 8de73bc Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants