feat: Persistent addr2line with ELF isolation and rebuild detection (IDFGH-18193) - #6
feat: Persistent addr2line with ELF isolation and rebuild detection (IDFGH-18193)#6nebkat wants to merge 1 commit into
Conversation
80102b6 to
d985b5b
Compare
Spawn addr2line once per ELF and feed addresses on stdin instead of
launching a fresh process per decoded line. The first decode still pays
addr2line's full ELF-load cost; every subsequent decode reuses the same
subprocess. Encapsulated as a focused Addr2LineRunner in a new
esp_idf_panic_decoder/addr2line.py module.
Subprocess management
* Persistent pool keyed by ELF path, one Popen per ELF.
* Single-address conversation: write `<addr>\n0xfefefefe\n`, read
response, terminate when the 3-line sentinel echo
(`0xfefefefe\n??\n??:0\n`) appears. 0xfefefefe is a classic
uninit-memory marker, well outside any ESP code region, and is
filtered by PcAddressMatcher.is_executable_address so it cannot
collide with a real lookup. The regex tolerates 64-bit pointer-width
padding.
* One-shot subprocess.check_output fallback per address if the
persistent path fails (broken pipe / OS error).
* atexit cleanup, plus __enter__/__exit__ on Addr2LineRunner and
PcAddressDecoder for callers that want deterministic teardown.
ELF isolation and stale detection
* Copy each ELF to a per-runner tempfile.mkdtemp() directory on spawn;
addr2line opens the copy, not the original. Prevents Windows from
blocking the linker on our open BFD handle during a rebuild.
* Stat the original (mtime, size) on every lookup; tear down the
cached process and respawn against a fresh copy when it changes, so
a rebuild during a monitor session is picked up automatically.
* Temp directory is removed on close().
Public API
* PcAddressDecoder, translate_addresses, decode_address and
perform_addr2line keep their existing signatures. perform_addr2line is
now a thin batched wrapper over the new single-address lookup_address.
* New PcAddressDecoder.lookup_address(address, elf_file, is_rom)
method for single-address callers.
* New PcAddressDecoder.close() for explicit teardown (optional —
atexit handles it otherwise).
* Removed parse_addr2line_output and the ADDR2LINE_ADDRESS_LOOKAHEAD_RE /
ADDR2LINE_FILE_LINE_RE constants it used. Nothing calls it now that
addresses are looked up one at a time, and keeping it would mean a
second copy of the frame parsing that now lives in addr2line.py.
d985b5b to
67012c0
Compare
|
Hi @nebkat, thank you for contributing. I would like to understand your use case better. Is this trying to solve the issue where the Monitor becomes unresponsive when in a boot loop? (as mentioned in espressif/esp-idf-monitor#45) I definitely agree that running addr2line in a subprocess for each line is not ideal. But considering that this only runs on a couple of lines when the chip is booted and then again when there is a panic, I think the cost is not that high (at least in a standard scenario). We can consider using a simpler solution - we could introduce a cache, which would cover the bootloop scenario quite nicely without needing to change the logic that much. I think that with a new "Force quit" option in IDF Monitor, this becomes less of an issue, so a simple cache could further improve the experience of using our tools without adding a lot of complexity. Let me know what you think. |
|
Hi @peterdragun, We are actually using backtraces at runtime for debugging non-fatal errors and event loop slowdowns so we get a lot more than just the panic. (Side note: this should be encouraged somewhere in the docs as it is quite the superpower! I think back to the days of inserting countless log lines to figure out where a failure was coming from...) Without this change it becomes very hard to follow what is going on as the screen starts to lag while resolving each entry. Admittedly we are on the extreme end as we frequently end up printing backtraces and they are quite long, but, with this change it is all instant and works perfectly. I appreciate this adds some complexity but it is overall nicely contained within this package and maintains the same external API so I think it could be a nice addition for these more advanced workflows. Separately I am considering making a PR that allows selectively enabling/disabling the backtrace decoding, as there are times when we do not need them but don't want to quit out of the monitor to toggle it. |
Why
PcAddressDecoderspawns a freshaddr2linefor every monitor line that contains hex addresses. Nearly all of that cost is startup —addr2lineloads and indexes the ELF each time — and it is paid again for every decoded backtrace line, on an ELF that has not changed. Keeping one process alive per ELF and feeding it addresses on stdin removes that cost from every decode after the first.What
addr2linesubprocess handling moves into a newesp_idf_panic_decoder/addr2line.py(Addr2LineRunner), leavingpc_address_decoder.pyto do address matching and presentation:<addr>\n0xfefefefe\nand reads until the sentinel's 3-line echo (0xfefefefe/??/??:0) marks the end of the response.-iemits a variable number of inlined frames, so a terminator is needed;0xfefefefeis well outside any ESP code region and is rejected byPcAddressMatcher.is_executable_address, so it cannot collide with a real lookup.subprocess.check_outputper address if the persistent path fails (broken pipe, spawn error), so a decode still returns a result.addr2lineopens a temp copy of the ELF, not the ELF itself. On Windows an open BFD handle can block the linker from rewriting the ELF during a rebuild — a real risk once the handle is long-lived rather than momentary.atexit, plusclose()and__enter__/__exit__for callers that want deterministic teardown.Because the spawn cost is gone, the per-ELF batching in
translate_addressesno longer earns its bookkeeping and collapses to a per-address loop.is_executable_addressfiltering and its first-ELF-wins precedence are unchanged.API
PcAddressDecoder(...),translate_addresses,perform_addr2line,decode_addressandpc_address_bufferkeep their signatures and behaviour —esp-idf-monitorneeds no changes and picks up the speedup as-is. Added:lookup_address(address, elf_file, is_rom)andclose().One removal:
parse_addr2line_output. It is unused once addresses are looked up one at a time, and keeping it would mean a second copy of the frame parsing that now lives inaddr2line.py. It was undocumented and not used byesp-idf-monitor, the only known consumer — happy to restore it as a wrapper if you would rather not break the signature.Testing
Run against real backtraces from an
esp-idf-monitorsession: decoded output matches the previous implementation, with the speedup visible from the second decoded line onward. The parser was additionally diff-tested against the old implementation across inlined frames, unresolved??/??,(discriminator N), CRLF, Windows paths and demangled C++ signatures, and the subprocess lifecycle (reuse, respawn on rebuild, respawn after a killed process, fallback paths, temp cleanup) was exercised against a stubaddr2line.The repo has no test suite to extend, so none of that is included here. Glad to add one if you would like it in the same PR.