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

[RFC] [breaking-change] change the default linker of the thumbv*m-none-eabi* targets #160

Closed
japaric opened this issue Aug 10, 2018 · 13 comments · Fixed by rust-lang/rust#53648
Labels
decision-accepted We voted on this proposal and accepted it RFC

Comments

@japaric
Copy link
Member

japaric commented Aug 10, 2018

Summary

Change the default linker of the Thumb targets from arm-none-eabi-gcc to rustc's LLD (rust-lld).
The following targets would be affected:

  • thumbv6m-none-eabi
  • thumbv7m-none-eabi
  • thumbv7em-none-eabi
  • thumbv7em-none-eabihf

Background

The Thumb targets were added to the compiler well over a year ago. Back then we didn't know if we
were going to ship LLD with rustc so arm-none-eabi-gcc was selected as the default linker.

Fast forward to today LLD ships with rustc on tier 1 platforms (x86_64 Linux, macOS and Windows),
and rustc supports GCC-style, LLD-style and LD-style ELF linkers (the linker can be chosen using -C linker and -Z linker-flavor).

Motivation

We want to simplify the development setup for embedded developers that will be using the stable
channel. Right now the linker can't be changed on stable because the -Z linker-flavor flag is
unstable. Thus the default of GCC forces ARM Cortex-M developers to install arm-none-eabi-gcc
before they can build even the smallest embedded Rust program.

With the change proposed in this RFC those developers would be able to build and link embedded Rust
programs without having to install a C toolchain.

Detailed design

On the compiler side the change is simple so this section will focus on how end users will have to
deal with the breaking change.

Dealing with breakage

This change will be breaking for all users that are customizing the linking process with any of
these flags: -C link-arg, -C link-args, -Z pre-link-arg and -Z pre-link-args --
unfortunately this is the majority of users.

The user has two ways to fix their code:

They can switch back to GCC. Pass the flags -C linker=arm-none-eabi-gcc -Z linker-flavor=gcc to
the compiler and you'll be using GCC again. Note that once rust-lang/rust#52101 the unstable -Z linker-flavor flag won't be required.

They can change their custom linker flags to make them compatible with LLD. rust-lld will be using
the LD linker flavor which is incompatible with the current default linker flavor of GCC. The
changes will mostly consist of removing the -Wl, prefix that GCC uses. For example, -C link-arg=-Wl,-Tlink.x would become -C link-arg=-Tlink.x.

Rust stability promise

"This is a breaking change. Isn't this against Rust stability principle of "code never breaks when
moving to a newer stable release"?"

This is technically a breaking change but it will only affect nightly users because the linker is
only used to build binary crates, cdylibs crates and the like; all those types of crate require the
still unstable #[panic_implementation] feature. As long as this change is done before or at the
same time #[panic_implementation] is stabilized it won't break Rust stability promise.

Alternatives

We can leave things as they are. It won't be possible to use rust-lld on stable until either -Z linker-flavor is stabilized (-C linker=rust-lld -Z linker-flavor=ld.lld), or rust-lang/rust#52101
lands and the proposed idea of a ld_flavor field for target specifications (see
rust-lang/rust#52101 (comment)) is implemented.

Instead of changing the default linker we could stabilize -Z linker-flavor then it would be
possible to switch to rust-lld on stable using these flags: -C linker=rust-lld -C linker-flavor=ld.lld.

Disadvantages

The disadvantage of using LLD as the default linker is that it doesn't have a default library search
path. If you need to link to a system library like newlib compiled for ARM Cortex-M you would need
to supply the path to that library using the -L flag. This is not a problem if you are linking to
C code compiled using a build script.

GCC is a better linker in that case because it already knows where the system libraries are so it's
not necessary to pass a library search path that depends on a particular installation. However, it's
possible to switch the linker to GCC to make this scenario simpler (-C linker=arm-none-eabi-gcc
once rust-lang/rust#52101 lands).


We need the approval of the Cortex-M community to land this change in rustc. Do you think the change
is worth it?

cc @rust-embedded/all
cc @alevy (Tock OS)

@korken89
Copy link
Contributor

I think it is worth it, let's pull this band-aid now before it becomes a real issue to do the switch. As long as we clearly state the migration path to the users it will cause minimal pain.

@andre-richter
Copy link
Member

It is the better and more self-contained solution in the long run. So break it early to make it nicer in the long run.

@therealprof
Copy link
Contributor

I've already already used binutils ld in the past to get rid of the gcc dependency (there was a ARM-provided toolchain a while back that was severly broken) which means this change would actually allow me to remove the linker flavour and stabilise the crate without any further change by simply removing a line.

Even for gcc abusers it should be a rather small and innocent change to make; maybe we could even put a lint in the compiler to tell the user exactly how to change the config.

So +1 from me and thanks for writing it up, @japaric.

@jamesmunns
Copy link
Member

jamesmunns commented Aug 11, 2018

As long as setting the linker back to GCC doesn't require a nightly flag (as it seems from your comment in the other issue), I don't have any issue with this. Being able to switch back without nightly would allow the user to regain the old behavior, in case they run in to an issue with LLD before we have used LLD for a significant enough amount of time.

@adamgreig
Copy link
Member

👍 on changing the default to lld so long as we have the ability to specify gcc on stable. As has been mentioned it seems the better solution long-term and now is the time to make changes, but it would be a shame if we get to 2018 edition with stable embedded but without being able to use gcc on it.

@jamesmunns

As long as setting the linker back to GCC doesn't require a nightly flag

Unless I'm misunderstanding, this is currently the case and may remain the case for 2018 edition; we need rust-lang/rust#52101 to land before it's possible to set without nightly.

@alevy
Copy link

alevy commented Aug 12, 2018

@japaric speaking for the Tock project, we've already manually switched over both userland and the kernel from GCC to LLD as the linker, so this change would be strictly better for us.

@thejpster
Copy link
Contributor

Well, if it's working for Tock then that gives me some confidence, and sooner is better than later with this sort of thing.

+1 from me.

@japaric
Copy link
Member Author

japaric commented Aug 17, 2018

cc @phil-opp @oli-obk @mbr (from embed-rs)

@japaric japaric added the RFC label Aug 17, 2018
@japaric
Copy link
Member Author

japaric commented Aug 19, 2018

The prerequisite for this (PR rust-lang/rust#52101) has been approved.

@japaric
Copy link
Member Author

japaric commented Aug 20, 2018

The prerequisite for this (PR rust-lang/rust#52101) has been approved.

The PR has landed 🎉


This RFC has received approval, in the form of GH reactions and comments, from 20+ people representing the embedded WG, the TockOS project and the embed-rs organization. The only concern raised so far is that it should be possible to switch to GCC on stable and that have been solved by PR rust-lang/rust#52101 which landed a few hours ago.

I'll start preparing a rust-lang/rust PR to change the default linker, and an accompanying announcement about the upcoming breakage and the migration path.

I won't submit the PR until the end of the week to give other people more time to comment on this RFC.

Consider the RFC in Final Comment Period (FCP) phase with disposition to accept it.

@sekineh
Copy link

sekineh commented Aug 20, 2018

@japaric
One question. The linker can be selected via cargo arguments, right?

In that case CI should build some binary using both linkers.

@japaric
Copy link
Member Author

japaric commented Aug 20, 2018

One question. The linker can be selected via cargo arguments, right?

Via rustc flags actually but you can use either something like cargo rustc -- -C linker=arm-none-eabi-gcc, or a Cargo configuration file:

[target.thumbv7m-none-eabi]
rustflags = ["-C", "linker=arm-none-eabi-gcc"]

In that case CI should build some binary using both linkers.

Yes, we should test both linkers in rust-lang/rust CI.

@japaric japaric added decision-accepted We voted on this proposal and accepted it and removed disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Aug 23, 2018
@japaric
Copy link
Member Author

japaric commented Aug 23, 2018

This RFC is now officially accepted. 🎉

Thanks everyone for your input!

i have sent a PR to change the default linker (see rust-lang/rust#53648) and I have written an announcement explaining the breakage and how to fix the builds (see #196). (The announcement hasn't go out yet; it's only up for review right now.)

@japaric japaric closed this as completed Aug 23, 2018
bors added a commit to rust-lang/rust that referenced this issue Aug 27, 2018
change the default linker of the ARM Cortex-M targets

to rust-lld so users won't need an external linker to build programs

This will break nightly builds.

We discussed this within the embedded WG and with the embedded community in
rust-embedded/wg#160 and there was consensus in that this breaking change is
worthwhile and that we should do it now before it becomes impossible to do
without breaking stable builds.

We have already written an announcement (see rust-embedded/wg#196) that explains
the breakage and instructs the users how to fix their builds. The TL;DR is that
they can switch to the old behavior by passing the `-C linker` flag to rustc.
We'll post the announcement as soon as this change makes into nightly.

closes rust-embedded/wg#160

r? @alexcrichton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
decision-accepted We voted on this proposal and accepted it RFC
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants