-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
rustc_codegen_ssa: fix pre_link_args order #151165
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
base: main
Are you sure you want to change the base?
rustc_codegen_ssa: fix pre_link_args order #151165
Conversation
|
r? @nnethercote rustbot has assigned @nnethercote. Use |
| // FIXME: In practice built-in target specs use this for arbitrary order-independent options, | ||
| // introduce a target spec option for order-independent linker options and migrate built-in | ||
| // specs to it. | ||
| add_pre_link_args(cmd, sess, flavor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmd.export_symbols() has to be before any object files on some targets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also if you really need it before any arguments, this is still too late. get_linker already adds some args in some cases. In particular for Windows UWP it passes /LIBPATH (though there is a FIXME to move it out of get_linker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I understand, so, should I implement a new parameter to rustc named -Cbefore-link-args instead, put the result as the really first args, and patch the FIXME for get_linker ?
I believe sccache would auto-detect the tool to use and fallback to cc if it fails to detect. Looks like ccache doesn't do that. However you can create a symlink named gcc with ccache as target. So that may be an alternative to this PR. |
Indeed, creating a symlink, or a single executable fake linker running literally As I proposed in your review, creating a new param like Or either, just splitting the |
|
r? @bjorn3 |
|
I don't have time for reviewing this. @rustbot reroll |
Hi,
We had an issue while compiling the linux kernel with a composite linker command in Yocto when activating the rust parts of the kernel.
Kernel Makefiles run rustc with
-Clinker=$(HOSTCC)(scripts/Makefile.host, rust/Makefile)But,
HOSTCCcan be something like<my-wrapper-tool> <my-cc-as-arg>(in our yocto caseccache gcc), which is incompatible with the way rustc handles-Clinker, it is handled as aPathBuf, used directly as aCommandname, but theCommandconstructor doesn't accept arguments.We could split
HOSTCCtoSTART_HOSTCCandEND_HOSTCCto do something like-Clinker=$(START_HOSTCC) -Zpre-link-args='$(END_HOSTCC)', but a rustc limitation occurs.Indeed, some linker arguments given by target files (ex:
-m64from a target file) and from the functionexport_symbols(ex:--no-undefined-versionhere) where added to the command before thepre-link-args.Therefore, we could end up with a generated call like
<my-wrapper-tool> -m64 <my-cc-as-arg> ...(in our caseccache -m64 gcc ...), which is broken.This PR mitigates this issue sorting pre-link-args calls before target files pre-link-args, and before the call to
export_symbols, which allow to use any wrapper tool around the actual linker.Please make me know if I should rewrite it in another way (a new argument parameter ? directly split the PathBuf into arguments ?).
I could write some tests if needed, but I'm not sure about the best way to test it in the repository.
Thanks for reading, regards,
Alban