knurling-rs / defmt

Efficient, deferred formatting for logging on embedded systems
https://defmt.ferrous-systems.com/
Apache License 2.0
812 stars 74 forks source link

Why "pub fn panic() -> !" is "#[inline(always)]" #824

Open gobftald opened 6 months ago

gobftald commented 6 months ago

I don't know whether is general, but in my case this causes that in a "backtrace list" we miss the most important item, the point where the "user code" exactly calls "defmt::panic!()".

When "backtrace" shows the "file-line-column" information for an address, in the current "#[inline(always)]" version "defmt::export::panic" is showed to be called from "user code" but one "stack frame" before, skipping the exact place where the call was really called. Which is not a really useful information. In the "#[inline(never)]" version, since the address of "defmt::export::panic" is really put into the stack, the "backtrace list" properly shows the exact place of this call in the "user code". And this is what we really needs.

Urhengulas commented 6 months ago

@japaric can you shed some light?

gobftald commented 6 months ago

As I mentioned, may be it is specific, but it is definitely happening in my environment. I'm programming an esp32c3 chip, so I use eps-backtrace, which simply collects addresses from stack to show a backtrace list. It works well in case of core::panic, but if I use defmt::panic the backtrace does not show the place where the user code called defmt::panic. That point the list shows a defmt internal address where defmt finally calls core::panic.

If I change #[inline(always)] to #[inline(never)] of 'pub fn panic() -> !' in 'defmt/defmt/src/export/mod.rs' the backtrace list works in a proper way again, showing properly where "defmt::panicked" was called in the user code.

Is it some 'light' for you?

Urhengulas commented 6 months ago

Is it some 'light' for you?

No.

I would be interested in why it was implemented with inline(always).

Also probe-rs is (and probe-run was) able to show the exact location in the user code. So maybe it is supposed to be like that.

gobftald commented 6 months ago

Maybe it is ESP specific. Since they/me use their own 'monitor' program and 'parser' logic in their 'espflash tool to decode your compression and then the address in the output.

Thank you for your attention/help. Anyway, I need to use this minor customisation of your code to get proper backtrace.

Urhengulas commented 6 months ago

Anyway, I need to use this minor customisation of your code to get proper backtrace.

I get that. And in general I am happy to make this change. I just want to avoid to break other code, in case the inlining is actually necessary in the usual setup.