From 0501675d3613db3198548de0b1b4c16ef8c0e7f0 Mon Sep 17 00:00:00 2001 From: Javier Honduvilla Coto Date: Mon, 17 Jun 2024 11:05:34 +0100 Subject: [PATCH] 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 | 1 + src/profile.rs | 44 ++++++++++++++++++++++++++------------------ src/profiler.rs | 48 ++++++++++++++++++++++++++++++++++++++---------- src/usym.rs | 29 +++++++++++++++++++++++------ 4 files changed, 88 insertions(+), 34 deletions(-) diff --git a/src/main.rs b/src/main.rs index dca2241..ceea848 100644 --- a/src/main.rs +++ b/src/main.rs @@ -223,6 +223,7 @@ fn main() -> Result<(), Box> { .clone() .into_iter() .rev() + .map(|e| e.to_string()) .collect::>(); let ustack = ustack.join(";"); let kstack = sample diff --git a/src/profile.rs b/src/profile.rs index 0400f49..f637ff5 100644 --- a/src/profile.rs +++ b/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 = KsymIter::from_kallsyms().collect(); + let ksyms = KsymIter::from_kallsyms().collect::>(); 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, objs: &HashMap, -) -> HashMap>> { - let mut addresses_per_sample: HashMap>> = HashMap::new(); +) -> HashMap>> { + let mut addresses_per_sample: HashMap>> = 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!["".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>>, + addresses_per_sample: &HashMap>>, procs: &HashMap, objs: &HashMap, pid: i32, native_stack: &native_stack_t, -) -> Vec { +) -> Vec { let mut r = Vec::new(); for (i, addr) in native_stack.addresses.into_iter().enumerate() { @@ -149,25 +154,28 @@ fn symbolize_native_stack( } let Some(info) = procs.get(&pid) else { - r.push("".to_string()); + r.push(Frame::with_error("".to_string())); continue; }; let Some(mapping) = find_mapping(&info.mappings, addr) else { - r.push("".to_string()); + r.push(Frame::with_error("".to_string())); continue; }; // finally match objs.get(&mapping.executable_id) { Some(obj) => { - let failed_to_fetch_symbol = vec!["".to_string()]; - let failed_to_symbolize = vec!["".to_string()]; + let failed_to_fetch_symbol = vec![Frame::with_error( + "".to_string(), + )]; + let failed_to_symbolize = + vec![Frame::with_error("".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, @@ -175,12 +183,12 @@ fn symbolize_native_stack( 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"); } } } diff --git a/src/profiler.rs b/src/profiler.rs index e10ba9c..1291f7c 100644 --- a/src/profiler.rs +++ b/src/profiler.rs @@ -52,7 +52,6 @@ pub struct ProcessInfo { pub mappings: Vec, } -#[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, - pub kstack: Vec, + pub ustack: Vec, + pub kstack: Vec, 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 { + let format_symbolized_stack = |symbolized_stack: &Vec| -> 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 = vec![]; + let ustack_data = vec![]; let sample = SymbolizedAggregatedSample { pid: 98765, diff --git a/src/usym.rs b/src/usym.rs index 9b4ad35..6b04a46 100644 --- a/src/usym.rs +++ b/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, object_path: &PathBuf) -> Vec> { +pub fn symbolize_native_stack_blaze(addrs: Vec, object_path: &PathBuf) -> Vec> { 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, object_path: &PathBuf) -> V Err(e) => { res.resize( addrs.len(), - vec![format!(", 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!("", r)); + symbols.push(Frame { + address: 0x1111, + name: format!("", r), + inline: false, + }); } }