-
Notifications
You must be signed in to change notification settings - Fork 175
Add --nmagic linker arg, for unaligned flash origin support. #95
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
Conversation
Without this, the linker places some extra program header entries that can confuse flashing tools. Many thanks to @jonas-schievink for pointing me to this flag in Matrix.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jonas-schievink (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. |
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.
LGTM. We've done the same in a closed-source project a while ago, and it hasn't caused any issues there.
I'll leave this open for a while just in case someone has any comments or objections.
I'm trying to replicate this on any of my projects without success, any idea if there are more conditions than reported here to trigger the bug? |
We were seeing this only with GNU LD, but @Dirbaio also saw it with LLD. Maybe the linker or linker version matters? |
If I follow the instructions in the first post, I still don't reproduce this issue; the ELF looks fine in both debug and release modes, and I get the same thing in other projects too. I'm using rustc 1.46.0, but I'm sure I've generated non-aligned images for ages without issue. @Dirbaio, could you double check your |
I'm using rust nightly from rustup, and lld from Arch Linux repos. (not sure if rust uses system's lld or bundles its own though)
Nothing about rust in env. Repro is literally Just |
Aha, I can duplicate this using nightly but not on stable. Looks like this is an LLVM regression in nightly, then? Rust uses its own copy of LLD by default, it doesn't require one on the system. |
I did some more investigation.
The exact commit that changed this is llvm/llvm-project@87383e4 . Apparently it's for compat with GCC linker and with systems with bigger pages. It's not a nightly regression. IMO the right fix is The alternative is adding |
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.
This matches my expectations. LLD tries to match GNU LD's behavior.
bors r+
Configuration problem: |
Yeah, no bors in here. Let's just merge it then, shall we? ;) |
rationale is in rust-embedded/cortex-m-quickstart#95 we want to avoid potentially running into that issue in the future
Previously we told the linker the page size was small so that it would in effect remove any padding between program sections. This was a hack when switching linkers, but the proper way to do this is using the "-nmagic" flag which explicitly tells the linker to not align sections to page sizes. Some relevant discussions: - rust-embedded/cortex-m-quickstart#95 (comment) - #2180 Also copying in the linker docs which might be useful: ``` $ ld.lld --help | grep magic --nmagic Do not page align sections, link against static libraries. --no-nmagic Page align sections (default) --no-omagic Do not set the text data sections to be writable, page align sections (default) -N Alias for --omagic -n Alias for --nmagic --omagic Set the text and data sections to be readable and writable, do not page align sections, link against static libraries ``` Fixes #2219
If one switches to using the gnu linker (ld) instead of llvm because of this bug: rust-lang/rust#88704 then you get this:
linker switched using this .cargo/config.toml snippet.
Is there a solution that also works for gnu ld ? |
Try replacing [target.thumbv7m-none-eabi]
rustflags = [
"-C", "linker=arm-none-eabi-ld",
|
doh! yes, overlooked that... I'll give it a whirl and report back as time permits. |
I have to use 'gcc' because other arguments need to be passed to it, for map file and for keeping generated assembly files.
a solution that works is to use a build.rs file:
but you have to know which linker you're using to uncomment the right line. Some automatic way of doing it would be ideal. |
Without this, the linker places some extra program header entries that can confuse flashing tools.
Many thanks to @jonas-schievink for pointing me to this flag in Matrix.
How to reproduce the bug
cargo generate --git https://github.com/rust-embedded/cortex-m-quickstart
ORIGIN = 0x00027000
inmemory.x
cargo build
llvm-objdump -x target/thumbv7em-none-eabihf/debug/repro
Program header entries will look something like this:
The entry at 0x00020000 causes flashing tools (probe-rs at least) to flash data at 0x00020000, corrupting whatever's there. This happens whenever flash ORIGIN is not a multiple of 0x10000.
With
--nmagic
, program headers are correct and flashing does what it's supposed to do: