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

unused _defmt symbols are kept by the linker #588

Open
japaric opened this issue Sep 23, 2021 · 4 comments
Open

unused _defmt symbols are kept by the linker #588

japaric opened this issue Sep 23, 2021 · 4 comments

Comments

@japaric
Copy link
Member

japaric commented Sep 23, 2021

related to #586

Steps to reproduce

Start from app-template, configure it to use the main branch of defmt then modify hello to look like this

fn main() -> ! {
    app::exit()
}
$ cargo build --bin hello --release

$ arm-none-eabi-nm -CSn target/thumbv7em-none-eabihf/release/hello | rg _defmt
0000000c 00000001 N _defmt_version_ = 3
0000000d 00000001 N _defmt_encoding_ = rzcobs
00000282 00000022 T _defmt_timestamp
000002ea 00000002 T __defmt_default_timestamp
000002ec 00000014 T __defmt_default_panic
000003e8 0000008e T _defmt_acquire
00000476 000000e6 T _defmt_release
0000055c 0000019e T _defmt_write

This is going to become more relevant after #519 lands because even if you fully disable logs (DEFMT_LOG=off) the symbols are kept and they take up valuable Flash space.

@japaric
Copy link
Member Author

japaric commented Sep 23, 2021

Behavior is the same using GNU LD so probably not a LLD bug.
Removing EXTERN from defmt.x lets the linker GC the symbols as expected (but IME not having that can break linking in the presence of parallel codegen units)

@japaric
Copy link
Member Author

japaric commented Sep 23, 2021

it seems that rustc is wrapping all Cargo dependencies (rlibs) in --start-group and --end-group when it passes them to the linker (I'm almost sure that was not the case years ago?) -- tested that on nightly because I needed to use -Z print-link-args. If that's the case and --start-group / --end-group is used also on stable then we may not need the EXTERN command in the linker script and linking should Just Work. (--start-group / --end-group forces the linker to look at all object files specified between those two flags -- the default linker behavior is to stop scanning object files as soon as pending undefined references are all. The use of --start-group/--end-group strikes me as a bit odd: it will make the linking process slower)

so ... maybe we can drop the EXTERNs to fix this issue and things won't break horribly?

@BryanKadzban
Copy link
Contributor

Any recent changes here?

I found this issue after noticing that a big chunk of formatting code was linked into my WIP USB-HS DFU bootloader for an stm32f7. I'd already removed ~3K of formatting code by persistently using defmt::panic! and defmt::unwrap!, plus not depending on crates that do any raw panics. "cargo bloat" shows 1.7K in "<&T as core::fmt::Display>::fmt", which is only called by the core::panic! implementation, which is only invoked by __defmt_default_panic, which is only linked in because of this issue. (I provide my own #[defmt::panic_handler] diverging fn that just does a udf.)

I see that the linker script now does a "PROVIDE(_defmt_panic = __defmt_default_panic)", and I see that the implementation of #[defmt::panic_handler] names the function _defmt_panic, so it looks like the situation is a little different. Is there a good way to get the linker to drop the __defmt_default_panic now?

@BryanKadzban
Copy link
Contributor

I haven't checked yet if feature flags are available to build.rs files, but if they are, would you be open to a PR adding a feature flag (on by default) that if missing would disable this PROVIDE from the linker script? For binary crates that don't set a defmt::panic_handler, disabling the feature would break linking, but if the binary crate does provide such a function, they can disable the feature with no issues. And just removing the PROVIDE line definitely saves that full 1.7K from my binary, plus a little for smaller functions only called by it.

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

No branches or pull requests

2 participants