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 on linker customization convention #24

Closed
wants to merge 1 commit into from

Conversation

japaric
Copy link
Member

@japaric japaric commented May 13, 2017

Sumary:

Set a convention for passing custom arguments to the linker when using Cargo for
embedded development. This RFC proposes adopting a project local configuration
file, .cargo/config, to pass the arguments.


Rendered

My ultimate goal here is to remove the Xargo requirement for embedded, at least
for ARM Cortex-M, development. With the .cargo/config we can stick to the
built-in targets like thumbv7m-none-eabi. Then if we make rust-std
components (core + compiler-builtins) available for those built-in targets
via rustup, we can use normal Cargo to build ARM Cortex-M applications.

cc @alevy @FenrirWolf your projects are using custom targets to pass arguments
to the linker. Do you think a built-in target + .cargo/config file could meet
the needs of your projects?

cc @jamesmunns @thejpster @whitequark thoughts?

@japaric
Copy link
Member Author

japaric commented May 13, 2017

Although not mentioned in the RFC text, both approaches require files that Cargo doesn't keep track of: custom target specification files or .cargo/config. The current approach to provide those files to the end user is to use a Cargo project template. This template contains these extra files.

Examples of Cargo project templates:

@SimonSapin
Copy link

teensy3-rs for example has a fairly verbose linker script. It would be nice if it weren’t part of the boilerplate that needs to copied into every project.

@japaric
Copy link
Member Author

japaric commented May 13, 2017

@SimonSapin a build script can be used to remove the need to manually copy that into each project. Example. The end user would still need to configure the linker invocation to pass -Tlink.x or w/e the linker script name is. But that can be taken care of with a project template. Example

@Ericson2314
Copy link

Mmm i consider .cargo/config to be user+project configuration, i.e. not something to be version controlled. A virtual workspace root is the proper place for per-project config. [I wish certain configuration items could only go on virtual root rather than crate to make this clearer, but that's another discussion.]

@SimonSapin
Copy link

+1 on separating user config typically in .gitignore and project files required to build.

@whitequark
Copy link

  • Write a custom target specification file, usually basing it on a built-in
    target, and then adding to it the linker arguments in the {pre,,}-link-args
    fields.
  • Use a project local configuration file, .cargo/config, to pass linker
    arguments through the target.$T.rustflags variable.

For me, both approaches are unacceptable. Consider this: I'm trying to build something for a series of devices differing only in flash size (or similar irrelevant minutae). This is impossible to do without xargo with either of them AFAICT.

@thejpster
Copy link
Contributor

I like this, but I wanted to note that I hope we can get to a world where we can easily build embedded applications for multiple 'configurations' (what in the C/Make/SCons world I might have called 'targets'). For example, imagine an IoT MQTT/WiFi demo application which can be built for maybe four popular ARM/MIPS developer kits, as well as a Mac/Windows/Linux command line application. As far as the embedded builds go, I see a 'configuration' as being defined by:

The board (e.g. TI Stellaris Launchpad)
The SoC (e.g. TI LM4F120)
The Arch (the 'target', e.g. thumbv7em-none-eabi)

As you say, the linker parameters are a function of the board (when you have external RAM/Flash), the SoC (when you have on-board RAM/Flash) and the application (when you have a need to store stuff in weird sections, like .no_init blocks for pools allocators).

If your crate was say, a framework or an RTOS rather than a specific concrete application, then you might have multiple examples which are concrete applications. Again, you should be able to build any of those for any supported configuration.

@ppannuto
Copy link

Chiming in from the Tock world, this is wonderfully timely, as I'm trying to bump Tock to the latest nightly (and have been struggling immensely with it off and on for the last week)

Is there a reason that the contents of .cargo/config can't/shouldn't be in in Cargo.toml? Coming from a TinyOS background, build configuration in hidden files was one of the biggest pain points in figuring out what was going on in a system. I'm pretty strongly opposed to relying on a hidden file/directory without a really compelling reason (+cc @bradjc)

@japaric
Copy link
Member Author

japaric commented May 15, 2017

Thanks everyone for your comments!

@Ericson2314

Mmm i consider .cargo/config to be user+project configuration,

Could you elaborate on what "user" and "project" configuration mean to you?

In my mind there are only two types of settings in the Cargo world: crate settings and project local settings. Crate settings carry over with the crate regardless of whether it's build as a dependency or as the top crate. Project local settings only apply to the current project that's being built.

Cargo.toml has mostly crate settings like the crate name and its dependencies. These are used by Cargo when the crate is build as the top crate or as a dependency.

Cargo.toml also contains project local settings like [profile.dev] and [replace] (I think this one is also project local). These kind of settings only apply when the crate is being built as the top crate. IOW, the e.g. profile.dev settings of dependencies are completely ignored during a Cargo build.

.cargo/config only contains project local settings. That is: the .cargo/config files stored in dependencies are ignored by Cargo when building the top crate.

A virtual workspace root is the proper place for per-project config.

By virtual workspace do you mean a Cargo workspace or something else? Do note that linker arguments are only relevant to the top / application crate and that dependencies are oblivious to what those linker arguments are.

I wish certain configuration items could only go on virtual root rather than
crate to make this clearer, but that's another discussion.

If by that you mean that Cargo.toml shouldn't contain project local settings like [profile.dev] then I agree.

@SimonSapin

.cargo/config is not in .gitignore by default. If you were referring to .cargo/config as an "user" setting.

@whitequark

For the particular example of variable flash size you wouldn't need Xargo if we stick to .cargo/config. Following the model of the cortex-m-quickstart template you could make the memory.x linker script, which specifies the memory layout of the program, customizable through the build.rs script then you could change the flash size with invocations like these:

# one device
$ FLASH_SIZE=128K cargo build --target thumbv7m-none-eabi --example foo

# the other device
$ FLASH_SIZE=256K cargo build --target thumbv7m-none-eabi --example bar

The build script would take care of reading the FLASH_SIZE env variable and using it (or a reasonable default if it's not set) to create an appropriate memory.x file.

Do you have an example that wouldn't be covered by linker script customization via a build script?

@thejpster

I would love to see that as well but I think managing multiple configurations belongs to some other tool that drives Cargo. Cargo should have some defined / preferred way to let that other tool customize linker arguments / rustflags / etc.

If you would like to discuss such configuration management please open issue or continue an existing discussion like this one. Discussing the preferred way to customize linker arguments does belong here.

@ppannuto

I don't think there's any reason other than nobody has requested it. Cargo.toml already contains project local settings as I defined above. So either the stuff in .cargo/config could be moved to Cargo.toml or all the project local settings could be moved to a different non-hidden file (Cargo.local.toml or w/e). I prefer the later.

@japaric
Copy link
Member Author

japaric commented May 15, 2017

To elaborate a bit more on my previous comment:

The "project" in project local setting refers to the first Cargo project that you find by searching for a Cargo.toml in the current directory then in the parent directory and so on.

@thejpster

I suppose it's already possible to create such configuration manager. Most (or all?) of the settings in .cargo/config can be tweaked using environment variables (e.g. CARGO_TARGET_ARMV7_NONE_EABI_RUSTFLAGS=.. is the same as [target.thumbv7m-none-eabi.rustflags]). I have some ideas on this topic so I'll open a separate issue about this.

@whitequark

If your use cases is not supported by simply tweaking a linker script then you probably need something like the configuration manager mentioned above. Specially if e.g. you need to use crate A when building for device X but you need crate B when building for device Y. Here crates A and B could be device crates created using svd2rust and your application has been written in a device agnostic way using some HAL.

@ppannuto

I think the reason why this configuration is in a hidden file is that Cargo looks for the .cargo/config in the current directory then in the parent directory an so on until it reaches the root so it should be a hidden thing as per Linux / UNIX conventions (?) -- at least it should be hidden if it's in the home directory: ~/.cargo/config. I think Cargo then merges the configuration stored in all the .cargo/config files it found during that search. (I personally will never place a linker related configuration in ~/.cargo/config and have it apply to all projects; probably that would for some projects, but break the rest)


I think I should be more specific about this particular RFC: Should .cargo/config be the preferred way to configure linker arguments when you don't need to manage several configurations for your application? In this case you shouldn't need to use a configure manager and directly using Cargo / Xargo should suffice to build the application.

@Ericson2314
Copy link

@japaric Cabal recently redid things as http://cabal.readthedocs.io/en/latest/nix-local-build.html#configuring-builds-with-cabal-project and I think it's a great model we should emulate.

.cargo/config for the current project maps to cabal.project.local which is not version controlled, the virtual workspace root (Cargo.toml without crate) maps to cabal.project which shares the same syntax and is version controlled, and the lockfile is cabal.freeze.

Hopefully that mapping + docs make sense, and the philosophy of it all is clear.

@thejpster
Copy link
Contributor

After some thought, I agree with @Ericson2314 - .cargo/config should be uncontrolled. I don't really buy @alexcrichton's argument (I'm an embedded programmer and mucking around with the linker is sort of par for the course), but even if you did, I don't see how using a different config file changes the argument. Either Cargo isn't able to control linking, and we need another tool that can, or it is, and the specification should be in Cargo.toml.

@japaric
Copy link
Member Author

japaric commented May 16, 2017

@Ericson2314 Sorry, the linked documentation didn't help me understand their model; it explained details about the syntax but didn't give a high level overview of why things are structured the way they are. If you know an example where that tool is used in an embedded context where linker scripts and/or object files are passed to the linker that would be helpful. Otherwise I don't see how it adds value to the discussion.

@thejpster

.cargo/config should be uncontrolled

It already is in a sense. Nobody checks .cargo/config into their crates' code because Cargo will ignore it when the crate is built as a dependency.

The fact that templates are built today as crates that contain a .cargo/config is an implementation detail. You could (a) write the .cargo/config yourself and it wouldn't be version controlled, or (b) use some scaffolding tool that will build the .cargo/config for you.

Either Cargo isn't able to control linking

It can in several different ways: cargo rustc -- -C link-arg=.., RUSTFLAGS='-C link-arg=..' cargo build, [target.$T.rustflags] in .cargo/config.

or it is, and the specification should be in Cargo.toml.

Letting each dependency inject custom linker arguments is a can of worms IMO. Linker arguments are order sensitive so your application may build today but not tomorrow when you add a dependency that modifies the dependency graph that causes a reordering the linker arguments. Or perhaps you simply bumped the version of a dependency (from crates.io) and that new version decided to pass a linker argument that it's incompatible with your application. Then you also have problem of having more than one dependency passing a linker script to the linker so your linker ends up with more than one linker scripts in its arguments. None of these problems occur if the only place where linker arguments can be specified is at the top crate, which is what the .cargo/config approach is all about.

@alevy
Copy link

alevy commented May 16, 2017

Adding my two cents to the Tock perspective. There are two scenarios for us.

Building a kernel for a particular board

Currently, the world of Cargo makes this unfortunate. A board crate depends on a chip crate, and the board crate is the main crate (or at least it's where you run cargo). We currently end up using a custom target (e.g. sam4l.json for the Atmel SAM4L), which we copy into the board. That target specifies a linker script which is expected to be at a particular location (although technically the target is board customizable). That target is passed as a compile flag to cargo in a Makefile. This is nearly the only purpose of the Makefile.

This is all a bit of a mess considering there is only ever one meaningful target for a board, because it is tightly coupled to a chip and a linker script.

For this scenario, it really makes the most sense to specify the linker script and arguments to the linker in either Cargo.toml or some similar file (e.g. Cargo.local.toml as @japaric). Having it in a hidden folder is a problem for reasons people have already pointed out, as well as because it doesn't show up in editors like vim. It also just doesn't make sense as something that would be inhereted down the directory tree (while things like pointing to a specific linker binary path, for example, does).

Building Rust userland apps

This is a bit more complicated to comment on, both because it's a more complex scenario and because support for Rust in Tock userland is pretty half-baked so far, so we don't have much experience.

In practice, we want to compile for many targets at once (userland apps are portable and we bundle them into multi-arch bundles). However, the linker script does not change between architectures. Here, again, the .cargo/config solution works, but would be really unnecessarily verbose and bug prone if someone changes, .e.g, the linker used and forgets to copy that change to all targets.

Moreover, it also doesn't semantically make sense to use thumbv7-none-eabi for userland apps, since they are not running on bare-metal (they are running as a process in Tock). So semantically, they should use a target that looks more like thumbv7-tock-eabi.

In principal, we could try to merge that target into Rust master (as Redux has done, I believe) which would potentially obviate this whole discussion for Tock userland processes.

@japaric
Copy link
Member Author

japaric commented May 17, 2017

@alevy Thanks for the input.

That target specifies a linker script which is expected to be at a particular location

Linkers look for linker scripts in the library search path so if you have a -L $OUT_DIR/ld and a $OUT_DIR/ld/kernel_layout.ld generated through a build script then the linker will find it. I think it would make sense to do something like that in your kernel crate and have board crates only specify the memory layout in single linker script.

So the kernel_layout.ld would look like this:

INCLUDE chip_layout.ld

/* the rest will be the same as the current kernel_layout.ld */

and board crates would only contain the chip_layout.ld linker script. (And -Tkernel_layout.ld would be in .cargo/config)

We currently end up using a custom target (e.g. sam4l.json for the Atmel SAM4L)

That custom saml4 target has the exact same code gen options as the built-in thumbv7em-none-eabi; you could use that built-in target instead plus a .cargo/config (as soon as -Z pre-link-args lands in tree). The post-link-args field in your saml4.json should be provided by the build script (e.g. cargo:rustc-link-lib=static=c) of the crate that makes use of libc functions, IMO; the less linker arguments you have in .cargo/config the better, and Cargo supports -l linker arguments.

However, the linker script does not change between architectures.

In that case each application should not have a copy of the linker script; instead a crate like libtock could provide that linker script via a build script (as explained at the beginning of this comment). That way userland app authors would never have to directly deal with linker scripts.

Moreover, it also doesn't semantically make sense to use thumbv7-none-eabi for userland apps

Are those different from the none variants codegen-wise though? I'm wondering if you could just use a sysroot compiled for a none target (to hypothetically not need Xargo). I noticed that relocation-model is different but I'm not sure if that's something that should cause a recompilation of dependencies. If #[cfg(target_os = "tock")] ever makes it to libcore then you would certainly need to recompile the sysroot.

@alevy
Copy link

alevy commented May 17, 2017

@japaric Thanks those are useful comments above and beyond this discussion. I'll definitely look into those recommendations.

I noticed that relocation-model is different but I'm not sure if that's something that should cause a recompilation of dependencies

Most definitely. The LLVM output is totally different. So, now that you mention in, I don't think having a pre-compiled sysroot for thumb*-none-eabi would work for these apps, so we would almost certainly need Xargo in any case.

@whitequark
Copy link

Most (or all?) of the settings in .cargo/config can be tweaked using environment variables (e.g. CARGO_TARGET_ARMV7_NONE_EABI_RUSTFLAGS=.. is the same as [target.thumbv7m-none-eabi.rustflags]). I have some ideas on this topic so I'll open a separate issue about this.

@japaric I haven't realized that these can be tweaked via environment variables. I think all of my use cases are covered (or at least I can't imagine any right now), and it would be ergonomic enough with some simple cargo-x subcommand wrapper.

bors added a commit to rust-lang/rust that referenced this pull request May 18, 2017
add -Z pre-link-arg{,s} to rustc

This PR adds two unstable flags to `rustc`: `-Z pre-link-arg` and `-Z
pre-link-args`. These are the counterpart of the existing `-C link-arg{,s}`
flags and can be used to pass extra arguments at the *beginning* of the linker
invocation, before the Rust object files are passed.

I have [started] a discussion on the rust-embedded RFCs repo about settling on a
convention for passing extra arguments to the linker and there are two options
on discussion: `.cargo/config`'s `target.$T.rustflags` and custom target
specification files (`{pre,,post}-link-args` fields). However, to compare these
two options on equal footing this `-Z pre-link-arg` feature is required.

[started]: rust-embedded/wg#24

Therefore I'm requesting landing this `-Z pre-link-arg` flag as an experimental
feature to evaluate these two options.

cc @brson
r? @alexcrichton
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 18, 2017
add -Z pre-link-arg{,s} to rustc

This PR adds two unstable flags to `rustc`: `-Z pre-link-arg` and `-Z
pre-link-args`. These are the counterpart of the existing `-C link-arg{,s}`
flags and can be used to pass extra arguments at the *beginning* of the linker
invocation, before the Rust object files are passed.

I have [started] a discussion on the rust-embedded RFCs repo about settling on a
convention for passing extra arguments to the linker and there are two options
on discussion: `.cargo/config`'s `target.$T.rustflags` and custom target
specification files (`{pre,,post}-link-args` fields). However, to compare these
two options on equal footing this `-Z pre-link-arg` feature is required.

[started]: rust-embedded/wg#24

Therefore I'm requesting landing this `-Z pre-link-arg` flag as an experimental
feature to evaluate these two options.

cc @brson
r? @alexcrichton
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 19, 2017
add -Z pre-link-arg{,s} to rustc

This PR adds two unstable flags to `rustc`: `-Z pre-link-arg` and `-Z
pre-link-args`. These are the counterpart of the existing `-C link-arg{,s}`
flags and can be used to pass extra arguments at the *beginning* of the linker
invocation, before the Rust object files are passed.

I have [started] a discussion on the rust-embedded RFCs repo about settling on a
convention for passing extra arguments to the linker and there are two options
on discussion: `.cargo/config`'s `target.$T.rustflags` and custom target
specification files (`{pre,,post}-link-args` fields). However, to compare these
two options on equal footing this `-Z pre-link-arg` feature is required.

[started]: rust-embedded/wg#24

Therefore I'm requesting landing this `-Z pre-link-arg` flag as an experimental
feature to evaluate these two options.

cc @brson
r? @alexcrichton
@jamesmunns
Copy link
Member

@japaric I agree with the "compose" argument you made above. I use this (sort of) in teensy3, by having the "BSP Crate" inject the linker script to the $OUT_DIR, which allows the main crate to use (or not use) the linker script as provided. I don't compose them in any way yet, but it absolutely could be done that way.

Here is where I "inject": https://github.com/jamesmunns/teensy3-rs/blob/add-feature-flags/teensy3-sys/build.rs#L115-L123

And here is where I "receive": https://github.com/jamesmunns/teensy3-rs-demo/blob/teensy_3_5_test/.cargo/config#L4-L6

@SimonSapin
Copy link

Is copying the linker script into $OUT_DIR necessary? Could we instead have cargo:rustc-link-search={source_directory}?

@japaric
Copy link
Member Author

japaric commented Jun 6, 2017

@SimonSapin Not in the simpler scenarios, but if you want to generate a custom linker script in the build script according to whether Cargo features X and/or Y are enabled then it becomes necessary.

I think using $OUT_DIR is also good "hygiene"; if you use -L /path/to/src/dir the linker could untentionally use statically libraries or other linker scripts lying in your source directory instead of the ones you meant to use. With -L$OUT_DIR the linker will only use what you explicitly decided to put there and not what you happened to have in your source directory.

@FenrirWolf
Copy link

Just noticed that I was pinged here, but I'm not sure what I can add to the discussion since my project involves a custom userland rather than a strictly embedded context, and so I need Xargo no matter where I put my linker args.

@japaric
Copy link
Member Author

japaric commented Jun 22, 2017

@FenrirWolf I'd like to know whether you can move all the linker arguments from your target definition to .cargo/config at all. In your particular case you are stuck with Xargo because there's no armv6-none-newlibeahif (curious about whether the cfg(env = "newlib") is used at all) in the compiler and much less rust-std binaries for that target.

@FenrirWolf
Copy link

@japaric The real libstd does have a few cfg annotations for newlib, since i guess some of the more obscure in-tree targets make use of it somewhere. I just have it included in my spec for completeness' sake.

Either way, I tested it out and putting the linker arguments in .cargo/config works just fine.

@jamesmunns
Copy link
Member

I believe that all items listed here have been addressed in one way or another. I am marking this PR for a cleanup sweep.

If you would like this RFC to persist, please add a justification of what it still addresses as of the current date.

@jamesmunns jamesmunns added the feb-2019-cleanup These issues are proposed to be closed, as part of a cleanup of issues in February 2019 label Feb 4, 2019
@japaric
Copy link
Member Author

japaric commented Feb 17, 2019

I am closing this PR, please feel free to open a new issue if you would like this discussed further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feb-2019-cleanup These issues are proposed to be closed, as part of a cleanup of issues in February 2019
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants