Add Frame structure (#48)

Add `Frame` structure and use it in the symbolized stack trace. This new
structure stores the address, function name, and whether the function is
inlined or not. Before that all this information might have been encoded
as a string, which wasn't ideal as in many cases we might want to do
something with these fields. For example, some formats, such as Google's
pprof might require the address and function name (if the profile is
symbolized).

This is also very useful for debugging as having the address readily
available can be important while troubleshooting issues.

Note that right now the user stack addresses are relative to the object
file, while the kernel addresses are absolute. This might be something
that changes in the future, if we do kernel symbolization in the
backend, for example.

Test Plan
=========

Current tests + got some profiles that looked good.

src/main.rs

@@ -223,6 +223,7 @@ fn main() -> Result<(), Box<dyn Error>> {
                 .clone()
                 .into_iter()
                 .rev()
+                .map(|e| e.to_string())
                 .collect::<Vec<String>>();
             let ustack = ustack.join(";");
             let kstack = sample

src/profile.rs

@@ -6,6 +6,7 @@ use crate::bpf::profiler_bindings::native_stack_t;
 use crate::ksym::KsymIter;
 use crate::object::ExecutableId;
 use crate::profiler::ExecutableMapping;
+use crate::profiler::Frame;
 use crate::profiler::ObjectFileInfo;
 use crate::profiler::ProcessInfo;
 use crate::profiler::RawAggregatedProfile;
@@ -22,7 +23,7 @@ pub fn symbolize_profile(
     let mut r = SymbolizedAggregatedProfile::new();
     let addresses_per_sample = fetch_symbols_for_profile(profile, procs, objs);
 
-    let ksyms: Vec<crate::ksym::Ksym> = KsymIter::from_kallsyms().collect();
+    let ksyms = KsymIter::from_kallsyms().collect::<Vec<_>>();
 
     for sample in profile {
         debug!("--- raw sample:\n{}", sample);
@@ -58,7 +59,11 @@ pub fn symbolize_profile(
                         }
                     }
                 };
-                symbolized_sample.kstack.push(le_symbol.symbol_name);
+                symbolized_sample.kstack.push(Frame {
+                    name: le_symbol.symbol_name,
+                    address: addr,
+                    inline: false,
+                });
             }
         };
         r.push(symbolized_sample);
@@ -81,8 +86,8 @@ fn fetch_symbols_for_profile(
     profile: &RawAggregatedProfile,
     procs: &HashMap<i32, ProcessInfo>,
     objs: &HashMap<ExecutableId, ObjectFileInfo>,
-) -> HashMap<PathBuf, HashMap<u64, Vec<String>>> {
-    let mut addresses_per_sample: HashMap<PathBuf, HashMap<u64, Vec<String>>> = HashMap::new();
+) -> HashMap<PathBuf, HashMap<u64, Vec<Frame>>> {
+    let mut addresses_per_sample: HashMap<PathBuf, HashMap<u64, Vec<Frame>>> = HashMap::new();
 
     for sample in profile {
         let Some(native_stack) = sample.ustack else {
@@ -110,10 +115,10 @@ fn fetch_symbols_for_profile(
                         - obj.load_offset
                         + obj.load_vaddr;
 
-                    let key = obj.path.clone();
-                    let addrs = addresses_per_sample.entry(key).or_default();
-                    addrs.insert(normalized_addr, vec!["<default>".to_string()]);
-                    // <- default value is a bit janky
+                    addresses_per_sample
+                        .entry(obj.path.clone())
+                        .or_default()
+                        .insert(normalized_addr, vec![]);
                 }
                 None => {
                     error!("executable with id {} not found", mapping.executable_id);
@@ -135,12 +140,12 @@ fn fetch_symbols_for_profile(
 }
 
 fn symbolize_native_stack(
-    addresses_per_sample: &HashMap<PathBuf, HashMap<u64, Vec<String>>>,
+    addresses_per_sample: &HashMap<PathBuf, HashMap<u64, Vec<Frame>>>,
     procs: &HashMap<i32, ProcessInfo>,
     objs: &HashMap<ExecutableId, ObjectFileInfo>,
     pid: i32,
     native_stack: &native_stack_t,
-) -> Vec<String> {
+) -> Vec<Frame> {
     let mut r = Vec::new();
 
     for (i, addr) in native_stack.addresses.into_iter().enumerate() {
@@ -149,38 +154,41 @@ fn symbolize_native_stack(
         }
 
         let Some(info) = procs.get(&pid) else {
-            r.push("<could not find process>".to_string());
+            r.push(Frame::with_error("<could not find process>".to_string()));
             continue;
         };
 
         let Some(mapping) = find_mapping(&info.mappings, addr) else {
-            r.push("<could not find mapping>".to_string());
+            r.push(Frame::with_error("<could not find mapping>".to_string()));
             continue;
         };
 
         // finally
         match objs.get(&mapping.executable_id) {
             Some(obj) => {
-                let failed_to_fetch_symbol = vec!["<failed to fetch symbol for addr>".to_string()];
-                let failed_to_symbolize = vec!["<failed to symbolize>".to_string()];
+                let failed_to_fetch_symbol = vec![Frame::with_error(
+                    "<failed to fetch symbol for addr>".to_string(),
+                )];
+                let failed_to_symbolize =
+                    vec![Frame::with_error("<failed to symbolize>".to_string())];
 
                 let normalized_addr =
                     addr - mapping.start_addr + mapping.offset - obj.load_offset + obj.load_vaddr;
 
-                let func_names = match addresses_per_sample.get(&obj.path) {
+                let frames = match addresses_per_sample.get(&obj.path) {
                     Some(value) => match value.get(&normalized_addr) {
                         Some(v) => v,
                         None => &failed_to_fetch_symbol,
                     },
                     None => &failed_to_symbolize,
                 };
 
-                for func_name in func_names {
-                    r.push(func_name.clone());
+                for frame in frames {
+                    r.push(frame.clone());
                 }
             }
             None => {
-                debug!("build id not found");
+                debug!("executable id not found");
             }
         }
     }

src/profiler.rs

@@ -52,7 +52,6 @@ pub struct ProcessInfo {
     pub mappings: Vec<ExecutableMapping>,
 }
 
-#[allow(dead_code)]
 pub struct ObjectFileInfo {
     pub file: fs::File,
     pub path: PathBuf,
@@ -70,7 +69,6 @@ pub enum Unwinder {
 }
 
 #[derive(Debug, Clone)]
-#[allow(dead_code)]
 pub struct ExecutableMapping {
     pub executable_id: ExecutableId,
     // No build id means either JIT or that we could not fetch it. Change this.
@@ -162,7 +160,6 @@ const MAX_CHUNKS: usize = MAX_UNWIND_TABLE_CHUNKS as usize;
 // TODO: should make this configurable via a command line argument in future
 const PERF_BUFFER_BYTES: usize = 512 * 1024;
 
-#[allow(dead_code)]
 #[derive(Debug)]
 pub struct RawAggregatedSample {
     pub pid: i32,
@@ -200,19 +197,42 @@ impl fmt::Display for RawAggregatedSample {
     }
 }
 
+#[derive(Debug, Clone)]
+pub struct Frame {
+    pub address: u64,
+    pub name: String,
+    pub inline: bool,
+}
+
+impl fmt::Display for Frame {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        let inline_str = if self.inline { "[inlined] " } else { "" };
+        write!(fmt, "{}{}", inline_str, self.name)
+    }
+}
+
+impl Frame {
+    pub fn with_error(msg: String) -> Self {
+        Self {
+            address: 0xBAD,
+            name: msg,
+            inline: false,
+        }
+    }
+}
+
 #[derive(Default, Debug)]
-#[allow(dead_code)]
 pub struct SymbolizedAggregatedSample {
     pub pid: i32,
     pub tid: i32,
-    pub ustack: Vec<String>,
-    pub kstack: Vec<String>,
+    pub ustack: Vec<Frame>,
+    pub kstack: Vec<Frame>,
     pub count: u64,
 }
 
 impl fmt::Display for SymbolizedAggregatedSample {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let format_symbolized_stack = |symbolized_stack: &Vec<String>| -> String {
+        let format_symbolized_stack = |symbolized_stack: &Vec<Frame>| -> String {
             let mut res = vec![];
             if symbolized_stack.is_empty() {
                 res.push("NONE".to_string());
@@ -1361,11 +1381,19 @@ mod tests {
     fn display_symbolized_aggregated_sample() {
         let ustack_data: Vec<_> = ["ufunc3", "ufunc2", "ufunc1"]
             .into_iter()
-            .map(|s| s.to_string())
+            .map(|s| Frame {
+                address: 0x0,
+                name: s.to_string(),
+                inline: false,
+            })
             .collect();
         let kstack_data: Vec<_> = ["kfunc2", "kfunc1"]
             .into_iter()
-            .map(|s| s.to_string())
+            .map(|s| Frame {
+                address: 0x0,
+                name: s.to_string(),
+                inline: false,
+            })
             .collect();
 
         let sample = SymbolizedAggregatedSample {
@@ -1380,7 +1408,7 @@ mod tests {
         "SymbolizedAggregatedSample { pid: 1234567, tid: 1234568, ustack: \"[  0: ufunc3,  1: ufunc2,  2: ufunc1]\", kstack: \"[  0: kfunc2,  1: kfunc1]\", count: 128 }"
         "###);
 
-        let ustack_data: Vec<String> = vec![];
+        let ustack_data = vec![];
 
         let sample = SymbolizedAggregatedSample {
             pid: 98765,

src/usym.rs

@@ -8,9 +8,11 @@ use blazesym::symbolize::Sym;
 use blazesym::symbolize::Symbolized;
 use blazesym::symbolize::Symbolizer;
 
+use crate::profiler::Frame;
+
 const ADDR2LINE_BIN: &str = "/usr/bin/addr2line";
 
-pub fn symbolize_native_stack_blaze(addrs: Vec<u64>, object_path: &PathBuf) -> Vec<Vec<String>> {
+pub fn symbolize_native_stack_blaze(addrs: Vec<u64>, object_path: &PathBuf) -> Vec<Vec<Frame>> {
     let mut res = Vec::new();
 
     let src = Source::Elf(Elf::new(object_path));
@@ -20,7 +22,10 @@ pub fn symbolize_native_stack_blaze(addrs: Vec<u64>, object_path: &PathBuf) -> V
         Err(e) => {
             res.resize(
                 addrs.len(),
-                vec![format!("<blazesym: failed to symbolize due to {}", e)],
+                vec![Frame::with_error(format!(
+                    "<blazesym: failed to symbolize due to {}",
+                    e
+                ))],
             );
             return res;
         }
@@ -32,20 +37,32 @@ pub fn symbolize_native_stack_blaze(addrs: Vec<u64>, object_path: &PathBuf) -> V
         match symbol {
             Symbolized::Sym(Sym {
                 name,
-                addr: _,
+                addr,
                 offset: _,
                 code_info: _,
                 inlined,
                 ..
             }) => {
-                symbols.push(name.to_string());
+                symbols.push(Frame {
+                    address: addr,
+                    name: name.to_string(),
+                    inline: false,
+                });
 
                 for frame in inlined.iter() {
-                    symbols.push(format!("{} (inlined)", frame.name));
+                    symbols.push(Frame {
+                        address: addr,
+                        name: frame.name.to_string(),
+                        inline: true,
+                    });
                 }
             }
             Symbolized::Unknown(r) => {
-                symbols.push(format!("<blazesym: unknown symbol due to {}>", r));
+                symbols.push(Frame {
+                    address: 0x1111,
+                    name: format!("<blazesym: unknown symbol due to {}>", r),
+                    inline: false,
+                });
             }
         }