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

Experiment: Try making all compiler-builtins intrinsics weak symbols #113822

Closed
wants to merge 3 commits into from

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Jul 18, 2023

This is an experiment to try making all compiler-builtins symbols weak by default. This should solve issues like rust-lang/compiler-builtins#353 and rust-lang/compiler-builtins#420 where our symbols conflict with those from libgcc/libclang_rt.

r? @ghost

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jul 18, 2023
@Amanieu Amanieu marked this pull request as draft July 18, 2023 11:52
@rust-log-analyzer

This comment has been minimized.

@Amanieu
Copy link
Member Author

Amanieu commented Aug 1, 2023

The issue on Windows seems to be that weak symbols can be defined, but not dllexport'ed. This causes problems when building the standard library as a DLL since compiler_builtins is placed in it, and the rustc_driver DLL imports the compiler_builtins functions from it.

@mati865
Copy link
Contributor

mati865 commented Aug 8, 2023

Forwarding from rust-lang/compiler-builtins#546 (comment)

Weak symbols on Windows with ld.bfd are a can of worms.
Generally you can expect them to rather work as long as you are combining static archives with weak symbols into one final binary/library. Trying to do the same with shared libraries might work for simplest cases but generally think of it as broken.
You may have more luck with __declspec(selectany) but it will use more COMDATs which Windows limits to 2^16.

LLD is has working weak symbols (in both static and shared cases) but we cannot use it because of another Binutils bug import libraries created by LLD break symbol table with Binutils <2.40.

@Dylan-DPC Dylan-DPC added S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 22, 2023
@bors
Copy link
Contributor

bors commented Oct 17, 2023

☔ The latest upstream changes (presumably #116196) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 17, 2023
@Dylan-DPC
Copy link
Member

Closing this as it was an experiment

@Dylan-DPC Dylan-DPC closed this Nov 4, 2023
@Dylan-DPC Dylan-DPC removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants