[crashtracker] Demangle function names (#1031)

Author: danielsn

Cargo.lock

@@ -72,6 +72,7 @@ mod unix {
             crashtracker::default_signals(),
             TEST_COLLECTOR_TIMEOUT_MS,
             Some("".to_string()),
+            true,
         )?;
 
         let metadata = Metadata {

bin_tests/src/bin/crashtracker_bin_test.rs

@@ -321,7 +321,7 @@ fn assert_telemetry_message(crash_telemetry: &[u8], crash_typ: &str) {
 
     let base_expected_tags: std::collections::HashSet<&str> =
         std::collections::HashSet::from_iter([
-            "data_schema_version:1.3",
+            "data_schema_version:1.4",
             "incomplete:false",
             "is_crash:true",
             "profiler_collecting_sample:1",

bin_tests/tests/crashtracker_bin_test.rs

@@ -57,10 +57,12 @@ impl<'a> TryFrom<ReceiverConfig<'a>> for datadog_crashtracker::CrashtrackerRecei
 pub struct Config<'a> {
     pub additional_files: Slice<'a, CharSlice<'a>>,
     pub create_alt_stack: bool,
-    pub use_alt_stack: bool,
+    pub demangle_names: bool,
     /// The endpoint to send the crash report to (can be a file://).
     /// If None, the crashtracker will infer the agent host from env variables.
     pub endpoint: Option<&'a Endpoint>,
+    /// Optional filename for a unix domain socket if the receiver is used asynchonously
+    pub optional_unix_socket_filename: CharSlice<'a>,
     pub resolve_frames: StacktraceCollection,
     /// The set of signals we should be registered for.
     /// If empty, use the default set.
@@ -69,8 +71,7 @@ pub struct Config<'a> {
     /// This is given as a uint32_t, but the actual timeout needs to fit inside of an i32 (max
     /// 2^31-1). This is a limitation of the various interfaces used to guarantee the timeout.
     pub timeout_ms: u32,
-    /// Optional filename for a unix domain socket if the receiver is used asynchonously
-    pub optional_unix_socket_filename: CharSlice<'a>,
+    pub use_alt_stack: bool,
 }
 
 impl<'a> TryFrom<Config<'a>> for datadog_crashtracker::CrashtrackerConfiguration {
@@ -90,6 +91,7 @@ impl<'a> TryFrom<Config<'a>> for datadog_crashtracker::CrashtrackerConfiguration
         let signals = value.signals.iter().copied().collect();
         let timeout_ms = value.timeout_ms;
         let unix_socket_path = value.optional_unix_socket_filename.try_to_string_option()?;
+        let demangle_names = value.demangle_names;
         Self::new(
             additional_files,
             create_alt_stack,
@@ -99,6 +101,7 @@ impl<'a> TryFrom<Config<'a>> for datadog_crashtracker::CrashtrackerConfiguration
             signals,
             timeout_ms,
             unix_socket_path,
+            demangle_names,
         )
     }
 }

datadog-crashtracker-ffi/src/collector/datatypes.rs

@@ -6,6 +6,20 @@ use ddcommon::Endpoint;
 use ddcommon_ffi::{wrap_with_void_ffi_result, Handle, ToInner, VoidResult};
 use function_name::named;
 
+/// # Safety
+/// The `crash_info` can be null, but if non-null it must point to a Builder made by this module,
+/// which has not previously been dropped.
+#[no_mangle]
+#[must_use]
+#[named]
+pub unsafe extern "C" fn ddog_crasht_CrashInfo_demangle_names(
+    mut crash_info: *mut Handle<CrashInfo>,
+) -> VoidResult {
+    wrap_with_void_ffi_result!({
+        crash_info.to_inner_mut()?.demangle_names()?;
+    })
+}
+
 /// # Safety
 /// The `builder` can be null, but if non-null it must point to a Frame
 /// made by this module, which has not previously been dropped.

datadog-crashtracker-ffi/src/crash_info/api.rs

@@ -47,6 +47,8 @@ rand = "0.8.5"
 schemars = "0.8.21"
 serde = {version = "1.0", features = ["derive"]}
 serde_json = {version = "1.0"}
+symbolic-demangle = { version = "12.8.0", default-features = false, features = ["rust", "cpp", "msvc"] }
+symbolic-common = { version = "12.8.0", default-features = false }
 tokio = { version = "1.23", features = ["rt", "macros", "io-std", "io-util"] }
 uuid = { version = "1.4.1", features = ["v4", "serde"] }
 

datadog-crashtracker/Cargo.toml

@@ -139,6 +139,7 @@ fn test_crash() -> anyhow::Result<()> {
         default_signals(),
         timeout_ms,
         None,
+        true,
     )?;
     let metadata = Metadata::new(
         "libname".to_string(),
@@ -197,6 +198,7 @@ fn test_altstack_paradox() -> anyhow::Result<()> {
         default_signals(),
         timeout_ms,
         None,
+        true,
     );
 
     // This is slightly over-tuned to the language of the error message, but it'd require some
@@ -267,6 +269,7 @@ fn test_altstack_use_create() -> anyhow::Result<()> {
         signals,
         timeout_ms,
         None,
+        true,
     )?;
     let metadata = Metadata::new(
         "libname".to_string(),
@@ -392,6 +395,7 @@ fn test_altstack_use_nocreate() -> anyhow::Result<()> {
         signals,
         timeout_ms,
         None,
+        true,
     )?;
     let metadata = Metadata::new(
         "libname".to_string(),
@@ -521,6 +525,7 @@ fn test_altstack_nouse() -> anyhow::Result<()> {
         signals,
         timeout_ms,
         None,
+        true,
     )?;
     let metadata = Metadata::new(
         "libname".to_string(),
@@ -685,6 +690,7 @@ fn test_waitall_nohang() -> anyhow::Result<()> {
         signals,
         timeout_ms,
         None,
+        true,
     )?;
 
     let metadata = Metadata::new(

datadog-crashtracker/src/collector/api.rs

@@ -68,6 +68,24 @@ impl ErrorData {
     }
 }
 
+impl ErrorData {
+    pub fn demangle_names(&mut self) -> anyhow::Result<()> {
+        let mut errors = 0;
+        self.stack.demangle_names().unwrap_or_else(|_| errors += 1);
+        for thread in &mut self.threads {
+            thread
+                .stack
+                .demangle_names()
+                .unwrap_or_else(|_| errors += 1);
+        }
+        anyhow::ensure!(
+            errors == 0,
+            "Failed to demangle names, see frame comments for details"
+        );
+        Ok(())
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)]
 pub enum SourceType {
     Crashtracking,

datadog-crashtracker/src/crash_info/error_data.rs

@@ -62,7 +62,11 @@ pub struct CrashInfo {
 
 impl CrashInfo {
     pub fn current_schema_version() -> String {
-        "1.3".to_string()
+        "1.4".to_string()
+    }
+
+    pub fn demangle_names(&mut self) -> anyhow::Result<()> {
+        self.error.demangle_names()
     }
 }
 
@@ -131,7 +135,7 @@ mod tests {
     fn test_schema_matches_rfc() {
         let rfc_schema_filename = concat!(
             env!("CARGO_MANIFEST_DIR"),
-            "/../docs/RFCs/artifacts/0008-crashtracker-schema.json"
+            "/../docs/RFCs/artifacts/0009-crashtracker-schema.json"
         );
         let rfc_schema_json = fs::read_to_string(rfc_schema_filename).expect("File to exist");
         let rfc_schema: RootSchema = serde_json::from_str(&rfc_schema_json).expect("Valid json");

datadog-crashtracker/src/crash_info/mod.rs

@@ -10,6 +10,8 @@ use blazesym::{
 };
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
+use symbolic_common::Name;
+use symbolic_demangle::{Demangle, DemangleOptions};
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)]
 pub struct StackTrace {
@@ -69,6 +71,18 @@ impl StackTrace {
         self.incomplete = incomplete;
         Ok(())
     }
+
+    pub fn demangle_names(&mut self) -> anyhow::Result<()> {
+        let mut errors = 0;
+        for frame in &mut self.frames {
+            frame.demangle_name().unwrap_or_else(|e| {
+                frame.comments.push(e.to_string());
+                errors += 1;
+            });
+        }
+        anyhow::ensure!(errors == 0);
+        Ok(())
+    }
 }
 
 #[cfg(unix)]
@@ -137,6 +151,8 @@ pub struct StackFrame {
     pub function: Option<String>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub line: Option<u32>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub mangled_name: Option<String>,
 
     // Additional Info
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
@@ -198,6 +214,23 @@ impl StackFrame {
     }
 }
 
+impl StackFrame {
+    pub fn demangle_name(&mut self) -> anyhow::Result<()> {
+        if let Some(name) = self.function.take() {
+            match Name::from(&name).demangle(DemangleOptions::name_only()) {
+                Some(demangled) if demangled != name => {
+                    self.mangled_name = Some(name);
+                    self.function = Some(demangled.to_string());
+                }
+                _ => {
+                    self.function = Some(name);
+                }
+            }
+        }
+        Ok(())
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)]
 #[allow(clippy::upper_case_acronyms)]
 #[repr(C)]
@@ -253,6 +286,7 @@ impl super::test_utils::TestInstance for StackFrame {
         let column = Some(2 * seed as u32);
         let file = Some(format!("banana{seed}.rs"));
         let function = Some(format!("Bar::baz{seed}"));
+        let mangled_name = Some(format!("_ZN3Bar3baz{seed}E"));
         let line = Some((2 * seed + 1) as u32);
 
         let comments = vec![format!("This is a comment on frame {seed}")];
@@ -269,8 +303,70 @@ impl super::test_utils::TestInstance for StackFrame {
             column,
             file,
             function,
+            mangled_name,
             line,
             comments,
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_demangle_rust() {
+        let mut frame = StackFrame::new();
+        frame.function = Some("_ZN3std2rt10lang_start17h7a87e81ecc4a9d6cE".to_string());
+        frame.demangle_name().unwrap();
+        assert_eq!(frame.function, Some("std::rt::lang_start".to_string()));
+        assert_eq!(
+            frame.mangled_name,
+            Some("_ZN3std2rt10lang_start17h7a87e81ecc4a9d6cE".to_string())
+        );
+    }
+
+    #[test]
+    fn test_demangle_cpp() {
+        let mut frame = StackFrame::new();
+        frame.function = Some("_ZN3Foo3barEv".to_string());
+        frame.demangle_name().unwrap();
+        assert_eq!(frame.function, Some("Foo::bar".to_string()));
+        assert_eq!(frame.mangled_name, Some("_ZN3Foo3barEv".to_string()));
+    }
+
+    #[test]
+    fn test_demangle_msvc() {
+        let mut frame = StackFrame::new();
+        frame.function = Some("?bar@Foo@@QEAAXXZ".to_string());
+        frame.demangle_name().unwrap();
+        assert_eq!(frame.function, Some("Foo::bar".to_string()));
+        assert_eq!(frame.mangled_name, Some("?bar@Foo@@QEAAXXZ".to_string()));
+    }
+
+    #[test]
+    fn test_demangle_unmangled() {
+        let mut frame = StackFrame::new();
+        frame.function = Some("main".to_string());
+        frame.demangle_name().unwrap();
+        assert_eq!(frame.function, Some("main".to_string()));
+        assert_eq!(frame.mangled_name, None);
+    }
+
+    #[test]
+    fn test_demangle_empty() {
+        let mut frame = StackFrame::new();
+        frame.demangle_name().unwrap();
+        assert_eq!(frame.function, None);
+        assert_eq!(frame.mangled_name, None);
+    }
+
+    #[test]
+    fn test_demangle_invalid() {
+        let mut frame = StackFrame::new();
+        frame.function = Some("invalid_mangled_name".to_string());
+        frame.demangle_name().unwrap();
+        assert_eq!(frame.function, Some("invalid_mangled_name".to_string()));
+        assert_eq!(frame.mangled_name, None);
+    }
+}