Add monitor filter for decoding stack backtraces#9
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new PlatformIO device monitor filter to automatically decode N-ABLE/Nable hard fault “Call Stack Backtrace” addresses into file/line/function information using addr2line.
Changes:
- Introduces
nable_exception_decodermonitor filter implementation. - Detects backtrace blocks in serial output and replaces frames with decoded
addr2lineoutput. - Locates firmware and
addr2linevia build metadata (prog_path,cc_path).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| output = ( | ||
| subprocess.check_output(args + [addr]) | ||
| .decode(enc) | ||
| .strip() | ||
| ) | ||
|
|
||
| # newlines happen with inlined methods | ||
| output = output.replace("\n", "\n ") | ||
|
|
||
| output = self.strip_project_dir(output) | ||
| trace = "%s%s in %s\n" % (prefix, addr, output) | ||
| return trace | ||
|
|
||
| except subprocess.CalledProcessError as e: | ||
| sys.stderr.write( | ||
| "%s: failed to call %s: %s\n" | ||
| % (self.__class__.__name__, self.addr2line_path, e) | ||
| ) | ||
| return "%s%s in ??:?\n" % (prefix, addr) | ||
|
|
There was a problem hiding this comment.
build_backtrace() only handles subprocess.CalledProcessError, but subprocess.check_output() can also raise FileNotFoundError/OSError (e.g., non-executable/bad path) which would crash the monitor filter. Consider catching these exceptions and returning a fallback decoded line (similar to the CalledProcessError path) so the serial monitor keeps running.
| args = [self.addr2line_path, "-fipC", "-e", self.firmware_path] | ||
|
|
||
| try: | ||
| output = ( | ||
| subprocess.check_output(args + [addr]) | ||
| .decode(enc) | ||
| .strip() | ||
| ) |
There was a problem hiding this comment.
subprocess.check_output() is invoked without a timeout; if addr2line hangs (or the binary is very slow), it will block the monitor thread and stall the serial output. Consider adding a reasonable timeout and handling subprocess.TimeoutExpired with a clear fallback output.
| output = "" | ||
| lines = text.splitlines(True) | ||
|
|
||
| for line in lines: | ||
| ended = line.endswith("\n") or line.endswith("\r") | ||
| raw = line.rstrip("\r\n") if ended else line | ||
| ending = line[len(raw):] if ended else "" | ||
|
|
||
| # Detect start of backtrace | ||
| if "Call Stack Backtrace:" in raw: | ||
| self.in_backtrace = True | ||
| self.backtrace_buffer = "" | ||
| output += raw + (ending or "\n") | ||
| continue | ||
|
|
||
| # Detect end of backtrace | ||
| if self.in_backtrace and "======" in raw: | ||
| self.in_backtrace = False | ||
| # Process complete backtrace | ||
| decoded = self.process_backtrace(self.backtrace_buffer) | ||
| if decoded: | ||
| output += decoded | ||
| output += raw + (ending or "\n") | ||
| self.backtrace_buffer = "" | ||
| continue | ||
|
|
||
| # Buffer backtrace lines | ||
| if self.in_backtrace: | ||
| if ended: | ||
| self.backtrace_buffer += raw + "\n" | ||
| else: | ||
| self.backtrace_buffer += raw | ||
| else: | ||
| output += raw + ending | ||
|
|
||
| return output |
There was a problem hiding this comment.
rx() builds output via repeated string concatenation inside a loop. For high-throughput serial output this can become unnecessarily expensive (quadratic behavior). Consider accumulating pieces in a list and ''.join(...) at the end.
| for line in lines: | ||
| m = self.ADDR_PATTERN.search(line) | ||
| if m is not None: | ||
| decoded = self.build_backtrace(m.group(1), m.group(2)) | ||
| if decoded: | ||
| result += decoded | ||
| elif line.strip(): | ||
| # Preserve non-address informational lines inside the backtrace | ||
| result += line + "\n" | ||
|
|
||
| return result | ||
|
|
There was a problem hiding this comment.
process_backtrace()/build_backtrace() run addr2line once per frame. This can be noticeably slow for long backtraces and will delay monitor output. Consider batching addresses into a single addr2line invocation (it accepts multiple addresses) and then mapping the results back to frames.
| for line in lines: | |
| m = self.ADDR_PATTERN.search(line) | |
| if m is not None: | |
| decoded = self.build_backtrace(m.group(1), m.group(2)) | |
| if decoded: | |
| result += decoded | |
| elif line.strip(): | |
| # Preserve non-address informational lines inside the backtrace | |
| result += line + "\n" | |
| return result | |
| # First pass: collect all addresses to decode in one addr2line invocation | |
| prefixes = [] | |
| addrs = [] | |
| for line in lines: | |
| m = self.ADDR_PATTERN.search(line) | |
| if m is not None: | |
| prefixes.append(m.group(1)) | |
| addrs.append(m.group(2)) | |
| decoded_traces = None | |
| if addrs: | |
| decoded_traces = self._decode_addresses_batched(prefixes, addrs) | |
| # If batching failed for any reason, fall back to per-address decoding | |
| if decoded_traces is None: | |
| decoded_traces = [] | |
| for prefix, addr in zip(prefixes, addrs): | |
| decoded = self.build_backtrace(prefix, addr) | |
| decoded_traces.append(decoded or "") | |
| # Second pass: rebuild the result, inserting decoded traces | |
| trace_index = 0 | |
| for line in lines: | |
| m = self.ADDR_PATTERN.search(line) | |
| if m is not None: | |
| # For each backtrace frame line, append the corresponding decoded trace | |
| if trace_index < len(decoded_traces): | |
| result += decoded_traces[trace_index] | |
| trace_index += 1 | |
| elif line.strip(): | |
| # Preserve non-address informational lines inside the backtrace | |
| result += line + "\n" | |
| return result | |
| def _decode_addresses_batched(self, prefixes, addrs): | |
| """Decode multiple addresses in a single addr2line call. | |
| Returns a list of formatted backtrace strings (one per address), | |
| or None if batching fails and the caller should fall back. | |
| """ | |
| if not addrs: | |
| return [] | |
| enc = "mbcs" if IS_WINDOWS else "utf-8" | |
| # Use -fpC here so that each address produces exactly two lines | |
| # (function and location), which makes batching predictable. | |
| args = [self.addr2line_path, "-fpC", "-e", self.firmware_path] | |
| try: | |
| output = ( | |
| subprocess.check_output(args + addrs) | |
| .decode(enc) | |
| .strip() | |
| ) | |
| except subprocess.CalledProcessError as e: | |
| sys.stderr.write( | |
| "%s: failed to call %s (batched): %s\n" | |
| % (self.__class__.__name__, self.addr2line_path, e) | |
| ) | |
| return None | |
| lines = output.split("\n") if output else [] | |
| # Expect two lines per address (function and file:line) | |
| if len(lines) != 2 * len(addrs): | |
| # Unexpected format; fall back to per-address decoding | |
| return None | |
| decoded_traces = [] | |
| for idx, (prefix, addr) in enumerate(zip(prefixes, addrs)): | |
| func_line = lines[2 * idx] | |
| loc_line = lines[2 * idx + 1] | |
| frame_output = func_line + "\n" + loc_line | |
| # newlines happen with inlined methods (if any), indent them | |
| frame_output = frame_output.replace("\n", "\n ") | |
| frame_output = self.strip_project_dir(frame_output) | |
| trace = "%s%s in %s\n" % (prefix, addr, frame_output) | |
| decoded_traces.append(trace) | |
| return decoded_traces |
|
|
||
| def __call__(self): | ||
| """Initialize the filter.""" | ||
| self.buffer = "" |
There was a problem hiding this comment.
self.buffer is initialized but never used anywhere in this filter. Removing it would reduce confusion about whether partial-line buffering is intended.
| self.buffer = "" |
| IS_WINDOWS = sys.platform.startswith("win") | ||
|
|
||
| class NableExceptionDecoder(DeviceMonitorFilterBase): | ||
| """Nable backtrace decoder filter.""" |
There was a problem hiding this comment.
There is only a single blank line between the module-level constant IS_WINDOWS and the class NableExceptionDecoder definition. This repo’s Python files appear to follow PEP8’s two-blank-lines rule for top-level class definitions (e.g., platform.py). Consider adding an extra blank line here for consistency.
7c9eed8 to
afb1334
Compare
No description provided.