rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.71k stars 12.5k forks source link

Clang linker script support - incorrect argument #127183

Open boozook opened 2 months ago

boozook commented 2 months ago

Using clang as linker with any custom target (spec in json) that contains linker-script (field link-script) produces invocation like

# clang or cc, or gcc
"clang" "--script" "/path/to/link-script/exported/from/my-target-json"

But clang doesn't support --script and returns error clang: error: unsupported option '--script'. Instead of that, according documentation, we just need to pass it with -T, e.g.: -T/path/to/link-script.ld.

How to reproduce:

  1. Prepare spec json file for your custom target. It could be anything, just one thing matter - add link-script field. Something like this:

    {
      "llvm-target": "Your-HOST-target",
      "link-script": "ENTRY(main)",
      "linker": "clang"
    }
  2. Compile anything targeting to your custom target, e.g.: --target=./your-target.json

Expected result: Clang should be invoked with -T<link-script> instead of --script <link-script>.

I can reproduce with target-spec with explicitly set linker as clang, gcc, arm-none-eabi-gcc and cc.

Meta

rustc --version --verbose:

rustc 1.80.0-nightly (72fdf913c 2024-06-05)
binary: rustc
commit-hash: 72fdf913c53dd0e75313ba83e4aa80df3f6e2871
commit-date: 2024-06-05
host: aarch64-apple-darwin
release: 1.80.0-nightly
LLVM version: 18.1.6
strottos commented 2 months ago

Looks like it's in rustc_codegen_ssa/src/back/link.rs in add_link_script. As far as I can tell (and I'm no expert on this code but got curious) if link_script is set on the target it'll add a --script <path> to the linker args before running it. There's no evidence that it currently supports using clang as a linker and no evidence it can support any other argument for this; the supported variants are listed in rustc_target/src/spec/mod.rs against the enum LinkerFlavor, main ones being Gnu/Msvc/Darwin/Wasm (and some other more obscure ones).

One option might be to add a linker flavor Llvm(Cc) (where Cc is true if clang and false if lld) but it doesn't look like this is currently supported. Suspect this is a bit of work as the gnu one links to GCC libs and the like, guessing you'd need to figure out which equivalent LLVM libs but could be wrong. Probably needs triaging by someone who knows this stuff a bit better than me.

boozook commented 2 months ago

As far as I know, clang as linker is supported and used with flavors gnu-cc, darwin-cc, maybe unix-cc and some more.

strottos commented 2 months ago

I think it's supported in the sense that clang is largely compatible with gcc where command line options are concerned so can be used as a drop in linker. What's odd here is that, as you say, it passes a --script <script_file> with a temporary file it's just created where even gcc seems to only accept a -T parameter unless I'm mistaken. The fix could be a very simple --script -> -T as it seems to only support the gnu stuff specifically, e.g: https://github.com/rust-lang/rust/blob/master/compiler/rustc_codegen_ssa/src/back/link.rs#L1900

I wonder if just nobody else has used this and this is just an easily fixable bug, or maybe there's some variants of gcc or something else that accept --script (and heaven forbid only --script and not -T)?

strottos commented 2 months ago

Can't see any evidence gcc supports --script in the official GNU docs: https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html#index-T

strottos commented 2 months ago

Hmm, lld has the --script or -T option:


  --script=<value>        Read linker script
  -T <value>              Alias for --script```
strottos commented 2 months ago

As does ld (all on Linux, not sure about mac/windows):


  -T FILE, --script FILE      Read linker script```
boozook commented 2 months ago

Yup, both -T and --script is for both GNU- linkers - lldas well as ld.

But on macOs that is unknown option for ld from Xcodeutils (1, 2).

Also, I see the option in the code, but not --script there.


Also there is important difference:

strottos commented 2 months ago

Yeah I think the answer here is to move to a -T argument as everything I've looked at supports that. It's only a problem if something supports only --script and I haven't found anything that is like that. Again, someone with more experience may disagree.

I think for the second problem (another issue and maybe needs looking at in another ticket) you might have to add a clang specific flavor which currently doesn't exist. Then you could do this easily if you wanted to go down that route.

boozook commented 2 months ago

@strottos I could do it, but

  1. we need some research to determine what tools supports separate -T <script> and/or merged -T<script>
  2. as you say above need to know what tools supports only --script, if such exist
  3. could be great to have some integration tests with various tools, various versions on various OSs.

Maybe could be good to have something like cc::is_flag_supported() to get to know in runtime.

For now I see it as following:

strottos commented 2 months ago

Agree with most of that, only bit I'm not sure about is the is_flag_supported() suggestion, not sure you could do that easily. Don't think it knows what end program you're passing it and it just assumes it matches the GNU system. You could make it spawn it with a --version and check certain variants but (a) that's messy, (b) adds a not completely trivial performance hit and (c) hard to be exhaustive potentially. I think adding in more flavors is better personally.

I actually really think the "change this to just -T" is a good one as all the GNU tools seem to support it and they're the only thing that is allowed because of this line.

All this being said, bowing out now in case someone with more expertise here wants to take this.

strottos commented 2 months ago

FWIW, I tried changing it to -T and it worked fine on my machine after breaking in the same way with --script :)

Had to do a bit of playing around to the targets right, essentially outputting the target specs with this: rustc +nightly -Z unstable-options --print target-spec-json removing the is-builtin option and adding the "link-script": "ENTRY(main)" option seemed to reproduce for me on a Gentoo Linux machine.