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

defmt::export::acquire appears twice in optimized machine code #586

Closed
japaric opened this issue Sep 23, 2021 · 2 comments · Fixed by #587
Closed

defmt::export::acquire appears twice in optimized machine code #586

japaric opened this issue Sep 23, 2021 · 2 comments · Fixed by #587

Comments

@japaric
Copy link
Member

japaric commented Sep 23, 2021

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::export::acquire();
    app::exit()
}
$ cargo build --bin hello --release

$ arm-none-eabi-objdump -Cd target/thumbv7em-none-eabihf/release/hello
 000001ca <defmt::export::acquire>:
  1ca:   b5f0        push    {r4, r5, r6, r7, lr}
  1cc:   af03        add r7, sp, #12
(..)
 252:   f7ff ffae   bl  1b2 <core::panicking::panic>
 256:   defe        udf #254    ; 0xfe

(..)

00000356 <_defmt_acquire>:
 356:   b5f0        push    {r4, r5, r6, r7, lr}
 358:   af03        add r7, sp, #12
(..)
 3de:   f7ff fee8   bl  1b2 <core::panicking::panic>
 3e2:   defe        udf #254    ; 0xfe

Above _defmt_acquire and defmt::export::acquire are exact duplicates.


defmt::export::acquire is defined like this:

#[inline(never)]
pub fn acquire() {
    extern "Rust" {
        fn _defmt_acquire();
    }
    unsafe { _defmt_acquire() }
}

_defmt_acquire is defined in defmt-rtt. My guess is that _defmt_acquire gets inlined in the export::acquire BUT a full copy of it stays around because it has both #[no_mangle] on it and we have EXTERN(_defmt_acquire) in the defmt.x linker script.

Unsure how to fix. I wouldn't expect EXTERN to act like KEEP (LLD issue?). Removing EXTERN is likely to cause linker errors in some cases.

I think this issue also applies to other _defmt_* symbols but haven't checked

@jonas-schievink
Copy link
Contributor

Could we make export::acquire #[inline] and _defmt_acquire #[inline(never)]?

@japaric
Copy link
Member Author

japaric commented Sep 23, 2021

That should work!

(Marking export::acquire inline(never), as it is right now, does seems odd. _defmt_acquire is the "binary interface" and that's what should be kept around as a symbol so marking that one inline(never) makes more sense)

This also appears to affect release v0.2.3

bors bot added a commit that referenced this issue Sep 23, 2021
587: tweak inline attributes to remove machine code duplication r=jonas-schievink a=japaric

fixes #586 and saves 140 bytes of Flash in the example (repro case) given therein

Co-authored-by: Jorge Aparicio <jorge.aparicio@ferrous-systems.com>
@bors bors bot closed this as completed in 4bb54b9 Sep 23, 2021
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 a pull request may close this issue.

2 participants