From 7f9ec033d923b6ec59bf0feb5f82532d17f0ad98 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 14 Jun 2024 17:28:06 -0400 Subject: [PATCH] [crashtracker] Take relative address and build id for remote symbolication (#473) --- crashtracker/src/crash_info.rs | 21 ++++-- crashtracker/src/lib.rs | 2 +- crashtracker/src/stacktrace.rs | 54 +++++++++++++- profiling-ffi/src/crashtracker/crash_info.rs | 20 ++++++ profiling-ffi/src/crashtracker/datatypes.rs | 74 +++++++++++++++++++- 5 files changed, 161 insertions(+), 10 deletions(-) diff --git a/crashtracker/src/crash_info.rs b/crashtracker/src/crash_info.rs index bd6639310..92ce76f50 100644 --- a/crashtracker/src/crash_info.rs +++ b/crashtracker/src/crash_info.rs @@ -5,8 +5,6 @@ use crate::stacktrace::StackFrame; use crate::telemetry::TelemetryCrashUploader; use crate::CrashtrackerConfiguration; use anyhow::Context; -#[cfg(unix)] -use blazesym::symbolize::{Process, Source, Symbolizer}; use chrono::{DateTime, Utc}; use ddcommon::tag::Tag; use serde::{Deserialize, Serialize}; @@ -102,8 +100,19 @@ impl Default for CrashInfo { #[cfg(unix)] impl CrashInfo { - pub fn resolve_names(&mut self, src: &Source) -> anyhow::Result<()> { - let symbolizer = Symbolizer::new(); + pub fn normalize_ips(&mut self, pid: u32) -> anyhow::Result<()> { + let normalizer = blazesym::normalize::Normalizer::new(); + let pid = pid.into(); + self.stacktrace.iter_mut().for_each(|frame| { + frame + .normalize_ip(&normalizer, pid) + .unwrap_or_else(|err| eprintln!("Error resolving name {err}")) + }); + Ok(()) + } + + pub fn resolve_names(&mut self, src: &blazesym::symbolize::Source) -> anyhow::Result<()> { + let symbolizer = blazesym::symbolize::Symbolizer::new(); for frame in &mut self.stacktrace { // Resolving names is best effort, just print the error and continue frame @@ -114,10 +123,10 @@ impl CrashInfo { } pub fn resolve_names_from_process(&mut self, pid: u32) -> anyhow::Result<()> { - let mut process = Process::new(pid.into()); + let mut process = blazesym::symbolize::Process::new(pid.into()); // https://github.com/libbpf/blazesym/issues/518 process.map_files = false; - let src = Source::Process(process); + let src = blazesym::symbolize::Source::Process(process); self.resolve_names(&src) } } diff --git a/crashtracker/src/lib.rs b/crashtracker/src/lib.rs index d49b6c9e5..1574273ed 100644 --- a/crashtracker/src/lib.rs +++ b/crashtracker/src/lib.rs @@ -69,4 +69,4 @@ pub use crash_handler::{update_config, update_metadata}; pub use crash_info::*; #[cfg(unix)] pub use receiver::{receiver_entry_point_stdin, reciever_entry_point_unix_socket}; -pub use stacktrace::{StackFrame, StackFrameNames}; +pub use stacktrace::{NormalizedAddress, NormalizedAddressMeta, StackFrame, StackFrameNames}; diff --git a/crashtracker/src/stacktrace.rs b/crashtracker/src/stacktrace.rs index 218cd7ceb..333565a34 100644 --- a/crashtracker/src/stacktrace.rs +++ b/crashtracker/src/stacktrace.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use serde::{Deserialize, Serialize}; +use std::path::PathBuf; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Default)] pub struct StackFrameNames { @@ -33,16 +34,54 @@ pub struct StackFrame { pub names: Option>, #[serde(skip_serializing_if = "Option::is_none")] #[serde(default)] + pub normalized_ip: Option, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] pub sp: Option, #[serde(skip_serializing_if = "Option::is_none")] #[serde(default)] pub symbol_address: Option, } +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub enum NormalizedAddressMeta { + Apk(PathBuf), + Elf { + path: PathBuf, + build_id: Option>, + }, + Unknown, + Unexpected(String), +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct NormalizedAddress { + pub file_offset: u64, + pub meta: NormalizedAddressMeta, +} + #[cfg(unix)] mod unix { use super::*; - use blazesym::symbolize::{Input, Source, Sym, Symbolized, Symbolizer}; + use blazesym::{ + normalize::{Normalizer, UserMeta}, + symbolize::{Input, Source, Sym, Symbolized, Symbolizer}, + Pid, + }; + + impl From<&UserMeta> for NormalizedAddressMeta { + fn from(value: &UserMeta) -> Self { + match value { + UserMeta::Apk(a) => Self::Apk(a.path.clone()), + UserMeta::Elf(e) => Self::Elf { + path: e.path.clone(), + build_id: e.build_id.clone(), + }, + UserMeta::Unknown(_) => Self::Unknown, + _ => Self::Unexpected(format!("{value:?}")), + } + } + } impl From> for StackFrameNames { fn from(value: Sym) -> Self { @@ -58,6 +97,19 @@ mod unix { } impl StackFrame { + pub fn normalize_ip(&mut self, normalizer: &Normalizer, pid: Pid) -> anyhow::Result<()> { + if let Some(ip) = &self.ip { + let ip = ip.trim_start_matches("0x"); + let ip = u64::from_str_radix(ip, 16)?; + let normed = normalizer.normalize_user_addrs(pid, &[ip])?; + anyhow::ensure!(normed.outputs.len() == 1); + let (file_offset, meta_idx) = normed.outputs[0]; + let meta = (&normed.meta[meta_idx]).into(); + self.normalized_ip = Some(NormalizedAddress { file_offset, meta }); + } + Ok(()) + } + pub fn resolve_names( &mut self, src: &Source, diff --git a/profiling-ffi/src/crashtracker/crash_info.rs b/profiling-ffi/src/crashtracker/crash_info.rs index b9b24f486..93dfd4472 100644 --- a/profiling-ffi/src/crashtracker/crash_info.rs +++ b/profiling-ffi/src/crashtracker/crash_info.rs @@ -31,6 +31,26 @@ pub unsafe extern "C" fn ddog_crashinfo_drop(crashinfo: *mut CrashInfo) { } } +/// Best effort attempt to normalize all `ip` on the stacktrace. +/// `pid` must be the pid of the currently active process where the ips came from. +/// +/// # Safety +/// `crashinfo` must be a valid pointer to a `CrashInfo` object. +#[cfg(unix)] +#[no_mangle] +#[must_use] +pub unsafe extern "C" fn ddog_crashinfo_normalize_ips( + crashinfo: *mut CrashInfo, + pid: u32, +) -> CrashtrackerResult { + (|| { + let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; + crashinfo.normalize_ips(pid) + })() + .context("ddog_crashinfo_normalize_ips failed") + .into() +} + /// Adds a "counter" variable, with the given value. Useful for determining if /// "interesting" operations were occurring when the crash did. /// diff --git a/profiling-ffi/src/crashtracker/datatypes.rs b/profiling-ffi/src/crashtracker/datatypes.rs index a7b42be95..0912f5a41 100644 --- a/profiling-ffi/src/crashtracker/datatypes.rs +++ b/profiling-ffi/src/crashtracker/datatypes.rs @@ -4,7 +4,7 @@ use crate::exporter::{self, ProfilingEndpoint}; pub use datadog_crashtracker::{ProfilingOpTypes, StacktraceCollection}; use ddcommon::tag::Tag; -use ddcommon_ffi::slice::{AsBytes, CharSlice}; +use ddcommon_ffi::slice::{AsBytes, ByteSlice, CharSlice}; use ddcommon_ffi::{Error, Slice, StringWrapper}; use std::ops::Not; use std::time::Duration; @@ -263,6 +263,73 @@ impl From> for CrashtrackerResult { } } +#[repr(C)] +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum NormalizedAddressTypes { + // Make None 0 so that default construction gives none + None = 0, + Elf, +} + +#[repr(C)] +pub struct NormalizedAddress<'a> { + file_offset: u64, + build_id: ByteSlice<'a>, + path: CharSlice<'a>, + typ: NormalizedAddressTypes, +} + +impl<'a> TryFrom> for Option { + type Error = anyhow::Error; + + fn try_from(value: NormalizedAddress<'a>) -> Result { + Self::try_from(&value) + } +} + +impl<'a> TryFrom<&NormalizedAddress<'a>> for Option { + type Error = anyhow::Error; + + fn try_from(value: &NormalizedAddress<'a>) -> Result { + if value.typ == NormalizedAddressTypes::None { + Ok(None) + } else { + Ok(Some(value.try_into()?)) + } + } +} + +impl<'a> TryFrom> for datadog_crashtracker::NormalizedAddress { + type Error = anyhow::Error; + + fn try_from(value: NormalizedAddress<'a>) -> Result { + Self::try_from(&value) + } +} + +impl<'a> TryFrom<&NormalizedAddress<'a>> for datadog_crashtracker::NormalizedAddress { + type Error = anyhow::Error; + + fn try_from(value: &NormalizedAddress<'a>) -> Result { + match &value.typ { + NormalizedAddressTypes::Elf => { + let build_id = if value.build_id.is_empty() { + None + } else { + Some(Vec::from(value.build_id.as_bytes())) + }; + let path = value.path.try_to_utf8()?.into(); + let meta = datadog_crashtracker::NormalizedAddressMeta::Elf { build_id, path }; + Ok(Self { + file_offset: value.file_offset, + meta, + }) + } + _ => anyhow::bail!("Unsupported normalized address type {:?}", value.typ), + } + } +} + #[repr(C)] pub struct StackFrameNames<'a> { colno: ddcommon_ffi::Option, @@ -298,9 +365,11 @@ impl<'a> TryFrom<&StackFrameNames<'a>> for datadog_crashtracker::StackFrameNames #[repr(C)] pub struct StackFrame<'a> { + build_id: CharSlice<'a>, ip: usize, module_base_address: usize, names: Slice<'a, StackFrameNames<'a>>, + normalized_ip: NormalizedAddress<'a>, sp: usize, symbol_address: usize, } @@ -316,7 +385,6 @@ impl<'a> TryFrom<&StackFrame<'a>> for datadog_crashtracker::StackFrame { Some(format!("{v:#X}")) } } - let ip = to_hex(value.ip); let module_base_address = to_hex(value.module_base_address); let names = if value.names.is_empty() { @@ -328,12 +396,14 @@ impl<'a> TryFrom<&StackFrame<'a>> for datadog_crashtracker::StackFrame { } Some(vec) }; + let normalized_ip = (&value.normalized_ip).try_into()?; let sp = to_hex(value.sp); let symbol_address = to_hex(value.symbol_address); Ok(Self { ip, module_base_address, names, + normalized_ip, sp, symbol_address, })