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

Functions with extermely long names can cause wasm-bindgen to fail to locate the main function #3362

Closed
sapphire-arches opened this issue Mar 20, 2023 · 2 comments · Fixed by #3364
Labels

Comments

@sapphire-arches
Copy link

sapphire-arches commented Mar 20, 2023

Describe the Bug

When a WASM module contains a function with an extremely long mangled name (>100_000 characters, as can be generated by rust-lang/rust#109363) wasmparser will choke on the names custom section. This means that at the walrus/wasm-bindgen layer any functions in the module after the problematically named function will not have names assigned. This is particularly deleterious if main is late, as it means wasm-bindgen won't emit the main glue (__wbindgen_start and friends).

Steps to Reproduce

  1. Use the example code from Exponential blowup in core::drop_in_place symbol size for builder-pattern structures that chain closures rust-lang/rust#109363, or clone https://github.com/bobtwinkles/wasm-bindgen-bug/
  2. Build that code with the module name bad or baad and rustc 1.66 - 1.70 (other versions not tested). The module name good happens to work but it's up to the order rustc chooses to emit the code, I'll chalk this one up to nominative determinism.

Expected Behavior

wasm-bindgen should maybe complain louder that it has failed to load all function names? I'm not really sure what can be done about this at the wasm-bindgen layer to be honest, I'm mostly just filing this bug here because I'm not sure what layer of the stack is really at fault here and want somewhere to anchor the discussion.

Actual Behavior

wasm-bindgen silently fails to generate main glue.

Additional Context

I discovered this while working on a Sycamore app, in particular the sycamore::builder::ElementBuilder type suffers from the exponential drop glue function symbol size problem mentioned in the Rust bug above.

@Liamolucko
Copy link
Collaborator

Huh, that's strange. The limit on function name lengths in wasmparser is intentional:

https://github.com/bytecodealliance/wasm-tools/blob/7108f5530dda9a07f5087ce4d013730b432438bc/crates/wasmparser/src/limits.rs#L26

But I'm not sure why it's getting silently ignored.

Liamolucko added a commit to Liamolucko/wasm-bindgen that referenced this issue Mar 22, 2023
Fixes rustwasm#3362

In some cases, the name section can fail to be parsed because one of the names is too long, which then causes the main function to not be detected because we don't know what any functions' names are.

However, `main` is also exported with the name `main`, so we can look for an export named `main` instead to avoid relying on the name section.
@sapphire-arches
Copy link
Author

walrus treats failure to parse the name section as nonfatal: https://github.com/rustwasm/walrus/blob/9d6c9de432d6a97478dc76ebdf18aed51584c3af/src/module/mod.rs#L217-L231

so wasm-bindgen thinks all the names have been parsed successfully when they have not. Running wasm-bindgen with RUSTLOG=warn or greater will print that warning on the offending modules.

alexcrichton pushed a commit that referenced this issue Mar 24, 2023
Fixes #3362

In some cases, the name section can fail to be parsed because one of the names is too long, which then causes the main function to not be detected because we don't know what any functions' names are.

However, `main` is also exported with the name `main`, so we can look for an export named `main` instead to avoid relying on the name section.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants