Skip to content

use "INSERT AFTER" in embedded-test.x#67

Merged
t-moe merged 2 commits intoprobe-rs:masterfrom
kaspar030:insert-after
Jul 8, 2025
Merged

use "INSERT AFTER" in embedded-test.x#67
t-moe merged 2 commits intoprobe-rs:masterfrom
kaspar030:insert-after

Conversation

@kaspar030
Copy link
Contributor

@kaspar030 kaspar030 commented Jul 6, 2025

TL/DR: add INSERT AFTER .comment; to embedded-test.x so it never overrides a default linker script.

Currently, the embedded-test.x is supposed to be added to builds by adding -Tembedded-test.x to the linker args.

-Tfoo is used to specify a new default linker script. If multiple linker scripts are added using -T..., the first replaces the default script, the rest is then considered to be augmenting other linker scripts.

On embedded, there are usually other linker scripts before -Tembedded-test.x, e.g., on cortex-m, it is -Tlink.x.

Now on std, we don't usually have another linker script, and some ld/lld provided default script is used. Here, adding -Tembedded-test.x would override that default script, causing linking to fail.

For the std example, I worked around that by not using -T but adding a relative path to the in-repo embedded-test.x. That works in this repo, for this example, but requires similar action for any users. The ususal println!("cargo::rustc-link-arg-tests=-Tembedded-test.x"); doesn't work on std.

I went into a deep dive on how this works, and found out:

  • for -Txxx, the linker search path is looked up
  • Both GNU ld and lld parse non-objects/archives as linker scripts, but without -T, the linker search path is not used.
  • Both GNU ld and lld do search the linker path if a file is added via -lfilename, and don't forcably use libfilename if filename is prefixed with :, but only GNU ld accepts linker scripts this way
  • both GNU ld and lld always treat a linker script as augmenting linker script if it contains INSERT AFTER/BEFORE, even if it is added using -T.

So in order to not override the default linker script on std, I've added INSERT AFTER .comment; into embedded-test.x. In my testing, that .comment section is inserted by GNU ld and lld on linux, cortex-m, riscv and xtensa.

Signed-off-by: Kaspar Schleiser kaspar@schleiser.de

@t-moe
Copy link
Contributor

t-moe commented Jul 8, 2025

Thank you for deep-diving into this. That is a nice solution.
Can you add a small comment in the linker-file, explaining the line and why its needed?

kaspar030 added 2 commits July 8, 2025 13:18
Signed-off-by: Kaspar Schleiser <kaspar@schleiser.de>
Signed-off-by: Kaspar Schleiser <kaspar@schleiser.de>
@kaspar030
Copy link
Contributor Author

Can you add a small comment in the linker-file, explaining the line and why its needed?

Done, not sure about the wording ...

@t-moe t-moe merged commit 81857b6 into probe-rs:master Jul 8, 2025
11 checks passed
@t-moe
Copy link
Contributor

t-moe commented Jul 8, 2025

Thank you.
Do you need a release now or can I wait & bundle the changes with other stuff?

@kaspar030
Copy link
Contributor Author

Do you need a release now or can I wait & bundle the changes with other stuff?

It can wait, we can use this from our branch for now.

Thanks!

@kaspar030 kaspar030 deleted the insert-after branch July 8, 2025 11:27
@bugadani
Copy link
Contributor

bugadani commented Aug 5, 2025

In my testing, that .comment section is inserted by GNU ld and lld on linux, cortex-m, riscv and xtensa.

I don't know what you did, but naively trying to update embedded-test results in esp-hal tests failing to compile with .comment not found for insert.

@t-moe
Copy link
Contributor

t-moe commented Aug 5, 2025

I don't know what you did, but naively trying to update embedded-test results in esp-hal tests failing to compile with .comment not found for insert.

can you share a failing build?

@bugadani

This comment was marked as outdated.

@bugadani
Copy link
Contributor

bugadani commented Aug 5, 2025

You can now see the build errors e.g. at https://github.com/esp-rs/esp-hal/actions/runs/16747543756/job/47409604439?pr=3889

@t-moe
Copy link
Contributor

t-moe commented Aug 5, 2025

You can now see the build errors e.g. at https://github.com/esp-rs/esp-hal/actions/runs/16747543756/job/47409604439?pr=3889

hmm, fails on xtensa only.
@kaspar030 you tested this on xtensa as well?

@t-moe t-moe mentioned this pull request Aug 5, 2025
@kaspar030
Copy link
Contributor Author

I think I did test on xtensa, wouldn't swear on it though. I'm on vacations for two more weeks, will deal with it then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants