Skip to content

wasm: Explicitly export all symbols with LLD #54766

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

Merged
merged 1 commit into from
Oct 6, 2018

Conversation

alexcrichton
Copy link
Member

This commit fixes an oddity on the wasm target where LTO can produce
working executables but plain old optimizations doesn't. The compiler
already knows what set of symbols it would like to export, but LLD only
discovers this list transitively through symbol visibilities. LLD may
not, however, always find all the symbols that we'd like to export.

For example if you depend on an rlib with a #[no_mangle] symbol, then
if you don't actually use anything from the rlib then the symbol won't
appear in the final artifact! It will appear, however, with LTO. This
commit attempts to rectify this situation by ensuring that all symbols
rustc would otherwise preserve through LTO are also preserved through
the linking process with LLD by default.

This commit fixes an oddity on the wasm target where LTO can produce
working executables but plain old optimizations doesn't. The compiler
already knows what set of symbols it would like to export, but LLD only
discovers this list transitively through symbol visibilities. LLD may
not, however, always find all the symbols that we'd like to export.

For example if you depend on an rlib with a `#[no_mangle]` symbol, then
if you don't actually use anything from the rlib then the symbol won't
appear in the final artifact! It will appear, however, with LTO. This
commit attempts to rectify this situation by ensuring that all symbols
rustc would otherwise preserve through LTO are also preserved through
the linking process with LLD by default.
@rust-highfive
Copy link
Contributor

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 2, 2018
@alexcrichton
Copy link
Member Author

r? @michaelwoerister

@michaelwoerister
Copy link
Member

Looks good to me! In the long run this should probably be implemented via a linker script like for the other linkers, so as not to run into commandline length issues. I'll let you decide if you want to do that in this PR or just r=me.

@alexcrichton
Copy link
Member Author

I agree! It looks though like LLD doesn't currently have an option for that for the wasm linker, but I'll file an issue.

@bors: r=michaelwoerister

@bors
Copy link
Collaborator

bors commented Oct 4, 2018

📌 Commit c7f4f3a has been approved by michaelwoerister

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 4, 2018
@alexcrichton
Copy link
Member Author

Hm actually, that may not be necessary. We've already got automatic logic to fall back to using @-files if we blow the command line length limit, and that's effectively the solution we'd have for this anyway because there's no dedicated option for "use this file to list all exported symbols", so in that sense I think we're only problematic here if we frequently fall back to using an @-file after blowing the system limits, which may cause links to be slower than they would otherwise need to be

@bors
Copy link
Collaborator

bors commented Oct 6, 2018

⌛ Testing commit c7f4f3a with merge b4d9835...

bors added a commit that referenced this pull request Oct 6, 2018
wasm: Explicitly export all symbols with LLD

This commit fixes an oddity on the wasm target where LTO can produce
working executables but plain old optimizations doesn't. The compiler
already knows what set of symbols it would like to export, but LLD only
discovers this list transitively through symbol visibilities. LLD may
not, however, always find all the symbols that we'd like to export.

For example if you depend on an rlib with a `#[no_mangle]` symbol, then
if you don't actually use anything from the rlib then the symbol won't
appear in the final artifact! It will appear, however, with LTO. This
commit attempts to rectify this situation by ensuring that all symbols
rustc would otherwise preserve through LTO are also preserved through
the linking process with LLD by default.
@bors
Copy link
Collaborator

bors commented Oct 6, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing b4d9835 to master...

@bors bors merged commit c7f4f3a into rust-lang:master Oct 6, 2018
@alexcrichton alexcrichton deleted the wasm-all-symbols branch October 8, 2018 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants