rustc_codegen_ssa: fix pre_link_args order

[alban-moizan-smile]

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, HOSTCC can be something like <my-wrapper-tool> <my-cc-as-arg> (in our yocto case ccache gcc), which is incompatible with the way rustc handles -Clinker, it is handled as a PathBuf, used directly as a Command name, but the Command constructor doesn't accept arguments.

We could split HOSTCC to START_HOSTCC and END_HOSTCC to 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: -m64 from a target file) and from the function export_symbols (ex: --no-undefined-version here) where added to the command before the pre-link-args.

Therefore, we could end up with a generated call like <my-wrapper-tool> -m64 <my-cc-as-arg> ... (in our case ccache -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

[rustbot]

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[bjorn3]

cmd.export_symbols() has to be before any object files on some targets.

[bjorn3]

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.

[alban-moizan-smile]

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 ?

[bjorn3]

But, HOSTCC can be something like (in our yocto case ccache gcc)

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.

[alban-moizan-smile]

But, HOSTCC can be something like (in our yocto case ccache gcc)

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 ccache gcc before would work, but it becomes less clear what is actually running here, and it only works with ccache or sccache, if you want to do more esoteric things like valgrind gcc, it's broken.

As I proposed in your review, creating a new param like -Cbefore-link-args (or named diffrently) could mitigate the issue while not changing the current ordering.

Or either, just splitting the -Clinker into a Vec and putting the first value in the Command::new and others in args ?

[nnethercote]

r? @bjorn3

[bjorn3]

I don't have time for reviewing this.

@rustbot reroll

[mati865]

I won't have time for it either.

@rustbot reroll

[BoxyUwU]

r? compiler

[eholk]

Based on https://forge.rust-lang.org/compiler/proposals-and-stabilization.html#compiler-flags, it sounds like the next step here would be to rework this as a -Z option and then we can merge this PR. If using nightly is okay for you, this should be enough. Otherwise, I'd suggest opening an MCP in parallel to get this promoted to a -C option.

[wesleywiser]

I think this is waiting on the author to resolve the review comments in this thread: #151165 (comment)

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[JohnCSimon]

@alban-moizan-smile

Ping from triage: - can you post your status on this PR?
I'll be closing because of inactivity otherwise. Thanks