-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
-Z linker-flavor #40018
-Z linker-flavor #40018
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
Yes, the Solaris linker is multi-arch. As long as it is provided well-formed objects it should be able to link them. With that said, I intend to evaluate what it would take to get lld working on Solaris later this year and teach it some Solaris-specific tricks. |
Oh, and btw, this is awesome as it will make it easier for us to teach rust how to use the Solaris linker directly. |
This kinda collides and duplicates my effort to refactor target specs altogether but since it is pretty similar to what I'm going for wrt linkers I guess it's fine. |
The target spec field needn't be required field as it can just default to gnu, just like pretty much everything else does already anyway |
/cc @dhduvall |
src/librustc_back/target/mod.rs
Outdated
"em" => LinkerFlavor::Em, | ||
"gnu" => LinkerFlavor::Gnu, | ||
"ld" => LinkerFlavor::Ld, | ||
"msvc" => LinkerFlavor::Msvc, |
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.
These same four strings have been repeated quite a few times throughout this PR, perhaps they could be centralized to one location though?
src/librustc_trans/back/linker.rs
Outdated
} | ||
} | ||
|
||
impl<'a> Linker for LdLinker<'a> { |
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.
Most of this seems duplicated from a definition above, would it be possible to share code instead of duplicating?
// Always enable NX protection when it is available | ||
"-Wl,-z,noexecstack".to_string(), | ||
]); | ||
|
||
TargetOptions { | ||
dynamic_linking: true, | ||
executables: true, | ||
target_family: Some("unix".to_string()), | ||
linker_is_gnu: true, |
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.
Is an option like this still needed?
This all sounds like a great idea to me, thanks @japaric! I think it's some good internal refactoring regardless as well. As a minor bikeshed, can we perhaps rename |
Done.
I got some deduplication by making
Yes.
Done |
Ah yeah for deduplication of gcc/ld I was thinking we could just strip Also could the "did you mean ..." be deduplicated as well? Just trying to contain the number of places we have to update when we inevitably add a new flavor of linker :) Also yeah that's true about |
It also looks like Travis may be failing? |
ping @japaric |
squeaky wheel @japaric |
Closing due to inactivity, but feel free to resubmit with a rebase! |
Rebased.
Done. Merged GccLinker and LdLinker into a single GccLinker that decides whether to add the -Wl based on the value of a boolean is_ld field.
Done.
That rmake test should be fixed now. |
Now that we have the unstable book, should I document I forgot to ask in my previous comment: r? @alexcrichton |
☔ The latest upstream changes (presumably #40805) made this pull request unmergeable. Please resolve the merge conflicts. |
-Z linker-flavor (Please read the commit message first) This PR is an alternative to #36120 (internal lld linker). The main goal of this PR is to make it *possible* to use LLD as a linker to allow out of tree experimentation. Now that LLD is going to be shipped with LLVM 4.0, it should become easier to get a hold of LLD (hopefully, it will be packaged by Linux distros soon). Since LLD is a multiarch linker, it has the potential to make cross compilation easier (less tools need to be installed). Supposedly, LLD is also faster than the gold linker so LLD may improve build times where link times are significant (e.g. 100% incremental compilation reuse). The place where LLD shines is at linking Rust programs that don't depend on system libraries. For example, here's how you would link a bare metal ARM Cortex-M program: ``` $ xargo rustc --target thumbv7m-none-eabi -- -Z linker-flavor=ld -C linker=ld.lld -Z print-link-args "ld.lld" \ "-L" \ "$XARGO_HOME/lib/rustlib/thumbv7m-none-eabi/lib" \ "$PWD/target/thumbv7m-none-eabi/debug/deps/app-de1f86df314ad68c.0.o" \ "-o" \ "$PWD/target/thumbv7m-none-eabi/debug/deps/app-de1f86df314ad68c" \ "--gc-sections" \ "-L" \ "$PWD/target/thumbv7m-none-eabi/debug/deps" \ "-L" \ "$PWD/target/debug/deps" \ "-L" \ "$XARGO_HOME/lib/rustlib/thumbv7m-none-eabi/lib" \ "-Bstatic" \ "-Bdynamic" \ "$XARGO_HOME/lib/rustlib/thumbv7m-none-eabi/lib/libcore-11670d2bd4951fa7.rlib" $ file target/thumbv7m-none-eabi/debug/app app: ELF 32-bit LSB executable, ARM, EABI5 version 1 (SYSV), statically linked, not stripped, with debug_info ``` This doesn't require installing the `arm-none-eabi-gcc` toolchain. Even cooler (but I'm biased) is that you can link Rust programs that use [`steed`] (`steed` is a `std` re-implementation free of C dependencies for Linux systems) instead of `std` for a bunch of different architectures without having to install a single cross toolchain. [`steed`]: https://github.com/japaric/steed ``` $ xargo rustc --target aarch64-unknown-linux-steed --example hello --release -- -Z print-link-args "ld.lld" \ "-L" \ "$XARGO_HOME/lib/rustlib/aarch64-unknown-linux-steed/lib" \ "$PWD/target/aarch64-unknown-linux-steed/release/examples/hello-80c130ad884c0f8f.0.o" \ "-o" \ "$PWD/target/aarch64-unknown-linux-steed/release/examples/hello-80c130ad884c0f8f" \ "--gc-sections" \ "-L" \ "$PWD/target/aarch64-unknown-linux-steed/release/deps" \ "-L" \ "$PWD/target/release/deps" \ "-L" \ "$XARGO_HOME/lib/rustlib/aarch64-unknown-linux-steed/lib" \ "-Bstatic" \ "-Bdynamic" \ "/tmp/rustc.lAybk9Ltx93Q/libcompiler_builtins-589aede02de78434.rlib" $ file target/aarch64-unknown-linux-steed/release/examples/hello hello: ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), statically linked, not stripped, with debug_info ``` All these targets (architectures) worked with LLD: - [aarch64-unknown-linux-steed](https://github.com/japaric/steed/blob/lld/docker/aarch64-unknown-linux-steed.json) - [arm-unknown-linux-steedeabi](https://github.com/japaric/steed/blob/lld/docker/arm-unknown-linux-steedeabi.json) - [arm-unknown-linux-steedeabihf](https://github.com/japaric/steed/blob/lld/docker/arm-unknown-linux-steedeabihf.json) - [armv7-unknown-linux-steedeabihf](https://github.com/japaric/steed/blob/lld/docker/armv7-unknown-linux-steedeabihf.json) - [i686-unknown-linux-steed](https://github.com/japaric/steed/blob/lld/docker/i686-unknown-linux-steed.json) - [mips-unknown-linux-steed](https://github.com/japaric/steed/blob/lld/docker/mips-unknown-linux-steed.json) - [mipsel-unknown-linux-steed](https://github.com/japaric/steed/blob/lld/docker/mipsel-unknown-linux-steed.json) - [powerpc-unknown-linux-steed](https://github.com/japaric/steed/blob/lld/docker/powerpc-unknown-linux-steed.json) - [powerpc64-unknown-linux-steed](https://github.com/japaric/steed/blob/lld/docker/powerpc64-unknown-linux-steed.json) - [x86_64-unknown-linux-steed](https://github.com/japaric/steed/blob/lld/docker/x86_64-unknown-linux-steed.json) --- The case where lld is unergonomic is linking binaries that depend on system libraries. Like "Hello, world" for `x86_64-unknown-linux-gnu`. Because you have to pass as linker arguments: the path to the startup objects, the path to the dynamic linker and the library search paths. And all those are system specific so they can't be encoded in the target itself. ``` $ cargo \ rustc \ --release \ -- \ -C \ linker=ld.lld \ -Z \ linker-flavor=ld \ -C \ link-args='-dynamic-linker /lib64/ld-linux-x86-64.so.2 -L/usr/lib -L/usr/lib/gcc/x86_64-pc-linux-gnu/6.3.1 /usr/lib/Scrt1.o /usr/lib/crti.o /usr/lib/gcc/x86_64-pc-linux-gnu/6.3.1/crtbeginS.o /usr/lib/gcc/x86_64-pc-linux-gnu/6.3.1/crtendS.o /usr/lib/crtn.o' ``` --- Another case where `-Z linker-flavor` may come in handy is directly calling Solaris' linker which is also a multiarch linker (or so I have heard). cc @binarycrusader cc @alexcrichton Heads up: [breaking-change] due to changes in the target specification format.
☀️ Test successful - status-appveyor, status-travis |
Looks like the LinkerFlavor change introduced in rust-lang#40018 accidentally uses GCC for the WebAssembly target, causing Rust to never actually pass the post link args to emscripten. This then causes the code to be compiled as asm.js instead of WebAssembly, because the Binaryen tools never run due to the missing linker argument.
Compile WASM as WASM instead of asm.js Looks like the LinkerFlavor change introduced in rust-lang#40018 accidentally uses GCC for the WebAssembly target, causing Rust to never actually pass the post link args to emscripten. This then causes the code to be compiled as asm.js instead of WebAssembly, because the Binaryen tools never run due to the missing linker argument.
This field is now required; see rust-lang/rust#40018
In latest rustc,target specification json file is needed "linker-flavor" for this issue: rust-lang/rust#40018
In latest rustc,target specification json file is needed "linker-flavor" for this issue: rust-lang/rust#40018
In latest rustc,target specification json file is needed "linker-flavor" for this issue: rust-lang/rust#40018
In latest rustc,target specification json file is needed "linker-flavor" for this issue: rust-lang/rust#40018
In latest rustc,target specification json file is needed "linker-flavor" for this issue: rust-lang/rust#40018
In latest rustc target specification json file is needed "linker-flavor" for this issue: rust-lang/rust#40018
self.cmd.arg("-Wl,--whole-archive") | ||
.arg("-l").arg(lib) | ||
.arg("-Wl,--no-whole-archive"); | ||
self.linker_arg("--whole-archive").cmd.arg("-l").arg(lib); |
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.
FYI: this style (e.g. "-l" "System"
) isn't supported by ld64 on darwin - that linker requires the style "-lSystem"
. I'm testing a PR that changes this.
(Please read the commit message first)
This PR is an alternative to #36120 (internal lld linker). The
main goal of this PR is to make it possible to use LLD as a linker to allow
out of tree experimentation. Now that LLD is going to be shipped with LLVM 4.0,
it should become easier to get a hold of LLD (hopefully, it will be packaged by
Linux distros soon).
Since LLD is a multiarch linker, it has the potential to make cross compilation
easier (less tools need to be installed). Supposedly, LLD is also faster than
the gold linker so LLD may improve build times where link times are significant
(e.g. 100% incremental compilation reuse).
The place where LLD shines is at linking Rust programs that don't depend on
system libraries. For example, here's how you would link a bare metal ARM
Cortex-M program:
This doesn't require installing the
arm-none-eabi-gcc
toolchain.Even cooler (but I'm biased) is that you can link Rust programs that use
steed
(steed
is astd
re-implementation free of C dependencies for Linuxsystems) instead of
std
for a bunch of different architectures without havingto install a single cross toolchain.
All these targets (architectures) worked with LLD:
The case where lld is unergonomic is linking binaries that depend on system
libraries. Like "Hello, world" for
x86_64-unknown-linux-gnu
. Because you haveto pass as linker arguments: the path to the startup objects, the path to the
dynamic linker and the library search paths. And all those are system specific
so they can't be encoded in the target itself.
Another case where
-Z linker-flavor
may come in handy is directly callingSolaris' linker which is also a multiarch linker (or so I have heard). cc
@binarycrusader
cc @alexcrichton
Heads up: [breaking-change] due to changes in the target specification format.