Repository: esp-rs/espmonitor (language: rust)

Issue:
Improve addr2line support/behaviour
### Support Rust demangling

When `addr2line` demangles Rust symbols, you end up with spaces in a bunch of places, so the current `ADDR2LINE_RE` doesn't match the output. Example output:

`0x400de4eb: core::fmt::num::imp::<impl core::fmt::Display for i32>::fmt at /home/tcb/build/esp-rs/rust/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/src/rust/library/core/src/fmt/num.rs:280`

It might be easiest to just match `0x[0-9a-f]+:.*` and print the whole thing?

### Print inline or out of line?

Currently the behaviour is to replace addresses with `addr2line` output inline. Maybe it's a personal thing but I prefer the `idf_monitor` approach of printing the output on the line(s) below the one containing the address(es), in a distinct colour/style. Something like the following, maybe... (using [console](https://crates.io/crates/console) for colour control):

![image](https://user-images.githubusercontent.com/40573959/133280601-686ab32b-a766-4748-93fc-611e896d078f.png)

(Yeah, it's ugly, but at least I'm not gonna mistake it for the real program output.)

Thoughts?


Discussion / hints:
Agree that we should fix the spacing issue with Rust symbol names.  I was just looking into the [addr2line](https://docs.rs/addr2line/0.16.0/addr2line/) crate yesterday, which might be a better thing to use, since we won't have to rely on fragile text parsing to get the information we want.

I actually intentionally put the addr2line output mixed in with the output because the other behavior (separate line, what idf_monitor does) actually confused me for the longest time (I couldn't figure out where it was coming from and didn't know that the monitor app was doing it; I thought it was some weird error output generated by the running code itself).

I just worked up [this crossterm PR](https://github.com/esp-rs/espmonitor/pull/4) last night, which should fix the polling issues for good, as well as allow us to support console commands on Windows.  We could use its facilities to do colorized output for this extra output.  I'd be in favor of making the new output more distinct.  But since Rust symbol names tend to be a lot longer than C names, I agree it might be a good idea to rethink how that output is presented, as it could get much harder to read if the output is in-line.
Perhaps we can indeed put the output on separate lines, but in addition to adding some color, prefix and indent the line so it's clear that it's "special" and not a part of normal output.
Agreed that it should be much more distinct than idf_monitor's equivalent. Prefix and indent would definitely help. Maybe also a background colour.

Much prefer the addr2line crate also. Better than text parsing, doesn't require having the right toolchain on PATH, and allows things like iterating over inlined functions frame-by-frame from a single address...
Oh, I spent some time trying to use the `addr2line` crate, and it wasn't as easy as expected.  Turns out (if you look at the addr2line example CLI tool they provide) there's a bit more you have to do than just use the `addr2line` crate's small API.  Mostly now I'm having issues because the `object` and `addr2line` crates like to borrow the input data in ways that make it hard for me to pass around everything that's needed to make this work.

Fix patch:
diff --git a/espmonitor/src/lib.rs b/espmonitor/src/lib.rs
index 4972c33..4ba2a30 100644
--- a/espmonitor/src/lib.rs
+++ b/espmonitor/src/lib.rs
@@ -17,7 +17,9 @@
 
 use addr2line::Context;
 use crossterm::{
+    QueueableCommand,
     event::{self, Event, KeyCode, KeyEvent, KeyModifiers},
+    style::{Color, Print, PrintStyledContent, Stylize},
     terminal::{disable_raw_mode, enable_raw_mode},
 };
 use gimli::{read::Reader, EndianRcSlice, RunTimeEndian};
@@ -27,7 +29,7 @@ use regex::Regex;
 use serial::{self, BaudRate, SerialPort, SystemPort};
 use std::{
     fs,
-    io::{self, ErrorKind, Read, Write},
+    io::{self, ErrorKind, Read, Write, stdout},
     process::exit,
     time::{Duration, Instant},
 };
@@ -44,8 +46,6 @@ lazy_static! {
         .expect("Failed to parse line separator regex");
     static ref FUNC_ADDR_RE: Regex = Regex::new(r"0x4[0-9a-f]{7}")
         .expect("Failed to parse program address regex");
-    static ref ADDR2LINE_RE: Regex = Regex::new(r"^0x[0-9a-f]+:\s+([^ ]+)\s+at\s+(\?\?|[0-9]+):(\?|[0-9]+)")
-        .expect("Failed to parse addr2line output regex");
 }
 
 macro_rules! rprintln {
@@ -148,10 +148,11 @@ fn run_child(args: AppArgs) -> Result<(), Box<dyn std::error::Error>> {
         symbols,
     };
 
+    let mut output = stdout();
     let mut buf = [0u8; 1024];
     loop {
         match dev.read(&mut buf) {
-            Ok(bytes) if bytes > 0 => handle_serial(&mut serial_state, &buf[0..bytes])?,
+            Ok(bytes) if bytes > 0 => handle_serial(&mut serial_state, &buf[0..bytes], &mut output)?,
             Ok(_) => if dev.read_dsr().is_err() {
                 rprintln!("Device disconnected; exiting");
                 break Ok(());
@@ -190,7 +191,7 @@ fn reset_chip(dev: &mut SystemPort) -> io::Result<()> {
     Ok(())
 }
 
-fn handle_serial(state: &mut SerialState, buf: &[u8]) -> io::Result<()> {
+fn handle_serial(state: &mut SerialState, buf: &[u8], output: &mut dyn Write) -> io::Result<()> {
     let data = String::from_utf8_lossy(buf);
     let mut lines = LINE_SEP_RE.split(&data).collect::<Vec<&str>>();
 
@@ -211,8 +212,7 @@ fn handle_serial(state: &mut SerialState, buf: &[u8]) -> io::Result<()> {
             };
 
         if !full_line.is_empty() {
-            let processed_line = process_line(state, full_line);
-            rprintln!("{}", processed_line);
+            output_line(state, full_line, output)?;
             state.unfinished_line.clear();
         }
     }
@@ -221,25 +221,24 @@ fn handle_serial(state: &mut SerialState, buf: &[u8]) -> io::Result<()> {
         state.unfinished_line.push_str(nel);
         state.last_unfinished_line_at = Instant::now();
     } else if !state.unfinished_line.is_empty() && state.last_unfinished_line_at.elapsed() > UNFINISHED_LINE_TIMEOUT {
-        let processed_line = process_line(state, &state.unfinished_line);
-        rprintln!("{}", processed_line);
+        output_line(state, &state.unfinished_line, output)?;
         state.unfinished_line.clear();
     }
 
     Ok(())
 }
 
-fn process_line(state: &SerialState, line: &str) -> String {
-    let mut updated_line = line.to_string();
-
+fn output_line(state: &SerialState, line: &str, output: &mut dyn Write) -> io::Result<()> {
     if let Some(symbols) = state.symbols.as_ref() {
+        let mut cur_start_offset = 0;
+
         for mat in FUNC_ADDR_RE.find_iter(line) {
-            let (function, file, line) = u64::from_str_radix(&mat.as_str()[2..], 16)
+            let (function, file, lineno) = u64::from_str_radix(&mat.as_str()[2..], 16)
                 .ok()
                 .map(|addr| {
                     let function = find_function_name(symbols, addr);
-                    let (file, line) = find_location(symbols, addr);
-                    (function, file, line)
+                    let (file, lineno) = find_location(symbols, addr);
+                    (function, file, lineno)
                 })
                 .unwrap_or((None, None, None));
 
@@ -247,12 +246,24 @@ fn process_line(state: &SerialState, line: &str) -> String {
                 s.unwrap_or_else(|| "??".to_string())
             }
 
-            let name = format!("{} [{}:{}:{}]", mat.as_str(), or_qq(function), or_qq(file), or_qq(line.map(|l| l.to_string())));
-            updated_line = updated_line.replace(mat.as_str(), &name);
+            output.queue(Print(line[cur_start_offset..mat.end()].to_string()))?;
+            cur_start_offset = mat.end();
+            let symbolicated_name = format!(" [{}:{}:{}]", or_qq(function), or_qq(file), or_qq(lineno.map(|l| l.to_string())))
+                .with(Color::Yellow);
+            output.queue(PrintStyledContent(symbolicated_name))?;
+        }
+
+        if cur_start_offset < line.len() {
+            output.queue(Print(line[cur_start_offset..line.len()].to_string()))?;
         }
+    } else {
+        output.queue(Print(line.to_string()))?;
     }
 
-    updated_line
+    output.write_all(b"\r\n")?;
+    output.flush()?;
+
+    Ok(())
 }
 
 fn handle_input(dev: &mut SystemPort, key_event: KeyEvent) -> io::Result<()> {
@@ -286,29 +297,3 @@ fn find_location(symbols: &Symbols<'_>, addr: u64) -> (Option<String>, Option<u3
         ))
         .unwrap_or((None, None))
 }
-
-//fn find_frame(context: &Context<EndianRcSlice<RunTimeEndian>>, addr: u64) -> Result<Option<(Option<String>, Option<String>, Option<u32>)>, String> {
-//    context
-//        .find_frames(addr)
-//        .and_then(|mut frames| frames.next())
-//        .map_err(|err| err.to_string())
-//        .map(|frame| frame.map(|frame|
-//            (
-//                frame.function.and_then(|fname| fname.name.to_string_lossy().ok().map(|name| name.into_owned())),
-//                frame.location.as_ref().and_then(|location| location.file).map(|file| file.to_string()),
-//                frame.location.as_ref().and_then(|location| location.line)
-//            )
-//        ))
-//}
-//
-//fn find_symbol_and_loc<'a, O: Object<'a, 'a> + 'a>(obj: O, context: &Context<EndianRcSlice<RunTimeEndian>>, addr: u64) -> Result<Option<(Option<String>, Option<String>, Option<u32>)>, String> {
-//    let symbols = obj.symbol_map();
-//    let fname = symbols.get(addr).map(|sym| sym.name().to_string());
-//
-//    let location = context.find_location(addr)
-//        .map_err(|err| err.to_string())?;
-//    let file = location.as_ref().and_then(|location| location.file).map(|file| file.to_string());
-//    let line = location.as_ref().and_then(|location| location.line);
-//
-//    Ok(fname.map(|fname| (Some(fname), file, line)))
-//}


Test patch: