From 175369acf06fbe924edfc2453c7c555186e5bfdb Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Wed, 20 Nov 2024 17:55:58 -0500 Subject: [PATCH 01/36] [crashtracker] Add builder class for CrashInfo --- crashtracker/src/lib.rs | 2 +- crashtracker/src/rfc5_crash_info/builder.rs | 221 ++++++++++++++++++ crashtracker/src/rfc5_crash_info/metadata.rs | 13 ++ crashtracker/src/rfc5_crash_info/mod.rs | 20 +- crashtracker/src/rfc5_crash_info/os_info.rs | 8 + .../src/rfc5_crash_info/stacktrace.rs | 4 +- .../src/rfc5_crash_info/unknown_value.rs | 6 + 7 files changed, 266 insertions(+), 8 deletions(-) create mode 100644 crashtracker/src/rfc5_crash_info/builder.rs create mode 100644 crashtracker/src/rfc5_crash_info/unknown_value.rs diff --git a/crashtracker/src/lib.rs b/crashtracker/src/lib.rs index 3eef00a18..fd8989512 100644 --- a/crashtracker/src/lib.rs +++ b/crashtracker/src/lib.rs @@ -49,7 +49,7 @@ mod collector; mod crash_info; #[cfg(all(unix, feature = "receiver"))] mod receiver; -mod rfc5_crash_info; +pub mod rfc5_crash_info; #[cfg(all(unix, any(feature = "collector", feature = "receiver")))] mod shared; diff --git a/crashtracker/src/rfc5_crash_info/builder.rs b/crashtracker/src/rfc5_crash_info/builder.rs new file mode 100644 index 000000000..ed6864e0a --- /dev/null +++ b/crashtracker/src/rfc5_crash_info/builder.rs @@ -0,0 +1,221 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use chrono::{DateTime, Utc}; +use error_data::ThreadData; +use stacktrace::StackTrace; +use unknown_value::UnknownValue; +use uuid::Uuid; + +use super::*; + +#[derive(Debug, Default)] +pub struct ErrorDataBuilder { + pub kind: Option, + pub message: Option, + pub stack: Option, + pub threads: Option>, +} + +impl ErrorDataBuilder { + pub fn build(self) -> anyhow::Result<(ErrorData, bool /* incomplete */)> { + let incomplete = self.stack.is_none(); + let is_crash = true; + let kind = self.kind.context("required field 'kind' missing")?; + let message = self.message; + let source_type = SourceType::Crashtracking; + let stack = self.stack.unwrap_or(StackTrace { + format: "Missing Stacktrace".to_string(), + frames: vec![], + }); + let threads = self.threads.unwrap_or_default(); + Ok(( + ErrorData { + is_crash, + kind, + message, + source_type, + stack, + threads, + }, + incomplete, + )) + } + + pub fn new() -> Self { + Self::default() + } + + pub fn with_kind(&mut self, kind: ErrorKind) -> &mut Self { + self.kind = Some(kind); + self + } + + pub fn with_message(&mut self, message: String) -> &mut Self { + self.message = Some(message); + self + } + + pub fn with_stack(&mut self, stack: StackTrace) -> &mut Self { + self.stack = Some(stack); + self + } + + pub fn with_threads(&mut self, threads: Vec) -> &mut Self { + self.threads = Some(threads); + self + } +} + +#[derive(Debug, Default)] +pub struct CrashInfoBuilder { + pub counters: Option>, + pub error: ErrorDataBuilder, + pub files: Option>>, + pub fingerprint: Option, + pub incomplete: Option, + pub log_messages: Option>, + pub metadata: Option, + pub os_info: Option, + pub proc_info: Option, + pub sig_info: Option, + pub span_ids: Option>, + pub timestamp: Option>, + pub trace_ids: Option>, + pub uuid: Option, +} + +impl CrashInfoBuilder { + pub fn build(self) -> anyhow::Result { + let counters = self.counters.unwrap_or_default(); + let data_schema_version = CrashInfo::current_schema_version().to_string(); + let (error, incomplete_error) = self.error.build()?; + let files = self.files.unwrap_or_default(); + let fingerprint = self.fingerprint; + let incomplete = incomplete_error; // TODO + let log_messages = self.log_messages.unwrap_or_default(); + let metadata = self.metadata.unwrap_or_else(Metadata::unknown_value); + let os_info = self.os_info.unwrap_or_else(OsInfo::unknown_value); + let proc_info = self.proc_info; + let sig_info = self.sig_info; + let span_ids = self.span_ids.unwrap_or_default(); + let timestamp = self.timestamp.unwrap_or_else(Utc::now).to_string(); + let trace_ids = self.trace_ids.unwrap_or_default(); + let uuid = self.uuid.unwrap_or_else(|| Uuid::new_v4().to_string()); + Ok(CrashInfo { + counters, + data_schema_version, + error, + files, + fingerprint, + incomplete, + log_messages, + metadata, + os_info, + proc_info, + sig_info, + span_ids, + timestamp, + trace_ids, + uuid, + }) + } + + pub fn new() -> Self { + Self::default() + } + + pub fn with_counters(&mut self, counters: HashMap) -> &mut Self { + self.counters = Some(counters); + self + } + + pub fn with_kind(&mut self, kind: ErrorKind) -> &mut Self { + self.error.with_kind(kind); + self + } + + pub fn with_files(&mut self, files: HashMap>) -> &mut Self { + self.files = Some(files); + self + } + pub fn with_fingerprint(&mut self, fingerprint: String) -> &mut Self { + self.fingerprint = Some(fingerprint); + self + } + pub fn with_incomplete(&mut self, incomplete: bool) -> &mut Self { + self.incomplete = Some(incomplete); + self + } + pub fn with_log_messages(&mut self, log_messages: Vec) -> &mut Self { + self.log_messages = Some(log_messages); + self + } + + pub fn with_message(&mut self, message: String) -> &mut Self { + self.error.with_message(message); + self + } + + pub fn with_metadata(&mut self, metadata: Metadata) -> &mut Self { + self.metadata = Some(metadata); + self + } + + pub fn with_os_info(&mut self, os_info: OsInfo) -> &mut Self { + self.os_info = Some(os_info); + self + } + + pub fn with_os_info_this_machine(&mut self) -> &mut Self { + self.with_os_info(::os_info::get().into()) + } + + pub fn with_proc_info(&mut self, proc_info: ProcInfo) -> &mut Self { + self.proc_info = Some(proc_info); + self + } + + pub fn with_sig_info(&mut self, sig_info: SigInfo) -> &mut Self { + self.sig_info = Some(sig_info); + self + } + + pub fn with_span_ids(&mut self, span_ids: Vec) -> &mut Self { + self.span_ids = Some(span_ids); + self + } + + pub fn with_stack(&mut self, stack: StackTrace) -> &mut Self { + self.error.with_stack(stack); + self + } + + pub fn with_threads(&mut self, threads: Vec) -> &mut Self { + self.error.with_threads(threads); + self + } + + pub fn with_timestamp(&mut self, timestamp: DateTime) -> &mut Self { + self.timestamp = Some(timestamp); + self + } + + pub fn with_timestamp_now(&mut self) -> &mut Self { + self.with_timestamp(Utc::now()) + } + + pub fn with_trace_ids(&mut self, trace_ids: Vec) -> &mut Self { + self.trace_ids = Some(trace_ids); + self + } + + pub fn with_uuid(&mut self, uuid: String) -> &mut Self { + self.uuid = Some(uuid); + self + } + + pub fn with_uuid_random(&mut self) -> &mut Self { + self.with_uuid(Uuid::new_v4().to_string()) + } +} diff --git a/crashtracker/src/rfc5_crash_info/metadata.rs b/crashtracker/src/rfc5_crash_info/metadata.rs index 67836493b..79d819b12 100644 --- a/crashtracker/src/rfc5_crash_info/metadata.rs +++ b/crashtracker/src/rfc5_crash_info/metadata.rs @@ -3,6 +3,8 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use super::unknown_value::UnknownValue; + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] pub struct Metadata { pub library_name: String, @@ -13,6 +15,17 @@ pub struct Metadata { pub tags: Vec, } +impl UnknownValue for Metadata { + fn unknown_value() -> Self { + Self { + library_name: "unknown".to_string(), + library_version: "unknown".to_string(), + family: "unknown".to_string(), + tags: vec![], + } + } +} + impl From for Metadata { fn from(value: crate::crash_info::CrashtrackerMetadata) -> Self { let tags = value diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index e65bc6aeb..cf53dc093 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -1,6 +1,7 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 +mod builder; mod error_data; mod metadata; mod os_info; @@ -8,6 +9,9 @@ mod proc_info; mod sig_info; mod spans; mod stacktrace; +mod unknown_value; + +pub use builder::*; use anyhow::Context; use error_data::{thread_data_from_additional_stacktraces, ErrorData, ErrorKind, SourceType}; @@ -35,8 +39,8 @@ pub struct CrashInfo { pub log_messages: Vec, pub metadata: Metadata, pub os_info: OsInfo, - pub proc_info: ProcInfo, - pub sig_info: SigInfo, + pub proc_info: Option, //TODO, update the schema + pub sig_info: Option, //TODO, update the schema #[serde(default, skip_serializing_if = "Vec::is_empty")] pub span_ids: Vec, pub timestamp: String, @@ -45,10 +49,16 @@ pub struct CrashInfo { pub uuid: String, } +impl CrashInfo { + pub fn current_schema_version() -> String { + "1.0".to_string() + } +} + impl From for CrashInfo { fn from(value: crate::crash_info::CrashInfo) -> Self { let counters = value.counters; - let data_schema_version = String::from("1.0"); + let data_schema_version = CrashInfo::current_schema_version(); let error = { let is_crash = true; let kind = ErrorKind::UnixSignal; @@ -71,8 +81,8 @@ impl From for CrashInfo { let log_messages = vec![]; let metadata = value.metadata.unwrap().into(); let os_info = value.os_info.into(); - let proc_info = value.proc_info.unwrap().into(); - let sig_info = value.siginfo.unwrap().into(); + let proc_info = value.proc_info.map(ProcInfo::from); + let sig_info = value.siginfo.map(SigInfo::from); let span_ids = value .span_ids .into_iter() diff --git a/crashtracker/src/rfc5_crash_info/os_info.rs b/crashtracker/src/rfc5_crash_info/os_info.rs index 56dbf5e22..ee9d16fa8 100644 --- a/crashtracker/src/rfc5_crash_info/os_info.rs +++ b/crashtracker/src/rfc5_crash_info/os_info.rs @@ -3,6 +3,8 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use super::unknown_value::UnknownValue; + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] pub struct OsInfo { pub architecture: String, @@ -11,6 +13,12 @@ pub struct OsInfo { pub version: String, } +impl UnknownValue for OsInfo { + fn unknown_value() -> Self { + os_info::Info::unknown().into() + } +} + impl From for OsInfo { fn from(value: os_info::Info) -> Self { let architecture = value.architecture().unwrap_or("unknown").to_string(); diff --git a/crashtracker/src/rfc5_crash_info/stacktrace.rs b/crashtracker/src/rfc5_crash_info/stacktrace.rs index 2cc1f4a10..a8498d5ad 100644 --- a/crashtracker/src/rfc5_crash_info/stacktrace.rs +++ b/crashtracker/src/rfc5_crash_info/stacktrace.rs @@ -8,8 +8,8 @@ use crate::NormalizedAddress; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] pub struct StackTrace { - format: String, - frames: Vec, + pub format: String, + pub frames: Vec, } impl From> for StackTrace { diff --git a/crashtracker/src/rfc5_crash_info/unknown_value.rs b/crashtracker/src/rfc5_crash_info/unknown_value.rs new file mode 100644 index 000000000..ba2e42e82 --- /dev/null +++ b/crashtracker/src/rfc5_crash_info/unknown_value.rs @@ -0,0 +1,6 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +pub trait UnknownValue { + fn unknown_value() -> Self; +} From 8c2b85c2bd8cb55ee5c6f28f059fced286016abc Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Thu, 21 Nov 2024 18:02:10 -0500 Subject: [PATCH 02/36] wrap macro --- crashtracker-ffi/src/crash_info/stacktrace.rs | 270 ++++++++++++++++++ crashtracker-ffi/src/crash_info/to_inner.rs | 20 ++ 2 files changed, 290 insertions(+) create mode 100644 crashtracker-ffi/src/crash_info/stacktrace.rs create mode 100644 crashtracker-ffi/src/crash_info/to_inner.rs diff --git a/crashtracker-ffi/src/crash_info/stacktrace.rs b/crashtracker-ffi/src/crash_info/stacktrace.rs new file mode 100644 index 000000000..2e23ed116 --- /dev/null +++ b/crashtracker-ffi/src/crash_info/stacktrace.rs @@ -0,0 +1,270 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use std::ptr; + +use super::to_inner::ToInner; +use crate::{option_from_char_slice, Result}; +use ::function_name::named; +use anyhow::Context; +use ddcommon_ffi::{ + slice::{AsBytes, ByteSlice}, + CharSlice, Error, Slice, +}; + +/// Represents a StackFrame. Do not access its member for any reason, only use +/// the C API functions on this struct. +#[repr(C)] +pub struct StackFrame { + // This may be null, but if not it will point to a valid StackFrame. + inner: *mut datadog_crashtracker::rfc5_crash_info::StackFrame, +} + +impl ToInner for StackFrame { + type Inner = datadog_crashtracker::rfc5_crash_info::StackFrame; + + unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut Self::Inner> { + self.inner + .as_mut() + .context("inner pointer was null, indicates use after free") + } +} + +impl StackFrame { + pub(super) fn new(crash_info: datadog_crashtracker::rfc5_crash_info::StackFrame) -> Self { + StackFrame { + inner: Box::into_raw(Box::new(crash_info)), + } + } + + pub(super) fn take( + &mut self, + ) -> Option> { + // Leaving a null will help with double-free issues that can + // arise in C. Of course, it's best to never get there in the + // first place! + let raw = std::mem::replace(&mut self.inner, std::ptr::null_mut()); + + if raw.is_null() { + None + } else { + Some(unsafe { Box::from_raw(raw) }) + } + } +} + +impl Drop for StackFrame { + fn drop(&mut self) { + drop(self.take()) + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////// +// FFI API // +//////////////////////////////////////////////////////////////////////////////////////////////////// +/// Returned by [ddog_prof_Profile_new]. +#[repr(C)] +pub enum StackFrameNewResult { + Ok(StackFrame), + #[allow(dead_code)] + Err(Error), +} + +/// Create a new StackFrame, and returns an opaque reference to it. +/// # Safety +/// No safety issues. +#[no_mangle] +#[must_use] +pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> StackFrameNewResult { + StackFrameNewResult::Ok(StackFrame::new( + datadog_crashtracker::rfc5_crash_info::StackFrame::default(), + )) +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame +/// made by this module, which has not previously been dropped. +#[no_mangle] +pub unsafe extern "C" fn ddog_crasht_StackFrame_drop(frame: *mut StackFrame) { + // Technically, this function has been designed so if it's double-dropped + // then it's okay, but it's not something that should be relied on. + if !frame.is_null() { + drop((*frame).take()) + } +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip( + mut frame: *mut StackFrame, + ip: CharSlice, +) -> Result { + (|| { + let frame = frame.to_inner_mut()?; + frame.ip = option_from_char_slice(ip)?; + anyhow::Ok(()) + })() + .context(concat!(function_name!(), " failed")) + .into() +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address( + mut frame: *mut StackFrame, + module_base_address: CharSlice, +) -> Result { + (|| { + let frame = frame.to_inner_mut()?; + frame.module_base_address = option_from_char_slice(module_base_address)?; + anyhow::Ok(()) + })() + .context(concat!(function_name!(), " failed")) + .into() +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp( + mut frame: *mut StackFrame, + sp: CharSlice, +) -> Result { + (|| { + let frame = frame.to_inner_mut()?; + frame.sp = option_from_char_slice(sp)?; + anyhow::Ok(()) + })() + .context(concat!(function_name!(), " failed")) + .into() +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address( + mut frame: *mut StackFrame, + symbol_address: CharSlice, +) -> Result { + (|| { + let frame = frame.to_inner_mut()?; + frame.symbol_address = option_from_char_slice(symbol_address)?; + anyhow::Ok(()) + })() + .context(concat!(function_name!(), " failed")) + .into() +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( + mut frame: *mut StackFrame, + build_id: CharSlice, +) -> Result { + (|| { + let frame = frame.to_inner_mut()?; + frame.build_id = option_from_char_slice(build_id)?; + anyhow::Ok(()) + })() + .context(concat!(function_name!(), " failed")) + .into() +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path( + mut frame: *mut StackFrame, + path: CharSlice, +) -> Result { + (|| { + let frame = frame.to_inner_mut()?; + frame.path = option_from_char_slice(path)?; + anyhow::Ok(()) + })() + .context(concat!(function_name!(), " failed")) + .into() +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( + mut frame: *mut StackFrame, + relative_address: CharSlice, +) -> Result { + (|| { + let frame = frame.to_inner_mut()?; + frame.relative_address = option_from_char_slice(relative_address)?; + anyhow::Ok(()) + })() + .context(concat!(function_name!(), " failed")) + .into() +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_file( + mut frame: *mut StackFrame, + file: CharSlice, +) -> Result { + (|| { + let frame = frame.to_inner_mut()?; + frame.file = option_from_char_slice(file)?; + anyhow::Ok(()) + })() + .context(concat!(function_name!(), " failed")) + .into() +} +macro_rules! wrap { + ($body:expr) => { + (|| $body)() + .context(concat!(function_name!(), " failed")) + .into() + }; +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_function( + mut frame: *mut StackFrame, + function: CharSlice, +) -> Result { + wrap!({ + let frame = frame.to_inner_mut()?; + frame.function = option_from_char_slice(function)?; + anyhow::Ok(()) + }) +} diff --git a/crashtracker-ffi/src/crash_info/to_inner.rs b/crashtracker-ffi/src/crash_info/to_inner.rs new file mode 100644 index 000000000..5eb142426 --- /dev/null +++ b/crashtracker-ffi/src/crash_info/to_inner.rs @@ -0,0 +1,20 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use anyhow::Context; + +pub(crate) trait ToInner { + type Inner; + unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut Self::Inner>; +} + +impl ToInner for *mut T +where + T: ToInner, +{ + type Inner = T::Inner; + unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut Self::Inner> { + let inner = self.as_mut().context("Null pointer")?; + inner.to_inner_mut() + } +} From 55d6bc86b0f3771bc380977e3e0fbcaa20500654 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Thu, 21 Nov 2024 21:54:09 -0500 Subject: [PATCH 03/36] use the wrap macro more --- Cargo.lock | 16 ++++ crashtracker-ffi/Cargo.toml | 1 + crashtracker-ffi/src/crash_info/datatypes.rs | 6 +- crashtracker-ffi/src/crash_info/mod.rs | 5 +- crashtracker-ffi/src/crash_info/stacktrace.rs | 77 +++++++++---------- crashtracker/src/rfc5_crash_info/builder.rs | 3 + crashtracker/src/rfc5_crash_info/mod.rs | 1 + .../src/rfc5_crash_info/stacktrace.rs | 4 +- 8 files changed, 69 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f4ba71c08..d92af2aef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1374,6 +1374,7 @@ dependencies = [ "datadog-crashtracker", "ddcommon", "ddcommon-ffi", + "function_name", "hyper 0.14.31", "symbolic-common", "symbolic-demangle", @@ -2229,6 +2230,21 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "42703706b716c37f96a77aea830392ad231f44c9e9a67872fa5548707e11b11c" +[[package]] +name = "function_name" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1ab577a896d09940b5fe12ec5ae71f9d8211fff62c919c03a3750a9901e98a7" +dependencies = [ + "function_name-proc-macro", +] + +[[package]] +name = "function_name-proc-macro" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "673464e1e314dd67a0fd9544abc99e8eb28d0c7e3b69b033bcff9b2d00b87333" + [[package]] name = "futures" version = "0.3.31" diff --git a/crashtracker-ffi/Cargo.toml b/crashtracker-ffi/Cargo.toml index c6de3f695..2a1db320b 100644 --- a/crashtracker-ffi/Cargo.toml +++ b/crashtracker-ffi/Cargo.toml @@ -31,3 +31,4 @@ ddcommon-ffi = { path = "../ddcommon-ffi", default-features = false } hyper = {version = "0.14", features = ["backports", "deprecated"], default-features = false} symbolic-demangle = { version = "12.8.0", default-features = false, features = ["rust", "cpp", "msvc"], optional = true } symbolic-common = { version = "12.8.0", default-features = false, optional = true } +function_name = "0.3.0" diff --git a/crashtracker-ffi/src/crash_info/datatypes.rs b/crashtracker-ffi/src/crash_info/datatypes.rs index 1b01ec2d4..58db5ba14 100644 --- a/crashtracker-ffi/src/crash_info/datatypes.rs +++ b/crashtracker-ffi/src/crash_info/datatypes.rs @@ -179,7 +179,7 @@ impl<'a> TryFrom<&StackFrameNames<'a>> for datadog_crashtracker::StackFrameNames } #[repr(C)] -pub struct StackFrame<'a> { +pub struct StackFrameOld<'a> { build_id: CharSlice<'a>, ip: usize, module_base_address: usize, @@ -189,10 +189,10 @@ pub struct StackFrame<'a> { symbol_address: usize, } -impl<'a> TryFrom<&StackFrame<'a>> for datadog_crashtracker::StackFrame { +impl<'a> TryFrom<&StackFrameOld<'a>> for datadog_crashtracker::StackFrame { type Error = anyhow::Error; - fn try_from(value: &StackFrame<'a>) -> Result { + fn try_from(value: &StackFrameOld<'a>) -> Result { fn to_hex(v: usize) -> Option { if v == 0 { None diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index 18e49e0cf..dce600cdd 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -3,6 +3,9 @@ mod datatypes; pub use datatypes::*; +mod stacktrace; +pub use stacktrace::*; +pub mod to_inner; use crate::{option_from_char_slice, Result}; use anyhow::Context; @@ -175,7 +178,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfo_set_siginfo( pub unsafe extern "C" fn ddog_crasht_CrashInfo_set_stacktrace( crashinfo: *mut CrashInfo, thread_id: CharSlice, - stacktrace: Slice, + stacktrace: Slice, ) -> Result { (|| { let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; diff --git a/crashtracker-ffi/src/crash_info/stacktrace.rs b/crashtracker-ffi/src/crash_info/stacktrace.rs index 2e23ed116..756f8b71d 100644 --- a/crashtracker-ffi/src/crash_info/stacktrace.rs +++ b/crashtracker-ffi/src/crash_info/stacktrace.rs @@ -62,6 +62,18 @@ impl Drop for StackFrame { //////////////////////////////////////////////////////////////////////////////////////////////////// // FFI API // //////////////////////////////////////////////////////////////////////////////////////////////////// + +/// Wraps a C-FFI function in standard form +/// Expects the function to return a result type that implements into +/// and to be decorated with #[named]. +macro_rules! wrap { + ($body:expr) => { + (|| $body)() + .context(concat!(function_name!(), " failed")) + .into() + }; +} + /// Returned by [ddog_prof_Profile_new]. #[repr(C)] pub enum StackFrameNewResult { @@ -98,18 +110,17 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_drop(frame: *mut StackFrame) { /// which has not previously been dropped. /// The CharSlice must be valid. #[no_mangle] +#[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip( mut frame: *mut StackFrame, ip: CharSlice, ) -> Result { - (|| { + wrap!({ let frame = frame.to_inner_mut()?; frame.ip = option_from_char_slice(ip)?; anyhow::Ok(()) - })() - .context(concat!(function_name!(), " failed")) - .into() + }) } /// # Safety @@ -117,18 +128,17 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip( /// which has not previously been dropped. /// The CharSlice must be valid. #[no_mangle] +#[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address( mut frame: *mut StackFrame, module_base_address: CharSlice, ) -> Result { - (|| { + wrap!({ let frame = frame.to_inner_mut()?; frame.module_base_address = option_from_char_slice(module_base_address)?; anyhow::Ok(()) - })() - .context(concat!(function_name!(), " failed")) - .into() + }) } /// # Safety @@ -136,18 +146,17 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address( /// which has not previously been dropped. /// The CharSlice must be valid. #[no_mangle] +#[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp( mut frame: *mut StackFrame, sp: CharSlice, ) -> Result { - (|| { + wrap!({ let frame = frame.to_inner_mut()?; frame.sp = option_from_char_slice(sp)?; anyhow::Ok(()) - })() - .context(concat!(function_name!(), " failed")) - .into() + }) } /// # Safety @@ -155,18 +164,17 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp( /// which has not previously been dropped. /// The CharSlice must be valid. #[no_mangle] +#[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address( mut frame: *mut StackFrame, symbol_address: CharSlice, ) -> Result { - (|| { + wrap!({ let frame = frame.to_inner_mut()?; frame.symbol_address = option_from_char_slice(symbol_address)?; anyhow::Ok(()) - })() - .context(concat!(function_name!(), " failed")) - .into() + }) } /// # Safety @@ -174,18 +182,17 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address( /// which has not previously been dropped. /// The CharSlice must be valid. #[no_mangle] +#[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( mut frame: *mut StackFrame, build_id: CharSlice, ) -> Result { - (|| { + wrap!({ let frame = frame.to_inner_mut()?; frame.build_id = option_from_char_slice(build_id)?; anyhow::Ok(()) - })() - .context(concat!(function_name!(), " failed")) - .into() + }) } /// # Safety @@ -193,18 +200,17 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( /// which has not previously been dropped. /// The CharSlice must be valid. #[no_mangle] +#[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path( mut frame: *mut StackFrame, path: CharSlice, ) -> Result { - (|| { + wrap!({ let frame = frame.to_inner_mut()?; frame.path = option_from_char_slice(path)?; anyhow::Ok(()) - })() - .context(concat!(function_name!(), " failed")) - .into() + }) } /// # Safety @@ -212,18 +218,17 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path( /// which has not previously been dropped. /// The CharSlice must be valid. #[no_mangle] +#[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( mut frame: *mut StackFrame, relative_address: CharSlice, ) -> Result { - (|| { + wrap!({ let frame = frame.to_inner_mut()?; frame.relative_address = option_from_char_slice(relative_address)?; anyhow::Ok(()) - })() - .context(concat!(function_name!(), " failed")) - .into() + }) } /// # Safety @@ -231,32 +236,26 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( /// which has not previously been dropped. /// The CharSlice must be valid. #[no_mangle] +#[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_file( mut frame: *mut StackFrame, file: CharSlice, ) -> Result { - (|| { + wrap!({ let frame = frame.to_inner_mut()?; frame.file = option_from_char_slice(file)?; anyhow::Ok(()) - })() - .context(concat!(function_name!(), " failed")) - .into() -} -macro_rules! wrap { - ($body:expr) => { - (|| $body)() - .context(concat!(function_name!(), " failed")) - .into() - }; + }) } + /// # Safety /// The `frame` can be null, but if non-null it must point to a Frame made by this module, /// which has not previously been dropped. /// The CharSlice must be valid. #[no_mangle] +#[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_function( mut frame: *mut StackFrame, diff --git a/crashtracker/src/rfc5_crash_info/builder.rs b/crashtracker/src/rfc5_crash_info/builder.rs index ed6864e0a..5aeb191ed 100644 --- a/crashtracker/src/rfc5_crash_info/builder.rs +++ b/crashtracker/src/rfc5_crash_info/builder.rs @@ -139,14 +139,17 @@ impl CrashInfoBuilder { self.files = Some(files); self } + pub fn with_fingerprint(&mut self, fingerprint: String) -> &mut Self { self.fingerprint = Some(fingerprint); self } + pub fn with_incomplete(&mut self, incomplete: bool) -> &mut Self { self.incomplete = Some(incomplete); self } + pub fn with_log_messages(&mut self, log_messages: Vec) -> &mut Self { self.log_messages = Some(log_messages); self diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index cf53dc093..3c4d93dde 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -12,6 +12,7 @@ mod stacktrace; mod unknown_value; pub use builder::*; +pub use stacktrace::*; use anyhow::Context; use error_data::{thread_data_from_additional_stacktraces, ErrorData, ErrorKind, SourceType}; diff --git a/crashtracker/src/rfc5_crash_info/stacktrace.rs b/crashtracker/src/rfc5_crash_info/stacktrace.rs index a8498d5ad..f3289b5ee 100644 --- a/crashtracker/src/rfc5_crash_info/stacktrace.rs +++ b/crashtracker/src/rfc5_crash_info/stacktrace.rs @@ -110,7 +110,7 @@ impl From> for StackTrace { } } -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema, Default)] pub struct StackFrame { // Absolute addresses #[serde(default, skip_serializing_if = "Option::is_none")] @@ -147,6 +147,7 @@ pub struct StackFrame { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] #[allow(clippy::upper_case_acronyms)] +#[repr(C)] pub enum BuildIdType { GNU, GO, @@ -157,6 +158,7 @@ pub enum BuildIdType { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] #[allow(clippy::upper_case_acronyms)] +#[repr(C)] pub enum FileType { APK, ELF, From 5c36e01b1334c5279381b09c641f73912cc65599 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Thu, 21 Nov 2024 22:14:55 -0500 Subject: [PATCH 04/36] stacktrace --- crashtracker-ffi/src/crash_info/mod.rs | 2 + crashtracker-ffi/src/crash_info/stacktrace.rs | 225 ++++-------------- .../src/rfc5_crash_info/stacktrace.rs | 15 ++ 3 files changed, 58 insertions(+), 184 deletions(-) diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index dce600cdd..18fab44b2 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -3,6 +3,8 @@ mod datatypes; pub use datatypes::*; +mod stackframe; +pub use stackframe::*; mod stacktrace; pub use stacktrace::*; pub mod to_inner; diff --git a/crashtracker-ffi/src/crash_info/stacktrace.rs b/crashtracker-ffi/src/crash_info/stacktrace.rs index 756f8b71d..69995ed8b 100644 --- a/crashtracker-ffi/src/crash_info/stacktrace.rs +++ b/crashtracker-ffi/src/crash_info/stacktrace.rs @@ -1,27 +1,23 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use std::ptr; - use super::to_inner::ToInner; -use crate::{option_from_char_slice, Result}; +use crate::Result; +use crate::StackFrame; use ::function_name::named; use anyhow::Context; -use ddcommon_ffi::{ - slice::{AsBytes, ByteSlice}, - CharSlice, Error, Slice, -}; +use ddcommon_ffi::Error; -/// Represents a StackFrame. Do not access its member for any reason, only use +/// Represents a StackTrace. Do not access its member for any reason, only use /// the C API functions on this struct. #[repr(C)] -pub struct StackFrame { - // This may be null, but if not it will point to a valid StackFrame. - inner: *mut datadog_crashtracker::rfc5_crash_info::StackFrame, +pub struct StackTrace { + // This may be null, but if not it will point to a valid StackTrace. + inner: *mut datadog_crashtracker::rfc5_crash_info::StackTrace, } -impl ToInner for StackFrame { - type Inner = datadog_crashtracker::rfc5_crash_info::StackFrame; +impl ToInner for StackTrace { + type Inner = datadog_crashtracker::rfc5_crash_info::StackTrace; unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut Self::Inner> { self.inner @@ -30,16 +26,16 @@ impl ToInner for StackFrame { } } -impl StackFrame { - pub(super) fn new(crash_info: datadog_crashtracker::rfc5_crash_info::StackFrame) -> Self { - StackFrame { +impl StackTrace { + pub(super) fn new(crash_info: datadog_crashtracker::rfc5_crash_info::StackTrace) -> Self { + StackTrace { inner: Box::into_raw(Box::new(crash_info)), } } pub(super) fn take( &mut self, - ) -> Option> { + ) -> Option> { // Leaving a null will help with double-free issues that can // arise in C. Of course, it's best to never get there in the // first place! @@ -53,12 +49,20 @@ impl StackFrame { } } -impl Drop for StackFrame { +impl Drop for StackTrace { fn drop(&mut self) { drop(self.take()) } } +/// Returned by [ddog_prof_Profile_new]. +#[repr(C)] +pub enum StackTraceNewResult { + Ok(StackTrace), + #[allow(dead_code)] + Err(Error), +} + //////////////////////////////////////////////////////////////////////////////////////////////////// // FFI API // //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -74,22 +78,14 @@ macro_rules! wrap { }; } -/// Returned by [ddog_prof_Profile_new]. -#[repr(C)] -pub enum StackFrameNewResult { - Ok(StackFrame), - #[allow(dead_code)] - Err(Error), -} - -/// Create a new StackFrame, and returns an opaque reference to it. +/// Create a new StackTrace, and returns an opaque reference to it. /// # Safety /// No safety issues. #[no_mangle] #[must_use] -pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> StackFrameNewResult { - StackFrameNewResult::Ok(StackFrame::new( - datadog_crashtracker::rfc5_crash_info::StackFrame::default(), +pub unsafe extern "C" fn ddog_crasht_StackTrace_new() -> StackTraceNewResult { + StackTraceNewResult::Ok(StackTrace::new( + datadog_crashtracker::rfc5_crash_info::StackTrace::new(), )) } @@ -97,173 +93,34 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> StackFrameNewResult { /// The `frame` can be null, but if non-null it must point to a Frame /// made by this module, which has not previously been dropped. #[no_mangle] -pub unsafe extern "C" fn ddog_crasht_StackFrame_drop(frame: *mut StackFrame) { +pub unsafe extern "C" fn ddog_crasht_StackTrace_drop(trace: *mut StackTrace) { // Technically, this function has been designed so if it's double-dropped // then it's okay, but it's not something that should be relied on. - if !frame.is_null() { - drop((*frame).take()) + if !trace.is_null() { + drop((*trace).take()) } } /// # Safety -/// The `frame` can be null, but if non-null it must point to a Frame made by this module, -/// which has not previously been dropped. -/// The CharSlice must be valid. -#[no_mangle] -#[must_use] -#[named] -pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip( - mut frame: *mut StackFrame, - ip: CharSlice, -) -> Result { - wrap!({ - let frame = frame.to_inner_mut()?; - frame.ip = option_from_char_slice(ip)?; - anyhow::Ok(()) - }) -} - -/// # Safety -/// The `frame` can be null, but if non-null it must point to a Frame made by this module, -/// which has not previously been dropped. -/// The CharSlice must be valid. -#[no_mangle] -#[must_use] -#[named] -pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address( - mut frame: *mut StackFrame, - module_base_address: CharSlice, -) -> Result { - wrap!({ - let frame = frame.to_inner_mut()?; - frame.module_base_address = option_from_char_slice(module_base_address)?; - anyhow::Ok(()) - }) -} - -/// # Safety -/// The `frame` can be null, but if non-null it must point to a Frame made by this module, -/// which has not previously been dropped. -/// The CharSlice must be valid. -#[no_mangle] -#[must_use] -#[named] -pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp( - mut frame: *mut StackFrame, - sp: CharSlice, -) -> Result { - wrap!({ - let frame = frame.to_inner_mut()?; - frame.sp = option_from_char_slice(sp)?; - anyhow::Ok(()) - }) -} - -/// # Safety -/// The `frame` can be null, but if non-null it must point to a Frame made by this module, -/// which has not previously been dropped. -/// The CharSlice must be valid. -#[no_mangle] -#[must_use] -#[named] -pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address( - mut frame: *mut StackFrame, - symbol_address: CharSlice, -) -> Result { - wrap!({ - let frame = frame.to_inner_mut()?; - frame.symbol_address = option_from_char_slice(symbol_address)?; - anyhow::Ok(()) - }) -} - -/// # Safety -/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// The `stacktrace` can be null, but if non-null it must point to a StackTrace made by this module, /// which has not previously been dropped. -/// The CharSlice must be valid. -#[no_mangle] -#[must_use] -#[named] -pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( - mut frame: *mut StackFrame, - build_id: CharSlice, -) -> Result { - wrap!({ - let frame = frame.to_inner_mut()?; - frame.build_id = option_from_char_slice(build_id)?; - anyhow::Ok(()) - }) -} - -/// # Safety -/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// The frame can be non-null, but if non-null it must point to a Frame made by this module, /// which has not previously been dropped. -/// The CharSlice must be valid. #[no_mangle] #[must_use] #[named] -pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path( - mut frame: *mut StackFrame, - path: CharSlice, +pub unsafe extern "C" fn ddog_crasht_StackTrace_push_frame( + mut trace: *mut StackTrace, + frame: *mut StackFrame, ) -> Result { wrap!({ - let frame = frame.to_inner_mut()?; - frame.path = option_from_char_slice(path)?; - anyhow::Ok(()) - }) -} - -/// # Safety -/// The `frame` can be null, but if non-null it must point to a Frame made by this module, -/// which has not previously been dropped. -/// The CharSlice must be valid. -#[no_mangle] -#[must_use] -#[named] -pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( - mut frame: *mut StackFrame, - relative_address: CharSlice, -) -> Result { - wrap!({ - let frame = frame.to_inner_mut()?; - frame.relative_address = option_from_char_slice(relative_address)?; - anyhow::Ok(()) - }) -} - -/// # Safety -/// The `frame` can be null, but if non-null it must point to a Frame made by this module, -/// which has not previously been dropped. -/// The CharSlice must be valid. -#[no_mangle] -#[must_use] -#[named] -pub unsafe extern "C" fn ddog_crasht_StackFrame_with_file( - mut frame: *mut StackFrame, - file: CharSlice, -) -> Result { - wrap!({ - let frame = frame.to_inner_mut()?; - frame.file = option_from_char_slice(file)?; - anyhow::Ok(()) - }) -} - - -/// # Safety -/// The `frame` can be null, but if non-null it must point to a Frame made by this module, -/// which has not previously been dropped. -/// The CharSlice must be valid. -#[no_mangle] -#[must_use] -#[named] -pub unsafe extern "C" fn ddog_crasht_StackFrame_with_function( - mut frame: *mut StackFrame, - function: CharSlice, -) -> Result { - wrap!({ - let frame = frame.to_inner_mut()?; - frame.function = option_from_char_slice(function)?; + let trace = trace.to_inner_mut()?; + let frame = *frame + .as_mut() + .context("Null frame pointer")? + .take() + .context("Frame had null inner pointer. Use after free?")?; + trace.frames.push(frame); anyhow::Ok(()) }) } diff --git a/crashtracker/src/rfc5_crash_info/stacktrace.rs b/crashtracker/src/rfc5_crash_info/stacktrace.rs index f3289b5ee..02a3d8cdf 100644 --- a/crashtracker/src/rfc5_crash_info/stacktrace.rs +++ b/crashtracker/src/rfc5_crash_info/stacktrace.rs @@ -12,6 +12,21 @@ pub struct StackTrace { pub frames: Vec, } +impl StackTrace { + pub fn new() -> Self { + Self { + format: "Datadog Crashtracker 1.0".to_string(), + frames: vec![], + } + } +} + +impl Default for StackTrace { + fn default() -> Self { + Self::new() + } +} + impl From> for StackTrace { fn from(value: Vec) -> Self { #[allow(clippy::type_complexity)] From cdf4b4965f95efb054952d37981f4beea16fc5cc Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 22 Nov 2024 10:04:29 -0500 Subject: [PATCH 05/36] stackframe --- crashtracker-ffi/src/collector/datatypes.rs | 7 +- crashtracker-ffi/src/crash_info/datatypes.rs | 7 +- crashtracker-ffi/src/crash_info/mod.rs | 4 +- crashtracker-ffi/src/crash_info/stackframe.rs | 263 ++++++++++++++++++ crashtracker-ffi/src/datatypes/mod.rs | 7 - ddcommon-ffi/src/slice.rs | 10 + 6 files changed, 281 insertions(+), 17 deletions(-) create mode 100644 crashtracker-ffi/src/crash_info/stackframe.rs diff --git a/crashtracker-ffi/src/collector/datatypes.rs b/crashtracker-ffi/src/collector/datatypes.rs index c73fa354b..5535a3ee9 100644 --- a/crashtracker-ffi/src/collector/datatypes.rs +++ b/crashtracker-ffi/src/collector/datatypes.rs @@ -1,7 +1,6 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use crate::option_from_char_slice; pub use datadog_crashtracker::{OpTypes, StacktraceCollection}; use ddcommon::Endpoint; use ddcommon_ffi::slice::{AsBytes, CharSlice}; @@ -45,8 +44,8 @@ impl<'a> TryFrom> for datadog_crashtracker::CrashtrackerRecei vec }; let path_to_receiver_binary = value.path_to_receiver_binary.try_to_utf8()?.to_string(); - let stderr_filename = option_from_char_slice(value.optional_stderr_filename)?; - let stdout_filename = option_from_char_slice(value.optional_stdout_filename)?; + let stderr_filename = value.optional_stderr_filename.try_to_string_option()?; + let stdout_filename = value.optional_stdout_filename.try_to_string_option()?; Self::new( args, env, @@ -89,7 +88,7 @@ impl<'a> TryFrom> for datadog_crashtracker::CrashtrackerConfiguration let endpoint = value.endpoint.cloned(); let resolve_frames = value.resolve_frames; let timeout_ms = value.timeout_ms; - let unix_socket_path = option_from_char_slice(value.optional_unix_socket_filename)?; + let unix_socket_path = value.optional_unix_socket_filename.try_to_string_option()?; Self::new( additional_files, create_alt_stack, diff --git a/crashtracker-ffi/src/crash_info/datatypes.rs b/crashtracker-ffi/src/crash_info/datatypes.rs index 58db5ba14..311b8f5da 100644 --- a/crashtracker-ffi/src/crash_info/datatypes.rs +++ b/crashtracker-ffi/src/crash_info/datatypes.rs @@ -1,7 +1,6 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use crate::option_from_char_slice; use ddcommon::tag::Tag; use ddcommon_ffi::{ slice::{AsBytes, ByteSlice}, @@ -166,9 +165,9 @@ impl<'a> TryFrom<&StackFrameNames<'a>> for datadog_crashtracker::StackFrameNames fn try_from(value: &StackFrameNames<'a>) -> Result { let colno = (&value.colno).into(); - let filename = option_from_char_slice(value.filename)?; + let filename = value.filename.try_to_string_option()?; let lineno = (&value.lineno).into(); - let name = option_from_char_slice(value.name)?; + let name = value.name.try_to_string_option()?; Ok(Self { colno, filename, @@ -236,7 +235,7 @@ impl<'a> TryFrom> for datadog_crashtracker::SigInfo { fn try_from(value: SigInfo<'a>) -> Result { let signum = value.signum; - let signame = option_from_char_slice(value.signame)?; + let signame = value.signame.try_to_string_option()?; let faulting_address = None; // TODO: Expose this to FFI Ok(Self { signum, diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index 18fab44b2..2551e764c 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -9,7 +9,7 @@ mod stacktrace; pub use stacktrace::*; pub mod to_inner; -use crate::{option_from_char_slice, Result}; +use crate::{Result}; use anyhow::Context; use ddcommon::Endpoint; use ddcommon_ffi::{slice::AsBytes, CharSlice, Slice, Timespec}; @@ -184,7 +184,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfo_set_stacktrace( ) -> Result { (|| { let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; - let thread_id = option_from_char_slice(thread_id)?; + let thread_id = thread_id.try_to_string_option()?; let mut stacktrace_vec = Vec::with_capacity(stacktrace.len()); for s in stacktrace.iter() { stacktrace_vec.push(s.try_into()?) diff --git a/crashtracker-ffi/src/crash_info/stackframe.rs b/crashtracker-ffi/src/crash_info/stackframe.rs new file mode 100644 index 000000000..d42b9e523 --- /dev/null +++ b/crashtracker-ffi/src/crash_info/stackframe.rs @@ -0,0 +1,263 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use super::to_inner::ToInner; +use crate::{Result}; +use ::function_name::named; +use anyhow::Context; +use ddcommon_ffi::{slice::AsBytes, CharSlice, Error}; + +/// Represents a StackFrame. Do not access its member for any reason, only use +/// the C API functions on this struct. +#[repr(C)] +pub struct StackFrame { + // This may be null, but if not it will point to a valid StackFrame. + inner: *mut datadog_crashtracker::rfc5_crash_info::StackFrame, +} + +impl ToInner for StackFrame { + type Inner = datadog_crashtracker::rfc5_crash_info::StackFrame; + + unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut Self::Inner> { + self.inner + .as_mut() + .context("inner pointer was null, indicates use after free") + } +} + +impl StackFrame { + pub(super) fn new(crash_info: datadog_crashtracker::rfc5_crash_info::StackFrame) -> Self { + StackFrame { + inner: Box::into_raw(Box::new(crash_info)), + } + } + + pub(super) fn take( + &mut self, + ) -> Option> { + // Leaving a null will help with double-free issues that can + // arise in C. Of course, it's best to never get there in the + // first place! + let raw = std::mem::replace(&mut self.inner, std::ptr::null_mut()); + + if raw.is_null() { + None + } else { + Some(unsafe { Box::from_raw(raw) }) + } + } +} + +impl Drop for StackFrame { + fn drop(&mut self) { + drop(self.take()) + } +} + +/// Returned by [ddog_prof_Profile_new]. +#[repr(C)] +pub enum StackFrameNewResult { + Ok(StackFrame), + #[allow(dead_code)] + Err(Error), +} + +//////////////////////////////////////////////////////////////////////////////////////////////////// +// FFI API // +//////////////////////////////////////////////////////////////////////////////////////////////////// + +/// Wraps a C-FFI function in standard form +/// Expects the function to return a result type that implements into +/// and to be decorated with #[named]. +macro_rules! wrap { + ($body:block) => { + (|| $body)() + .context(concat!(function_name!(), " failed")) + .into() + }; +} + +/// Create a new StackFrame, and returns an opaque reference to it. +/// # Safety +/// No safety issues. +#[no_mangle] +#[must_use] +pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> StackFrameNewResult { + StackFrameNewResult::Ok(StackFrame::new( + datadog_crashtracker::rfc5_crash_info::StackFrame::default(), + )) +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame +/// made by this module, which has not previously been dropped. +#[no_mangle] +pub unsafe extern "C" fn ddog_crasht_StackFrame_drop(frame: *mut StackFrame) { + // Technically, this function has been designed so if it's double-dropped + // then it's okay, but it's not something that should be relied on. + if !frame.is_null() { + drop((*frame).take()) + } +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip( + mut frame: *mut StackFrame, + ip: CharSlice, +) -> Result { + wrap!({ + let frame = frame.to_inner_mut()?; + frame.ip = ip.try_to_string_option()?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address( + mut frame: *mut StackFrame, + module_base_address: CharSlice, +) -> Result { + wrap!({ + let frame = frame.to_inner_mut()?; + frame.module_base_address = module_base_address.try_to_string_option()?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp( + mut frame: *mut StackFrame, + sp: CharSlice, +) -> Result { + wrap!({ + let frame = frame.to_inner_mut()?; + frame.sp = sp.try_to_string_option()?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address( + mut frame: *mut StackFrame, + symbol_address: CharSlice, +) -> Result { + wrap!({ + let frame = frame.to_inner_mut()?; + frame.symbol_address = symbol_address.try_to_string_option()?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( + mut frame: *mut StackFrame, + build_id: CharSlice, +) -> Result { + wrap!({ + let frame = frame.to_inner_mut()?; + frame.build_id = build_id.try_to_string_option()?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path( + mut frame: *mut StackFrame, + path: CharSlice, +) -> Result { + wrap!({ + let frame = frame.to_inner_mut()?; + frame.path = path.try_to_string_option()?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( + mut frame: *mut StackFrame, + relative_address: CharSlice, +) -> Result { + wrap!({ + let frame = frame.to_inner_mut()?; + frame.relative_address = relative_address.try_to_string_option()?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_file( + mut frame: *mut StackFrame, + file: CharSlice, +) -> Result { + wrap!({ + let frame = frame.to_inner_mut()?; + frame.file = file.try_to_string_option()?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_function( + mut frame: *mut StackFrame, + function: CharSlice, +) -> Result { + wrap!({ + let frame = frame.to_inner_mut()?; + frame.function = function.try_to_string_option()?; + anyhow::Ok(()) + }) +} diff --git a/crashtracker-ffi/src/datatypes/mod.rs b/crashtracker-ffi/src/datatypes/mod.rs index 04eb263ab..506c351cc 100644 --- a/crashtracker-ffi/src/datatypes/mod.rs +++ b/crashtracker-ffi/src/datatypes/mod.rs @@ -1,14 +1,7 @@ // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use ddcommon_ffi::slice::{AsBytes, CharSlice}; use ddcommon_ffi::Error; -use std::ops::Not; - -pub fn option_from_char_slice(s: CharSlice) -> anyhow::Result> { - let s = s.try_to_utf8()?.to_string(); - Ok(s.is_empty().not().then_some(s)) -} /// A generic result type for when a crashtracking operation may fail, /// but there's nothing to return in the case of success. diff --git a/ddcommon-ffi/src/slice.rs b/ddcommon-ffi/src/slice.rs index 4cc3ff417..9cf04a2d8 100644 --- a/ddcommon-ffi/src/slice.rs +++ b/ddcommon-ffi/src/slice.rs @@ -72,6 +72,16 @@ pub trait AsBytes<'a> { std::str::from_utf8(self.as_bytes()) } + #[inline] + fn try_to_string_option(&self) -> Result, Utf8Error> { + let s = std::str::from_utf8(self.as_bytes())?.to_string(); + if s.is_empty() { + Ok(None) + } else { + Ok(Some(s)) + } + } + #[inline] fn to_utf8_lossy(&self) -> Cow<'a, str> { String::from_utf8_lossy(self.as_bytes()) From 807e500f48aa7b48824f763307c6683fdb9fd6a1 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Mon, 25 Nov 2024 15:48:36 -0500 Subject: [PATCH 06/36] moved wrap function to common --- crashtracker-ffi/src/crash_info/mod.rs | 4 +- crashtracker-ffi/src/crash_info/stackframe.rs | 43 +++++++------------ crashtracker-ffi/src/crash_info/stacktrace.rs | 14 +----- crashtracker/src/rfc5_crash_info/builder.rs | 22 ++++++++++ .../src/rfc5_crash_info/stacktrace.rs | 6 +++ ddcommon-ffi/src/lib.rs | 1 + ddcommon-ffi/src/slice.rs | 2 +- 7 files changed, 51 insertions(+), 41 deletions(-) diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index 2551e764c..d8df31d2d 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -7,9 +7,11 @@ mod stackframe; pub use stackframe::*; mod stacktrace; pub use stacktrace::*; +mod builder; pub mod to_inner; +pub use builder::*; -use crate::{Result}; +use crate::Result; use anyhow::Context; use ddcommon::Endpoint; use ddcommon_ffi::{slice::AsBytes, CharSlice, Slice, Timespec}; diff --git a/crashtracker-ffi/src/crash_info/stackframe.rs b/crashtracker-ffi/src/crash_info/stackframe.rs index d42b9e523..f3cc988df 100644 --- a/crashtracker-ffi/src/crash_info/stackframe.rs +++ b/crashtracker-ffi/src/crash_info/stackframe.rs @@ -2,10 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 use super::to_inner::ToInner; -use crate::{Result}; +use crate::Result; use ::function_name::named; use anyhow::Context; -use ddcommon_ffi::{slice::AsBytes, CharSlice, Error}; +use ddcommon_ffi::{slice::AsBytes, wrap_with_ffi_result, CharSlice, Error}; /// Represents a StackFrame. Do not access its member for any reason, only use /// the C API functions on this struct. @@ -26,9 +26,11 @@ impl ToInner for StackFrame { } impl StackFrame { - pub(super) fn new(crash_info: datadog_crashtracker::rfc5_crash_info::StackFrame) -> Self { + pub(super) fn new() -> Self { StackFrame { - inner: Box::into_raw(Box::new(crash_info)), + inner: Box::into_raw(Box::new( + datadog_crashtracker::rfc5_crash_info::StackFrame::new(), + )), } } @@ -66,26 +68,13 @@ pub enum StackFrameNewResult { // FFI API // //////////////////////////////////////////////////////////////////////////////////////////////////// -/// Wraps a C-FFI function in standard form -/// Expects the function to return a result type that implements into -/// and to be decorated with #[named]. -macro_rules! wrap { - ($body:block) => { - (|| $body)() - .context(concat!(function_name!(), " failed")) - .into() - }; -} - /// Create a new StackFrame, and returns an opaque reference to it. /// # Safety /// No safety issues. #[no_mangle] #[must_use] pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> StackFrameNewResult { - StackFrameNewResult::Ok(StackFrame::new( - datadog_crashtracker::rfc5_crash_info::StackFrame::default(), - )) + StackFrameNewResult::Ok(StackFrame::new()) } /// # Safety @@ -111,7 +100,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip( mut frame: *mut StackFrame, ip: CharSlice, ) -> Result { - wrap!({ + wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.ip = ip.try_to_string_option()?; anyhow::Ok(()) @@ -129,7 +118,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address( mut frame: *mut StackFrame, module_base_address: CharSlice, ) -> Result { - wrap!({ + wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.module_base_address = module_base_address.try_to_string_option()?; anyhow::Ok(()) @@ -147,7 +136,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp( mut frame: *mut StackFrame, sp: CharSlice, ) -> Result { - wrap!({ + wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.sp = sp.try_to_string_option()?; anyhow::Ok(()) @@ -165,7 +154,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address( mut frame: *mut StackFrame, symbol_address: CharSlice, ) -> Result { - wrap!({ + wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.symbol_address = symbol_address.try_to_string_option()?; anyhow::Ok(()) @@ -183,7 +172,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( mut frame: *mut StackFrame, build_id: CharSlice, ) -> Result { - wrap!({ + wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.build_id = build_id.try_to_string_option()?; anyhow::Ok(()) @@ -201,7 +190,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path( mut frame: *mut StackFrame, path: CharSlice, ) -> Result { - wrap!({ + wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.path = path.try_to_string_option()?; anyhow::Ok(()) @@ -219,7 +208,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( mut frame: *mut StackFrame, relative_address: CharSlice, ) -> Result { - wrap!({ + wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.relative_address = relative_address.try_to_string_option()?; anyhow::Ok(()) @@ -237,7 +226,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_file( mut frame: *mut StackFrame, file: CharSlice, ) -> Result { - wrap!({ + wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.file = file.try_to_string_option()?; anyhow::Ok(()) @@ -255,7 +244,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_function( mut frame: *mut StackFrame, function: CharSlice, ) -> Result { - wrap!({ + wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.function = function.try_to_string_option()?; anyhow::Ok(()) diff --git a/crashtracker-ffi/src/crash_info/stacktrace.rs b/crashtracker-ffi/src/crash_info/stacktrace.rs index 69995ed8b..2230daf1e 100644 --- a/crashtracker-ffi/src/crash_info/stacktrace.rs +++ b/crashtracker-ffi/src/crash_info/stacktrace.rs @@ -6,6 +6,7 @@ use crate::Result; use crate::StackFrame; use ::function_name::named; use anyhow::Context; +use ddcommon_ffi::wrap_with_ffi_result; use ddcommon_ffi::Error; /// Represents a StackTrace. Do not access its member for any reason, only use @@ -67,17 +68,6 @@ pub enum StackTraceNewResult { // FFI API // //////////////////////////////////////////////////////////////////////////////////////////////////// -/// Wraps a C-FFI function in standard form -/// Expects the function to return a result type that implements into -/// and to be decorated with #[named]. -macro_rules! wrap { - ($body:expr) => { - (|| $body)() - .context(concat!(function_name!(), " failed")) - .into() - }; -} - /// Create a new StackTrace, and returns an opaque reference to it. /// # Safety /// No safety issues. @@ -113,7 +103,7 @@ pub unsafe extern "C" fn ddog_crasht_StackTrace_push_frame( mut trace: *mut StackTrace, frame: *mut StackFrame, ) -> Result { - wrap!({ + wrap_with_ffi_result!({ let trace = trace.to_inner_mut()?; let frame = *frame .as_mut() diff --git a/crashtracker/src/rfc5_crash_info/builder.rs b/crashtracker/src/rfc5_crash_info/builder.rs index 5aeb191ed..40d2744c5 100644 --- a/crashtracker/src/rfc5_crash_info/builder.rs +++ b/crashtracker/src/rfc5_crash_info/builder.rs @@ -125,6 +125,17 @@ impl CrashInfoBuilder { Self::default() } + /// Inserts the given counter to the current set of counters in the builder. + pub fn with_counter(&mut self, name: String, value: i64) -> anyhow::Result<&mut Self> { + anyhow::ensure!(!name.is_empty(), "Empty counter name not allowed"); + if let Some(ref mut counters) = &mut self.counters { + counters.insert(name, value); + } else { + self.counters = Some(HashMap::from([(name, value)])); + } + self + } + pub fn with_counters(&mut self, counters: HashMap) -> &mut Self { self.counters = Some(counters); self @@ -135,6 +146,17 @@ impl CrashInfoBuilder { self } + /// Appends the given file to the current set of files in the builder. + pub fn with_file(&mut self, filename: String, contents: Vec) -> &mut Self { + if let Some(ref mut files) = &mut self.files { + files.insert(filename, contents); + } else { + self.files = Some(HashMap::from([(filename, contents)])); + } + self + } + + /// Sets the current set of files in the builder. pub fn with_files(&mut self, files: HashMap>) -> &mut Self { self.files = Some(files); self diff --git a/crashtracker/src/rfc5_crash_info/stacktrace.rs b/crashtracker/src/rfc5_crash_info/stacktrace.rs index 02a3d8cdf..bdeaa228b 100644 --- a/crashtracker/src/rfc5_crash_info/stacktrace.rs +++ b/crashtracker/src/rfc5_crash_info/stacktrace.rs @@ -160,6 +160,12 @@ pub struct StackFrame { pub line: Option, } +impl StackFrame { + pub fn new() -> Self { + Self::default() + } +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] #[allow(clippy::upper_case_acronyms)] #[repr(C)] diff --git a/ddcommon-ffi/src/lib.rs b/ddcommon-ffi/src/lib.rs index 5163ca1c0..5452421aa 100644 --- a/ddcommon-ffi/src/lib.rs +++ b/ddcommon-ffi/src/lib.rs @@ -10,6 +10,7 @@ pub mod slice; pub mod string; pub mod tags; pub mod timespec; +pub mod utils; pub mod vec; pub use error::*; diff --git a/ddcommon-ffi/src/slice.rs b/ddcommon-ffi/src/slice.rs index 9cf04a2d8..862f84dcc 100644 --- a/ddcommon-ffi/src/slice.rs +++ b/ddcommon-ffi/src/slice.rs @@ -80,7 +80,7 @@ pub trait AsBytes<'a> { } else { Ok(Some(s)) } - } + } #[inline] fn to_utf8_lossy(&self) -> Cow<'a, str> { From 6f11b734ccaa392f3d217da41aff6f3c9e8cbb3a Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Mon, 25 Nov 2024 18:10:10 -0500 Subject: [PATCH 07/36] more functions implemented --- crashtracker-ffi/src/collector/counters.rs | 8 +- crashtracker-ffi/src/collector/mod.rs | 10 +- crashtracker-ffi/src/collector/spans.rs | 11 +- crashtracker-ffi/src/crash_info/datatypes.rs | 24 --- crashtracker-ffi/src/crash_info/mod.rs | 184 +----------------- crashtracker-ffi/src/crash_info/stackframe.rs | 33 ++-- crashtracker-ffi/src/crash_info/stacktrace.rs | 19 +- crashtracker-ffi/src/datatypes/mod.rs | 25 --- crashtracker-ffi/src/lib.rs | 2 - crashtracker-ffi/src/receiver.rs | 7 +- .../libdatadog-crashtracking-receiver.c | 4 +- crashtracker/src/rfc5_crash_info/builder.rs | 21 +- .../src/rfc5_crash_info/error_data.rs | 1 + crashtracker/src/rfc5_crash_info/mod.rs | 4 +- ddcommon-ffi/src/lib.rs | 2 + 15 files changed, 63 insertions(+), 292 deletions(-) delete mode 100644 crashtracker-ffi/src/datatypes/mod.rs diff --git a/crashtracker-ffi/src/collector/counters.rs b/crashtracker-ffi/src/collector/counters.rs index 7e9075abb..b54afba1e 100644 --- a/crashtracker-ffi/src/collector/counters.rs +++ b/crashtracker-ffi/src/collector/counters.rs @@ -2,8 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 use super::datatypes::OpTypes; -use crate::Result; use anyhow::Context; +use ddcommon_ffi::VoidResult; /// Resets all counters to 0. /// Expected to be used after a fork, to reset the counters on the child @@ -15,7 +15,7 @@ use anyhow::Context; /// No safety concerns. #[no_mangle] #[must_use] -pub unsafe extern "C" fn ddog_crasht_reset_counters() -> Result { +pub unsafe extern "C" fn ddog_crasht_reset_counters() -> VoidResult { datadog_crashtracker::reset_counters() .context("ddog_crasht_reset_counters failed") .into() @@ -28,7 +28,7 @@ pub unsafe extern "C" fn ddog_crasht_reset_counters() -> Result { /// /// # Safety /// No safety concerns. -pub unsafe extern "C" fn ddog_crasht_begin_op(op: OpTypes) -> Result { +pub unsafe extern "C" fn ddog_crasht_begin_op(op: OpTypes) -> VoidResult { datadog_crashtracker::begin_op(op) .context("ddog_crasht_begin_op failed") .into() @@ -41,7 +41,7 @@ pub unsafe extern "C" fn ddog_crasht_begin_op(op: OpTypes) -> Result { /// /// # Safety /// No safety concerns. -pub unsafe extern "C" fn ddog_crasht_end_op(op: OpTypes) -> Result { +pub unsafe extern "C" fn ddog_crasht_end_op(op: OpTypes) -> VoidResult { datadog_crashtracker::end_op(op) .context("ddog_crasht_end_op failed") .into() diff --git a/crashtracker-ffi/src/collector/mod.rs b/crashtracker-ffi/src/collector/mod.rs index 830279694..760fecb95 100644 --- a/crashtracker-ffi/src/collector/mod.rs +++ b/crashtracker-ffi/src/collector/mod.rs @@ -5,11 +5,11 @@ mod datatypes; mod spans; use super::crash_info::Metadata; -use crate::Result; use anyhow::Context; pub use counters::*; use datadog_crashtracker::CrashtrackerReceiverConfig; pub use datatypes::*; +use ddcommon_ffi::VoidResult; pub use spans::*; #[no_mangle] @@ -29,7 +29,7 @@ pub use spans::*; /// # Atomicity /// This function is not atomic. A crash during its execution may lead to /// unexpected crash-handling behaviour. -pub unsafe extern "C" fn ddog_crasht_shutdown() -> Result { +pub unsafe extern "C" fn ddog_crasht_shutdown() -> VoidResult { datadog_crashtracker::shutdown_crash_handler() .context("ddog_crasht_shutdown failed") .into() @@ -58,7 +58,7 @@ pub unsafe extern "C" fn ddog_crasht_update_on_fork( config: Config, receiver_config: ReceiverConfig, metadata: Metadata, -) -> Result { +) -> VoidResult { (|| { let config = config.try_into()?; let receiver_config = receiver_config.try_into()?; @@ -85,7 +85,7 @@ pub unsafe extern "C" fn ddog_crasht_init( config: Config, receiver_config: ReceiverConfig, metadata: Metadata, -) -> Result { +) -> VoidResult { (|| { let config = config.try_into()?; let receiver_config = receiver_config.try_into()?; @@ -111,7 +111,7 @@ pub unsafe extern "C" fn ddog_crasht_init( pub unsafe extern "C" fn ddog_crasht_init_without_receiver( config: Config, metadata: Metadata, -) -> Result { +) -> VoidResult { (|| { let config: datadog_crashtracker::CrashtrackerConfiguration = config.try_into()?; let metadata = metadata.try_into()?; diff --git a/crashtracker-ffi/src/collector/spans.rs b/crashtracker-ffi/src/collector/spans.rs index 4bf162228..a92cd7025 100644 --- a/crashtracker-ffi/src/collector/spans.rs +++ b/crashtracker-ffi/src/collector/spans.rs @@ -1,8 +1,9 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use crate::{Result, UsizeResult}; +use crate::UsizeResult; use anyhow::Context; +use ddcommon_ffi::VoidResult; /// Resets all stored spans to 0. /// Expected to be used after a fork, to reset the spans on the child @@ -14,7 +15,7 @@ use anyhow::Context; /// No safety concerns. #[no_mangle] #[must_use] -pub unsafe extern "C" fn ddog_crasht_clear_span_ids() -> Result { +pub unsafe extern "C" fn ddog_crasht_clear_span_ids() -> VoidResult { datadog_crashtracker::clear_spans() .context("ddog_crasht_clear_span_ids failed") .into() @@ -30,7 +31,7 @@ pub unsafe extern "C" fn ddog_crasht_clear_span_ids() -> Result { /// No safety concerns. #[no_mangle] #[must_use] -pub unsafe extern "C" fn ddog_crasht_clear_trace_ids() -> Result { +pub unsafe extern "C" fn ddog_crasht_clear_trace_ids() -> VoidResult { datadog_crashtracker::clear_traces() .context("ddog_crasht_clear_trace_ids failed") .into() @@ -118,7 +119,7 @@ pub unsafe extern "C" fn ddog_crasht_remove_span_id( id_high: u64, id_low: u64, idx: usize, -) -> Result { +) -> VoidResult { let id: u128 = (id_high as u128) << 64 | (id_low as u128); datadog_crashtracker::remove_span(id, idx) .context("ddog_crasht_remove_span_id failed") @@ -151,7 +152,7 @@ pub unsafe extern "C" fn ddog_crasht_remove_trace_id( id_high: u64, id_low: u64, idx: usize, -) -> Result { +) -> VoidResult { let id: u128 = (id_high as u128) << 64 | (id_low as u128); datadog_crashtracker::remove_trace(id, idx) .context("ddog_crasht_remove_trace_id failed") diff --git a/crashtracker-ffi/src/crash_info/datatypes.rs b/crashtracker-ffi/src/crash_info/datatypes.rs index 311b8f5da..9e9e0249b 100644 --- a/crashtracker-ffi/src/crash_info/datatypes.rs +++ b/crashtracker-ffi/src/crash_info/datatypes.rs @@ -1,7 +1,6 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use ddcommon::tag::Tag; use ddcommon_ffi::{ slice::{AsBytes, ByteSlice}, CharSlice, Error, Slice, @@ -244,26 +243,3 @@ impl<'a> TryFrom> for datadog_crashtracker::SigInfo { }) } } - -#[repr(C)] -pub struct Metadata<'a> { - pub library_name: CharSlice<'a>, - pub library_version: CharSlice<'a>, - pub family: CharSlice<'a>, - /// Should include "service", "environment", etc - pub tags: Option<&'a ddcommon_ffi::Vec>, -} - -impl<'a> TryFrom> for datadog_crashtracker::CrashtrackerMetadata { - type Error = anyhow::Error; - fn try_from(value: Metadata<'a>) -> anyhow::Result { - let library_name = value.library_name.try_to_utf8()?.to_string(); - let library_version = value.library_version.try_to_utf8()?.to_string(); - let family = value.family.try_to_utf8()?.to_string(); - let tags = value - .tags - .map(|tags| tags.iter().cloned().collect()) - .unwrap_or_default(); - Ok(Self::new(library_name, library_version, family, tags)) - } -} diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index d8df31d2d..8625b3897 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -10,11 +10,12 @@ pub use stacktrace::*; mod builder; pub mod to_inner; pub use builder::*; +mod metadata; +pub use metadata::*; -use crate::Result; use anyhow::Context; use ddcommon::Endpoint; -use ddcommon_ffi::{slice::AsBytes, CharSlice, Slice, Timespec}; +use ddcommon_ffi::VoidResult; /// Create a new crashinfo, and returns an opaque reference to it. /// # Safety @@ -48,7 +49,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfo_drop(crashinfo: *mut CrashInfo) { pub unsafe extern "C" fn ddog_crasht_CrashInfo_normalize_ips( crashinfo: *mut CrashInfo, pid: u32, -) -> Result { +) -> VoidResult { (|| { let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; crashinfo.normalize_ips(pid) @@ -57,181 +58,6 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfo_normalize_ips( .into() } -/// Adds a "counter" variable, with the given value. Useful for determining if -/// "interesting" operations were occurring when the crash did. -/// -/// # Safety -/// `crashinfo` must be a valid pointer to a `CrashInfo` object. -/// `name` should be a valid reference to a utf8 encoded String. -/// The string is copied into the crashinfo, so it does not need to outlive this -/// call. -#[no_mangle] -#[must_use] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_add_counter( - crashinfo: *mut CrashInfo, - name: CharSlice, - val: i64, -) -> Result { - (|| { - let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; - let name = name.to_utf8_lossy(); - crashinfo.add_counter(&name, val) - })() - .context("ddog_crasht_CrashInfo_add_counter failed") - .into() -} - -/// Adds the contents of "file" to the crashinfo -/// -/// # Safety -/// `crashinfo` must be a valid pointer to a `CrashInfo` object. -/// `name` should be a valid reference to a utf8 encoded String. -/// The string is copied into the crashinfo, so it does not need to outlive this -/// call. -#[no_mangle] -#[must_use] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_add_file( - crashinfo: *mut CrashInfo, - filename: CharSlice, -) -> Result { - (|| { - let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; - let filename = filename.to_utf8_lossy(); - crashinfo.add_file(&filename) - })() - .context("ddog_crasht_CrashInfo_add_file failed") - .into() -} - -/// Adds the tag with given "key" and "value" to the crashinfo -/// -/// # Safety -/// `crashinfo` must be a valid pointer to a `CrashInfo` object. -/// `key` should be a valid reference to a utf8 encoded String. -/// `value` should be a valid reference to a utf8 encoded String. -/// The string is copied into the crashinfo, so it does not need to outlive this -/// call. -#[no_mangle] -#[must_use] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_add_tag( - crashinfo: *mut CrashInfo, - key: CharSlice, - value: CharSlice, -) -> Result { - (|| { - let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; - let key = key.to_utf8_lossy().to_string(); - let value = value.to_utf8_lossy().to_string(); - crashinfo.add_tag(key, value) - })() - .context("ddog_crasht_CrashInfo_add_tag failed") - .into() -} - -/// Sets the crashinfo metadata -/// -/// # Safety -/// `crashinfo` must be a valid pointer to a `CrashInfo` object. -/// All references inside `metadata` must be valid. -/// Strings are copied into the crashinfo, and do not need to outlive this call. -#[no_mangle] -#[must_use] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_set_metadata( - crashinfo: *mut CrashInfo, - metadata: Metadata, -) -> Result { - (|| { - let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; - let metadata = metadata.try_into()?; - crashinfo.set_metadata(metadata) - })() - .context("ddog_crasht_CrashInfo_set_metadata failed") - .into() -} - -/// Sets the crashinfo siginfo -/// -/// # Safety -/// `crashinfo` must be a valid pointer to a `CrashInfo` object. -/// All references inside `metadata` must be valid. -/// Strings are copied into the crashinfo, and do not need to outlive this call. -#[no_mangle] -#[must_use] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_set_siginfo( - crashinfo: *mut CrashInfo, - siginfo: SigInfo, -) -> Result { - (|| { - let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; - let siginfo = siginfo.try_into()?; - crashinfo.set_siginfo(siginfo) - })() - .context("ddog_crasht_CrashInfo_set_siginfo failed") - .into() -} - -/// If `thread_id` is empty, sets `stacktrace` as the default stacktrace. -/// Otherwise, adds an additional stacktrace with id "thread_id". -/// -/// # Safety -/// `crashinfo` must be a valid pointer to a `CrashInfo` object. -/// All references inside `stacktraces` must be valid. -/// Strings are copied into the crashinfo, and do not need to outlive this call. -#[no_mangle] -#[must_use] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_set_stacktrace( - crashinfo: *mut CrashInfo, - thread_id: CharSlice, - stacktrace: Slice, -) -> Result { - (|| { - let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; - let thread_id = thread_id.try_to_string_option()?; - let mut stacktrace_vec = Vec::with_capacity(stacktrace.len()); - for s in stacktrace.iter() { - stacktrace_vec.push(s.try_into()?) - } - crashinfo.set_stacktrace(thread_id, stacktrace_vec) - })() - .context("ddog_crasht_CrashInfo_set_stacktrace failed") - .into() -} - -/// Sets the timestamp to the given unix timestamp -/// -/// # Safety -/// `crashinfo` must be a valid pointer to a `CrashInfo` object. -#[no_mangle] -#[must_use] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_set_timestamp( - crashinfo: *mut CrashInfo, - ts: Timespec, -) -> Result { - (|| { - let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; - crashinfo.set_timestamp(ts.into()) - })() - .context("ddog_crasht_CrashInfo_set_timestamp_to_now failed") - .into() -} - -/// Sets the timestamp to the current time -/// -/// # Safety -/// `crashinfo` must be a valid pointer to a `CrashInfo` object. -#[no_mangle] -#[must_use] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_set_timestamp_to_now( - crashinfo: *mut CrashInfo, -) -> Result { - (|| { - let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; - crashinfo.set_timestamp_to_now() - })() - .context("ddog_crasht_CrashInfo_set_timestamp_to_now failed") - .into() -} - /// Exports `crashinfo` to the backend at `endpoint` /// Note that we support the "file://" endpoint for local file output. /// # Safety @@ -241,7 +67,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfo_set_timestamp_to_now( pub unsafe extern "C" fn ddog_crasht_CrashInfo_upload_to_endpoint( crashinfo: *mut CrashInfo, endpoint: Option<&Endpoint>, -) -> Result { +) -> VoidResult { (|| { let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; let endpoint = endpoint.cloned(); diff --git a/crashtracker-ffi/src/crash_info/stackframe.rs b/crashtracker-ffi/src/crash_info/stackframe.rs index f3cc988df..72afb8f91 100644 --- a/crashtracker-ffi/src/crash_info/stackframe.rs +++ b/crashtracker-ffi/src/crash_info/stackframe.rs @@ -2,10 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 use super::to_inner::ToInner; -use crate::Result; use ::function_name::named; use anyhow::Context; -use ddcommon_ffi::{slice::AsBytes, wrap_with_ffi_result, CharSlice, Error}; +use ddcommon_ffi::{slice::AsBytes, wrap_with_ffi_result, CharSlice, Result, VoidResult}; /// Represents a StackFrame. Do not access its member for any reason, only use /// the C API functions on this struct. @@ -56,14 +55,6 @@ impl Drop for StackFrame { } } -/// Returned by [ddog_prof_Profile_new]. -#[repr(C)] -pub enum StackFrameNewResult { - Ok(StackFrame), - #[allow(dead_code)] - Err(Error), -} - //////////////////////////////////////////////////////////////////////////////////////////////////// // FFI API // //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -73,8 +64,8 @@ pub enum StackFrameNewResult { /// No safety issues. #[no_mangle] #[must_use] -pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> StackFrameNewResult { - StackFrameNewResult::Ok(StackFrame::new()) +pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> Result { + Ok(StackFrame::new()).into() } /// # Safety @@ -99,7 +90,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_drop(frame: *mut StackFrame) { pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip( mut frame: *mut StackFrame, ip: CharSlice, -) -> Result { +) -> VoidResult { wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.ip = ip.try_to_string_option()?; @@ -117,7 +108,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip( pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address( mut frame: *mut StackFrame, module_base_address: CharSlice, -) -> Result { +) -> VoidResult { wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.module_base_address = module_base_address.try_to_string_option()?; @@ -135,7 +126,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address( pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp( mut frame: *mut StackFrame, sp: CharSlice, -) -> Result { +) -> VoidResult { wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.sp = sp.try_to_string_option()?; @@ -153,7 +144,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp( pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address( mut frame: *mut StackFrame, symbol_address: CharSlice, -) -> Result { +) -> VoidResult { wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.symbol_address = symbol_address.try_to_string_option()?; @@ -171,7 +162,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address( pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( mut frame: *mut StackFrame, build_id: CharSlice, -) -> Result { +) -> VoidResult { wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.build_id = build_id.try_to_string_option()?; @@ -189,7 +180,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path( mut frame: *mut StackFrame, path: CharSlice, -) -> Result { +) -> VoidResult { wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.path = path.try_to_string_option()?; @@ -207,7 +198,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path( pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( mut frame: *mut StackFrame, relative_address: CharSlice, -) -> Result { +) -> VoidResult { wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.relative_address = relative_address.try_to_string_option()?; @@ -225,7 +216,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( pub unsafe extern "C" fn ddog_crasht_StackFrame_with_file( mut frame: *mut StackFrame, file: CharSlice, -) -> Result { +) -> VoidResult { wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.file = file.try_to_string_option()?; @@ -243,7 +234,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_file( pub unsafe extern "C" fn ddog_crasht_StackFrame_with_function( mut frame: *mut StackFrame, function: CharSlice, -) -> Result { +) -> VoidResult { wrap_with_ffi_result!({ let frame = frame.to_inner_mut()?; frame.function = function.try_to_string_option()?; diff --git a/crashtracker-ffi/src/crash_info/stacktrace.rs b/crashtracker-ffi/src/crash_info/stacktrace.rs index 2230daf1e..845dd74f6 100644 --- a/crashtracker-ffi/src/crash_info/stacktrace.rs +++ b/crashtracker-ffi/src/crash_info/stacktrace.rs @@ -2,12 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 use super::to_inner::ToInner; -use crate::Result; use crate::StackFrame; use ::function_name::named; use anyhow::Context; -use ddcommon_ffi::wrap_with_ffi_result; -use ddcommon_ffi::Error; +use ddcommon_ffi::{wrap_with_ffi_result, Result, VoidResult}; /// Represents a StackTrace. Do not access its member for any reason, only use /// the C API functions on this struct. @@ -56,14 +54,6 @@ impl Drop for StackTrace { } } -/// Returned by [ddog_prof_Profile_new]. -#[repr(C)] -pub enum StackTraceNewResult { - Ok(StackTrace), - #[allow(dead_code)] - Err(Error), -} - //////////////////////////////////////////////////////////////////////////////////////////////////// // FFI API // //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -73,10 +63,11 @@ pub enum StackTraceNewResult { /// No safety issues. #[no_mangle] #[must_use] -pub unsafe extern "C" fn ddog_crasht_StackTrace_new() -> StackTraceNewResult { - StackTraceNewResult::Ok(StackTrace::new( +pub unsafe extern "C" fn ddog_crasht_StackTrace_new() -> Result { + Ok(StackTrace::new( datadog_crashtracker::rfc5_crash_info::StackTrace::new(), )) + .into() } /// # Safety @@ -102,7 +93,7 @@ pub unsafe extern "C" fn ddog_crasht_StackTrace_drop(trace: *mut StackTrace) { pub unsafe extern "C" fn ddog_crasht_StackTrace_push_frame( mut trace: *mut StackTrace, frame: *mut StackFrame, -) -> Result { +) -> VoidResult { wrap_with_ffi_result!({ let trace = trace.to_inner_mut()?; let frame = *frame diff --git a/crashtracker-ffi/src/datatypes/mod.rs b/crashtracker-ffi/src/datatypes/mod.rs deleted file mode 100644 index 506c351cc..000000000 --- a/crashtracker-ffi/src/datatypes/mod.rs +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ -// SPDX-License-Identifier: Apache-2.0 - -use ddcommon_ffi::Error; - -/// A generic result type for when a crashtracking operation may fail, -/// but there's nothing to return in the case of success. -#[repr(C)] -pub enum Result { - Ok( - /// Do not use the value of Ok. This value only exists to overcome - /// Rust -> C code generation. - bool, - ), - Err(Error), -} - -impl From> for Result { - fn from(value: anyhow::Result<()>) -> Self { - match value { - Ok(_) => Self::Ok(true), - Err(err) => Self::Err(err.into()), - } - } -} diff --git a/crashtracker-ffi/src/lib.rs b/crashtracker-ffi/src/lib.rs index 1dc4a8a8d..a9811d61c 100644 --- a/crashtracker-ffi/src/lib.rs +++ b/crashtracker-ffi/src/lib.rs @@ -4,7 +4,6 @@ #[cfg(all(unix, feature = "collector"))] mod collector; mod crash_info; -mod datatypes; #[cfg(feature = "demangler")] mod demangler; #[cfg(all(unix, feature = "receiver"))] @@ -13,7 +12,6 @@ mod receiver; #[cfg(all(unix, feature = "collector"))] pub use collector::*; pub use crash_info::*; -pub use datatypes::*; #[cfg(feature = "demangler")] pub use demangler::*; #[cfg(all(unix, feature = "receiver"))] diff --git a/crashtracker-ffi/src/receiver.rs b/crashtracker-ffi/src/receiver.rs index 8349ff0dd..e7c586e01 100644 --- a/crashtracker-ffi/src/receiver.rs +++ b/crashtracker-ffi/src/receiver.rs @@ -1,9 +1,8 @@ // Copyright 2023-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use crate::Result; use anyhow::Context; -use ddcommon_ffi::{slice::AsBytes, CharSlice}; +use ddcommon_ffi::{slice::AsBytes, CharSlice, VoidResult}; #[no_mangle] #[must_use] /// Receives data from a crash collector via a pipe on `stdin`, formats it into @@ -16,7 +15,7 @@ use ddcommon_ffi::{slice::AsBytes, CharSlice}; /// See comments in [crashtracker/lib.rs] for a full architecture description. /// # Safety /// No safety concerns -pub unsafe extern "C" fn ddog_crasht_receiver_entry_point_stdin() -> Result { +pub unsafe extern "C" fn ddog_crasht_receiver_entry_point_stdin() -> VoidResult { datadog_crashtracker::receiver_entry_point_stdin() .context("ddog_crasht_receiver_entry_point_stdin failed") .into() @@ -37,7 +36,7 @@ pub unsafe extern "C" fn ddog_crasht_receiver_entry_point_stdin() -> Result { /// No safety concerns pub unsafe extern "C" fn ddog_crasht_receiver_entry_point_unix_socket( socket_path: CharSlice, -) -> Result { +) -> VoidResult { (|| { let socket_path = socket_path.try_to_utf8()?; datadog_crashtracker::receiver_entry_point_unix_socket(socket_path) diff --git a/crashtracker/libdatadog-crashtracking-receiver.c b/crashtracker/libdatadog-crashtracking-receiver.c index c8e3e265f..3f284e368 100644 --- a/crashtracker/libdatadog-crashtracking-receiver.c +++ b/crashtracker/libdatadog-crashtracking-receiver.c @@ -7,8 +7,8 @@ #include int main(void) { - ddog_crasht_Result new_result = ddog_crasht_receiver_entry_point_stdin(); - if (new_result.tag != DDOG_CRASHT_RESULT_OK) { + ddog_crasht_VoidResult new_result = ddog_crasht_receiver_entry_point_stdin(); + if (new_result.tag != DDOG_CRASHT_VOID_RESULT_OK) { ddog_CharSlice message = ddog_Error_message(&new_result.err); fprintf(stderr, "%.*s", (int)message.len, message.ptr); ddog_Error_drop(&new_result.err); diff --git a/crashtracker/src/rfc5_crash_info/builder.rs b/crashtracker/src/rfc5_crash_info/builder.rs index 40d2744c5..7e7f8771c 100644 --- a/crashtracker/src/rfc5_crash_info/builder.rs +++ b/crashtracker/src/rfc5_crash_info/builder.rs @@ -133,7 +133,7 @@ impl CrashInfoBuilder { } else { self.counters = Some(HashMap::from([(name, value)])); } - self + Ok(self) } pub fn with_counters(&mut self, counters: HashMap) -> &mut Self { @@ -141,9 +141,9 @@ impl CrashInfoBuilder { self } - pub fn with_kind(&mut self, kind: ErrorKind) -> &mut Self { + pub fn with_kind(&mut self, kind: ErrorKind) -> anyhow::Result<&mut Self> { self.error.with_kind(kind); - self + Ok(self) } /// Appends the given file to the current set of files in the builder. @@ -162,9 +162,10 @@ impl CrashInfoBuilder { self } - pub fn with_fingerprint(&mut self, fingerprint: String) -> &mut Self { + pub fn with_fingerprint(&mut self, fingerprint: String) -> anyhow::Result<&mut Self> { + anyhow::ensure!(!fingerprint.is_empty(), "Expect non-empty fingerprint"); self.fingerprint = Some(fingerprint); - self + Ok(self) } pub fn with_incomplete(&mut self, incomplete: bool) -> &mut Self { @@ -172,6 +173,16 @@ impl CrashInfoBuilder { self } + /// Appends the given message to the current set of messages in the builder. + pub fn with_log_message(&mut self, message: String) -> anyhow::Result<&mut Self> { + if let Some(ref mut messages) = &mut self.log_messages { + messages.push(message); + } else { + self.log_messages = Some(vec![message]); + } + Ok(self) + } + pub fn with_log_messages(&mut self, log_messages: Vec) -> &mut Self { self.log_messages = Some(log_messages); self diff --git a/crashtracker/src/rfc5_crash_info/error_data.rs b/crashtracker/src/rfc5_crash_info/error_data.rs index 216ce9b6e..74867d331 100644 --- a/crashtracker/src/rfc5_crash_info/error_data.rs +++ b/crashtracker/src/rfc5_crash_info/error_data.rs @@ -23,6 +23,7 @@ pub enum SourceType { } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +#[repr(C)] pub enum ErrorKind { Panic, UnhandledException, diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index 3c4d93dde..f007d7009 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -12,11 +12,11 @@ mod stacktrace; mod unknown_value; pub use builder::*; +pub use error_data::*; +pub use metadata::Metadata; pub use stacktrace::*; use anyhow::Context; -use error_data::{thread_data_from_additional_stacktraces, ErrorData, ErrorKind, SourceType}; -use metadata::Metadata; use os_info::OsInfo; use proc_info::ProcInfo; use schemars::JsonSchema; diff --git a/ddcommon-ffi/src/lib.rs b/ddcommon-ffi/src/lib.rs index 5452421aa..173d96738 100644 --- a/ddcommon-ffi/src/lib.rs +++ b/ddcommon-ffi/src/lib.rs @@ -6,6 +6,7 @@ mod error; pub mod array_queue; pub mod endpoint; pub mod option; +pub mod result; pub mod slice; pub mod string; pub mod tags; @@ -15,6 +16,7 @@ pub mod vec; pub use error::*; pub use option::*; +pub use result::*; pub use slice::{CharSlice, Slice}; pub use string::*; pub use timespec::*; From 535f3a1b244962fe30df65749d9899a0b4d4ec36 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Mon, 25 Nov 2024 18:17:54 -0500 Subject: [PATCH 08/36] actually add the files --- crashtracker-ffi/src/crash_info/builder.rs | 322 ++++++++++++++++++++ crashtracker-ffi/src/crash_info/metadata.rs | 50 +++ ddcommon-ffi/src/result.rs | 42 +++ ddcommon-ffi/src/utils.rs | 11 + 4 files changed, 425 insertions(+) create mode 100644 crashtracker-ffi/src/crash_info/builder.rs create mode 100644 crashtracker-ffi/src/crash_info/metadata.rs create mode 100644 ddcommon-ffi/src/result.rs create mode 100644 ddcommon-ffi/src/utils.rs diff --git a/crashtracker-ffi/src/crash_info/builder.rs b/crashtracker-ffi/src/crash_info/builder.rs new file mode 100644 index 000000000..39b08d0ef --- /dev/null +++ b/crashtracker-ffi/src/crash_info/builder.rs @@ -0,0 +1,322 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use super::{to_inner::ToInner, Metadata, StackTrace}; +use ::function_name::named; +use anyhow::Context; +use datadog_crashtracker::rfc5_crash_info::ErrorKind; +use ddcommon_ffi::{ + slice::AsBytes, wrap_with_ffi_result, CharSlice, Result, Slice, Timespec, VoidResult, +}; + +/// Represents a CrashInfoBuilder. Do not access its member for any reason, only use +/// the C API functions on this struct. +#[repr(C)] +pub struct CrashInfoBuilder { + // This may be null, but if not it will point to a valid CrashInfoBuilder. + inner: *mut datadog_crashtracker::rfc5_crash_info::CrashInfoBuilder, +} + +impl ToInner for CrashInfoBuilder { + type Inner = datadog_crashtracker::rfc5_crash_info::CrashInfoBuilder; + + unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut Self::Inner> { + self.inner + .as_mut() + .context("inner pointer was null, indicates use after free") + } +} + +impl CrashInfoBuilder { + pub(super) fn new() -> Self { + CrashInfoBuilder { + inner: Box::into_raw(Box::new( + datadog_crashtracker::rfc5_crash_info::CrashInfoBuilder::new(), + )), + } + } + + pub(super) fn take( + &mut self, + ) -> Option> { + // Leaving a null will help with double-free issues that can + // arise in C. Of course, it's best to never get there in the + // first place! + let raw = std::mem::replace(&mut self.inner, std::ptr::null_mut()); + + if raw.is_null() { + None + } else { + Some(unsafe { Box::from_raw(raw) }) + } + } +} + +impl Drop for CrashInfoBuilder { + fn drop(&mut self) { + drop(self.take()) + } +} + +//////////////////////////////////////////////////////////////////////////////////////////////////// +// FFI API // +//////////////////////////////////////////////////////////////////////////////////////////////////// + +/// Create a new CrashInfoBuilder, and returns an opaque reference to it. +/// # Safety +/// No safety issues. +#[no_mangle] +#[must_use] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_new() -> Result { + Ok(CrashInfoBuilder::new()).into() +} + +/// # 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. +#[no_mangle] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_drop(builder: *mut CrashInfoBuilder) { + // Technically, this function has been designed so if it's double-dropped + // then it's okay, but it's not something that should be relied on. + if !builder.is_null() { + drop((*builder).take()) + } +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_counter( + mut builder: *mut CrashInfoBuilder, + name: CharSlice, + value: i64, +) -> VoidResult { + wrap_with_ffi_result!({ + builder + .to_inner_mut()? + .with_counter(name.try_to_utf8()?.to_string(), value)?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The Kind must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_kind( + mut builder: *mut CrashInfoBuilder, + kind: ErrorKind, +) -> VoidResult { + wrap_with_ffi_result!({ + builder.to_inner_mut()?.with_kind(kind)?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_file( + mut builder: *mut CrashInfoBuilder, + filename: CharSlice, + contents: Slice, +) -> VoidResult { + wrap_with_ffi_result!({ + let filename = filename + .try_to_string_option()? + .context("filename cannot be empty string")?; + let contents = { + let mut accum = Vec::with_capacity(contents.len()); + for line in contents.iter() { + let line = line.try_to_utf8()?.to_string(); + accum.push(line); + } + accum + }; + + builder.to_inner_mut()?.with_file(filename, contents); + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_fingerprint( + mut builder: *mut CrashInfoBuilder, + fingerprint: CharSlice, +) -> VoidResult { + wrap_with_ffi_result!({ + builder + .to_inner_mut()? + .with_fingerprint(fingerprint.try_to_utf8()?.to_string())?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_incomplete( + mut builder: *mut CrashInfoBuilder, + incomplete: bool, +) -> VoidResult { + wrap_with_ffi_result!({ + builder.to_inner_mut()?.with_incomplete(incomplete); + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_log_message( + mut builder: *mut CrashInfoBuilder, + message: CharSlice, +) -> VoidResult { + wrap_with_ffi_result!({ + builder + .to_inner_mut()? + .with_log_message(message.try_to_utf8()?.to_string())?; + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// All arguments must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_metadata( + mut builder: *mut CrashInfoBuilder, + metadata: Metadata, +) -> VoidResult { + wrap_with_ffi_result!({ + builder.to_inner_mut()?.with_metadata(metadata.try_into()?); + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// All arguments must be valid. +/// Consumes the stack argument. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_stack( + mut builder: *mut CrashInfoBuilder, + mut stack: StackTrace, +) -> VoidResult { + wrap_with_ffi_result!({ + builder + .to_inner_mut()? + .with_stack(*stack.take().context("Stack was empty. Use after free?")?); + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp_now( + mut builder: *mut CrashInfoBuilder, +) -> VoidResult { + wrap_with_ffi_result!({ + builder.to_inner_mut()?.with_timestamp_now(); + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp( + mut builder: *mut CrashInfoBuilder, + ts: Timespec, +) -> VoidResult { + wrap_with_ffi_result!({ + builder.to_inner_mut()?.with_timestamp(ts.into()); + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid( + mut builder: *mut CrashInfoBuilder, + uuid: CharSlice, +) -> VoidResult { + wrap_with_ffi_result!({ + let builder = builder.to_inner_mut()?; + let uuid = uuid + .try_to_string_option()? + .context("UUID cannot be empty string")?; + builder.with_uuid(uuid); + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid_random( + mut builder: *mut CrashInfoBuilder, +) -> VoidResult { + wrap_with_ffi_result!({ + builder.to_inner_mut()?.with_uuid_random(); + anyhow::Ok(()) + }) +} + +// with_os_info +// with_os_info_this_machine +// with_proc_info +// with_sig_info +// with_span_ids +// with_stack +// with_threads +// with_trace_ids diff --git a/crashtracker-ffi/src/crash_info/metadata.rs b/crashtracker-ffi/src/crash_info/metadata.rs new file mode 100644 index 000000000..790213253 --- /dev/null +++ b/crashtracker-ffi/src/crash_info/metadata.rs @@ -0,0 +1,50 @@ +use ddcommon::tag::Tag; +use ddcommon_ffi::{slice::AsBytes, CharSlice}; + +#[repr(C)] +pub struct Metadata<'a> { + pub library_name: CharSlice<'a>, + pub library_version: CharSlice<'a>, + pub family: CharSlice<'a>, + /// Should include "service", "environment", etc + pub tags: Option<&'a ddcommon_ffi::Vec>, +} + +impl<'a> TryFrom> for datadog_crashtracker::rfc5_crash_info::Metadata { + type Error = anyhow::Error; + fn try_from(value: Metadata<'a>) -> anyhow::Result { + let library_name = value.library_name.try_to_utf8()?.to_string(); + let library_version = value.library_version.try_to_utf8()?.to_string(); + let family = value.family.try_to_utf8()?.to_string(); + let tags = if let Some(tags) = value.tags { + tags.into_iter().map(|t| t.to_string()).collect() + } else { + vec![] + }; + Ok(Self { + library_name, + library_version, + family, + tags, + }) + } +} + +impl<'a> TryFrom> for datadog_crashtracker::CrashtrackerMetadata { + type Error = anyhow::Error; + fn try_from(value: Metadata<'a>) -> anyhow::Result { + let library_name = value.library_name.try_to_utf8()?.to_string(); + let library_version = value.library_version.try_to_utf8()?.to_string(); + let family = value.family.try_to_utf8()?.to_string(); + let tags = value + .tags + .map(|tags| tags.iter().cloned().collect()) + .unwrap_or_default(); + Ok(Self { + library_name, + library_version, + family, + tags, + }) + } +} diff --git a/ddcommon-ffi/src/result.rs b/ddcommon-ffi/src/result.rs new file mode 100644 index 000000000..6b160c2e1 --- /dev/null +++ b/ddcommon-ffi/src/result.rs @@ -0,0 +1,42 @@ +// Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use crate::Error; + +/// A generic result type for when an operation may fail, +/// but there's nothing to return in the case of success. +#[repr(C)] +pub enum VoidResult { + Ok( + /// Do not use the value of Ok. This value only exists to overcome + /// Rust -> C code generation. + bool, + ), + Err(Error), +} + +impl From> for VoidResult { + fn from(value: anyhow::Result<()>) -> Self { + match value { + Ok(_) => Self::Ok(true), + Err(err) => Self::Err(err.into()), + } + } +} + +/// A generic result type for when an operation may fail, +/// or may return in case of success. +#[repr(C)] +pub enum Result { + Ok(T), + Err(Error), +} + +impl From> for Result { + fn from(value: anyhow::Result) -> Self { + match value { + Ok(v) => Self::Ok(v), + Err(err) => Self::Err(err.into()), + } + } +} diff --git a/ddcommon-ffi/src/utils.rs b/ddcommon-ffi/src/utils.rs new file mode 100644 index 000000000..4a6bbe3db --- /dev/null +++ b/ddcommon-ffi/src/utils.rs @@ -0,0 +1,11 @@ +/// Wraps a C-FFI function in standard form +/// Expects the function to return a result type that implements into +/// and to be decorated with #[named]. +#[macro_export] +macro_rules! wrap_with_ffi_result { + ($body:block) => { + (|| $body)() + .context(concat!(function_name!(), " failed")) + .into() + }; +} From 79465735c78bfeebac66b7fde9e4e8b905df4180 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Wed, 27 Nov 2024 10:24:48 -0500 Subject: [PATCH 09/36] Fixup after merge --- crashtracker/src/rfc5_crash_info/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index 007b1ea47..ae6c6cccf 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -9,13 +9,16 @@ mod proc_info; mod sig_info; mod spans; mod stacktrace; +mod test_utils; mod unknown_value; pub use builder::*; pub use error_data::*; pub use metadata::Metadata; +use sig_info::SigInfo; pub use stacktrace::*; +use crate::rfc5_crash_info::spans::Span; use anyhow::Context; use os_info::OsInfo; use proc_info::ProcInfo; @@ -183,7 +186,7 @@ mod tests { log_messages: vec![], metadata: Metadata::test_instance(seed), os_info: ::os_info::Info::unknown().into(), - proc_info: ProcInfo::test_instance(seed), + proc_info: Some(ProcInfo::test_instance(seed)), sig_info: Some(SigInfo::test_instance(seed)), span_ids, timestamp: chrono::DateTime::from_timestamp(1568898000 /* Datadog IPO */, 0) From 08aaac55f9480a48bb9730ab7aad59de33029071 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Tue, 3 Dec 2024 13:27:33 -0500 Subject: [PATCH 10/36] more builder functions --- crashtracker-ffi/src/crash_info/builder.rs | 56 ++++++++++++++++++- crashtracker-ffi/src/crash_info/mod.rs | 4 ++ crashtracker-ffi/src/crash_info/os_info.rs | 41 ++++++++++++++ crashtracker-ffi/src/crash_info/proc_info.rs | 14 +++++ crashtracker/src/rfc5_crash_info/mod.rs | 4 +- crashtracker/src/rfc5_crash_info/proc_info.rs | 2 +- 6 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 crashtracker-ffi/src/crash_info/os_info.rs create mode 100644 crashtracker-ffi/src/crash_info/proc_info.rs diff --git a/crashtracker-ffi/src/crash_info/builder.rs b/crashtracker-ffi/src/crash_info/builder.rs index 39b08d0ef..304290d33 100644 --- a/crashtracker-ffi/src/crash_info/builder.rs +++ b/crashtracker-ffi/src/crash_info/builder.rs @@ -1,7 +1,7 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use super::{to_inner::ToInner, Metadata, StackTrace}; +use super::{to_inner::ToInner, Metadata, OsInfo, ProcInfo, StackTrace}; use ::function_name::named; use anyhow::Context; use datadog_crashtracker::rfc5_crash_info::ErrorKind; @@ -222,6 +222,58 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_metadata( }) } +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// All arguments must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_os_info( + mut builder: *mut CrashInfoBuilder, + os_info: OsInfo, +) -> VoidResult { + wrap_with_ffi_result!({ + builder.to_inner_mut()?.with_os_info(os_info.try_into()?); + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// All arguments must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_os_info_this_machine( + mut builder: *mut CrashInfoBuilder, +) -> VoidResult { + wrap_with_ffi_result!({ + builder.to_inner_mut()?.with_os_info_this_machine(); + anyhow::Ok(()) + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// All arguments must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_proc_info( + mut builder: *mut CrashInfoBuilder, + proc_info: ProcInfo, +) -> VoidResult { + wrap_with_ffi_result!({ + builder + .to_inner_mut()? + .with_proc_info(proc_info.try_into()?); + anyhow::Ok(()) + }) +} + /// # Safety /// The `builder` can be null, but if non-null it must point to a Builder made by this module, /// which has not previously been dropped. @@ -312,8 +364,6 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid_random( }) } -// with_os_info -// with_os_info_this_machine // with_proc_info // with_sig_info // with_span_ids diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index 8625b3897..c9ee7cbc4 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -12,6 +12,10 @@ pub mod to_inner; pub use builder::*; mod metadata; pub use metadata::*; +mod os_info; +pub use os_info::*; +mod proc_info; +pub use proc_info::*; use anyhow::Context; use ddcommon::Endpoint; diff --git a/crashtracker-ffi/src/crash_info/os_info.rs b/crashtracker-ffi/src/crash_info/os_info.rs new file mode 100644 index 000000000..6c81032f3 --- /dev/null +++ b/crashtracker-ffi/src/crash_info/os_info.rs @@ -0,0 +1,41 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use ddcommon_ffi::{slice::AsBytes, CharSlice}; + +#[repr(C)] +pub struct OsInfo<'a> { + pub architecture: CharSlice<'a>, + pub bitness: CharSlice<'a>, + pub os_type: CharSlice<'a>, + pub version: CharSlice<'a>, +} + +impl<'a> TryFrom> for datadog_crashtracker::rfc5_crash_info::OsInfo { + type Error = anyhow::Error; + fn try_from(value: OsInfo<'a>) -> anyhow::Result { + let unknown = || "unknown".to_string(); + let architecture = value + .architecture + .try_to_string_option()? + .unwrap_or_else(unknown); + let bitness = value + .bitness + .try_to_string_option()? + .unwrap_or_else(unknown); + let os_type = value + .os_type + .try_to_string_option()? + .unwrap_or_else(unknown); + let version = value + .version + .try_to_string_option()? + .unwrap_or_else(unknown); + Ok(Self { + architecture, + bitness, + os_type, + version, + }) + } +} diff --git a/crashtracker-ffi/src/crash_info/proc_info.rs b/crashtracker-ffi/src/crash_info/proc_info.rs new file mode 100644 index 000000000..e815c0eea --- /dev/null +++ b/crashtracker-ffi/src/crash_info/proc_info.rs @@ -0,0 +1,14 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +#[repr(C)] +pub struct ProcInfo { + pid: u32, +} + +impl TryFrom for datadog_crashtracker::rfc5_crash_info::ProcInfo { + type Error = anyhow::Error; + fn try_from(value: ProcInfo) -> anyhow::Result { + Ok(Self { pid: value.pid }) + } +} diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index ae6c6cccf..2d0f6c16e 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -15,13 +15,13 @@ mod unknown_value; pub use builder::*; pub use error_data::*; pub use metadata::Metadata; +pub use os_info::*; +pub use proc_info::*; use sig_info::SigInfo; pub use stacktrace::*; use crate::rfc5_crash_info::spans::Span; use anyhow::Context; -use os_info::OsInfo; -use proc_info::ProcInfo; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::{collections::HashMap, fs::File, path::Path}; diff --git a/crashtracker/src/rfc5_crash_info/proc_info.rs b/crashtracker/src/rfc5_crash_info/proc_info.rs index 756a12ae1..0b95a497d 100644 --- a/crashtracker/src/rfc5_crash_info/proc_info.rs +++ b/crashtracker/src/rfc5_crash_info/proc_info.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] pub struct ProcInfo { - pid: u32, + pub pid: u32, } impl From for ProcInfo { From 5a61c462e7067a60c92daf210d3328d6b1f238cc Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Tue, 3 Dec 2024 14:46:18 -0500 Subject: [PATCH 11/36] with_thread --- crashtracker-ffi/src/crash_info/builder.rs | 23 +++++++++-- crashtracker-ffi/src/crash_info/mod.rs | 2 + .../src/crash_info/thread_data.rs | 38 +++++++++++++++++++ crashtracker/src/rfc5_crash_info/builder.rs | 13 ++++++- 4 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 crashtracker-ffi/src/crash_info/thread_data.rs diff --git a/crashtracker-ffi/src/crash_info/builder.rs b/crashtracker-ffi/src/crash_info/builder.rs index 304290d33..9191535b5 100644 --- a/crashtracker-ffi/src/crash_info/builder.rs +++ b/crashtracker-ffi/src/crash_info/builder.rs @@ -1,7 +1,7 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use super::{to_inner::ToInner, Metadata, OsInfo, ProcInfo, StackTrace}; +use super::{to_inner::ToInner, Metadata, OsInfo, ProcInfo, StackTrace, ThreadData}; use ::function_name::named; use anyhow::Context; use datadog_crashtracker::rfc5_crash_info::ErrorKind; @@ -294,6 +294,24 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_stack( }) } +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// All arguments must be valid. +/// Consumes the stack argument. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_thread( + mut builder: *mut CrashInfoBuilder, + thread: ThreadData, +) -> VoidResult { + wrap_with_ffi_result!({ + builder.to_inner_mut()?.with_thread(thread.try_into()?)?; + anyhow::Ok(()) + }) +} + /// # Safety /// The `builder` can be null, but if non-null it must point to a Builder made by this module, /// which has not previously been dropped. @@ -364,9 +382,6 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid_random( }) } -// with_proc_info // with_sig_info // with_span_ids -// with_stack -// with_threads // with_trace_ids diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index c9ee7cbc4..e87a0ed65 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -16,6 +16,8 @@ mod os_info; pub use os_info::*; mod proc_info; pub use proc_info::*; +mod thread_data; +pub use thread_data::*; use anyhow::Context; use ddcommon::Endpoint; diff --git a/crashtracker-ffi/src/crash_info/thread_data.rs b/crashtracker-ffi/src/crash_info/thread_data.rs new file mode 100644 index 000000000..c0c50a6cb --- /dev/null +++ b/crashtracker-ffi/src/crash_info/thread_data.rs @@ -0,0 +1,38 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use anyhow::Context; +use ddcommon_ffi::{slice::AsBytes, CharSlice}; + +use super::StackTrace; + +#[repr(C)] +pub struct ThreadData<'a> { + pub crashed: bool, + pub name: CharSlice<'a>, + pub stack: StackTrace, + pub state: CharSlice<'a>, +} + +impl<'a> TryFrom> for datadog_crashtracker::rfc5_crash_info::ThreadData { + type Error = anyhow::Error; + fn try_from(mut value: ThreadData<'a>) -> anyhow::Result { + let crashed = value.crashed; + let name = value + .name + .try_to_string_option()? + .context("Name cannot be empty")?; + let stack = *value + .stack + .take() + .context("missing stack, use after free?")?; + let state = value.state.try_to_string_option()?; + + Ok(Self { + crashed, + name, + stack, + state, + }) + } +} diff --git a/crashtracker/src/rfc5_crash_info/builder.rs b/crashtracker/src/rfc5_crash_info/builder.rs index 7e7f8771c..682425145 100644 --- a/crashtracker/src/rfc5_crash_info/builder.rs +++ b/crashtracker/src/rfc5_crash_info/builder.rs @@ -227,9 +227,18 @@ impl CrashInfoBuilder { self } - pub fn with_threads(&mut self, threads: Vec) -> &mut Self { + pub fn with_thread(&mut self, thread: ThreadData) -> anyhow::Result<&mut Self> { + if let Some(ref mut threads) = &mut self.error.threads { + threads.push(thread); + } else { + self.error.threads = Some(vec![thread]); + } + Ok(self) + } + + pub fn with_threads(&mut self, threads: Vec) -> anyhow::Result<&mut Self> { self.error.with_threads(threads); - self + Ok(self) } pub fn with_timestamp(&mut self, timestamp: DateTime) -> &mut Self { From fb719dd925a91da1ef9975b52bf185bc99dbe1c2 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Tue, 3 Dec 2024 17:21:06 -0500 Subject: [PATCH 12/36] siginfo --- Cargo.lock | 5 ++-- crashtracker-ffi/Cargo.toml | 1 + crashtracker-ffi/src/crash_info/builder.rs | 29 ++++++++++++++++---- crashtracker-ffi/src/crash_info/datatypes.rs | 21 -------------- crashtracker-ffi/src/crash_info/mod.rs | 2 ++ crashtracker-ffi/src/crash_info/sig_info.rs | 27 ++++++++++++++++++ crashtracker/src/rfc5_crash_info/mod.rs | 2 +- crashtracker/src/rfc5_crash_info/sig_info.rs | 2 ++ 8 files changed, 59 insertions(+), 30 deletions(-) create mode 100644 crashtracker-ffi/src/crash_info/sig_info.rs diff --git a/Cargo.lock b/Cargo.lock index 5d63c649d..0cdf1a3e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1377,6 +1377,7 @@ dependencies = [ "ddcommon-ffi", "function_name", "hyper 0.14.31", + "libc", "symbolic-common", "symbolic-demangle", ] @@ -3242,9 +3243,9 @@ checksum = "db13adb97ab515a3691f56e4dbab09283d0b86cb45abd991d8634a9d6f501760" [[package]] name = "libc" -version = "0.2.161" +version = "0.2.167" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e9489c2807c139ffd9c1794f4af0ebe86a828db53ecdc7fea2111d0fed085d1" +checksum = "09d6582e104315a817dff97f75133544b2e094ee22447d2acf4a74e189ba06fc" [[package]] name = "libloading" diff --git a/crashtracker-ffi/Cargo.toml b/crashtracker-ffi/Cargo.toml index 2a1db320b..75c86a32f 100644 --- a/crashtracker-ffi/Cargo.toml +++ b/crashtracker-ffi/Cargo.toml @@ -32,3 +32,4 @@ hyper = {version = "0.14", features = ["backports", "deprecated"], default-featu symbolic-demangle = { version = "12.8.0", default-features = false, features = ["rust", "cpp", "msvc"], optional = true } symbolic-common = { version = "12.8.0", default-features = false, optional = true } function_name = "0.3.0" +libc = "0.2.167" diff --git a/crashtracker-ffi/src/crash_info/builder.rs b/crashtracker-ffi/src/crash_info/builder.rs index 9191535b5..b81d93f6e 100644 --- a/crashtracker-ffi/src/crash_info/builder.rs +++ b/crashtracker-ffi/src/crash_info/builder.rs @@ -1,7 +1,7 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use super::{to_inner::ToInner, Metadata, OsInfo, ProcInfo, StackTrace, ThreadData}; +use super::{to_inner::ToInner, Metadata, OsInfo, ProcInfo, SigInfo, StackTrace, ThreadData}; use ::function_name::named; use anyhow::Context; use datadog_crashtracker::rfc5_crash_info::ErrorKind; @@ -274,6 +274,23 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_proc_info( }) } +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// All arguments must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_sig_info( + mut builder: *mut CrashInfoBuilder, + sig_info: SigInfo, +) -> VoidResult { + wrap_with_ffi_result!({ + builder.to_inner_mut()?.with_sig_info(sig_info.try_into()?); + anyhow::Ok(()) + }) +} + /// # Safety /// The `builder` can be null, but if non-null it must point to a Builder made by this module, /// which has not previously been dropped. @@ -319,11 +336,12 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_thread( #[no_mangle] #[must_use] #[named] -pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp_now( +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp( mut builder: *mut CrashInfoBuilder, + ts: Timespec, ) -> VoidResult { wrap_with_ffi_result!({ - builder.to_inner_mut()?.with_timestamp_now(); + builder.to_inner_mut()?.with_timestamp(ts.into()); anyhow::Ok(()) }) } @@ -335,12 +353,11 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp_now( #[no_mangle] #[must_use] #[named] -pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp( +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp_now( mut builder: *mut CrashInfoBuilder, - ts: Timespec, ) -> VoidResult { wrap_with_ffi_result!({ - builder.to_inner_mut()?.with_timestamp(ts.into()); + builder.to_inner_mut()?.with_timestamp_now(); anyhow::Ok(()) }) } diff --git a/crashtracker-ffi/src/crash_info/datatypes.rs b/crashtracker-ffi/src/crash_info/datatypes.rs index 9e9e0249b..76c25d165 100644 --- a/crashtracker-ffi/src/crash_info/datatypes.rs +++ b/crashtracker-ffi/src/crash_info/datatypes.rs @@ -222,24 +222,3 @@ impl<'a> TryFrom<&StackFrameOld<'a>> for datadog_crashtracker::StackFrame { }) } } - -#[repr(C)] -pub struct SigInfo<'a> { - pub signum: u64, - pub signame: CharSlice<'a>, -} - -impl<'a> TryFrom> for datadog_crashtracker::SigInfo { - type Error = anyhow::Error; - - fn try_from(value: SigInfo<'a>) -> Result { - let signum = value.signum; - let signame = value.signame.try_to_string_option()?; - let faulting_address = None; // TODO: Expose this to FFI - Ok(Self { - signum, - signame, - faulting_address, - }) - } -} diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index e87a0ed65..f8411d36b 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -18,6 +18,8 @@ mod proc_info; pub use proc_info::*; mod thread_data; pub use thread_data::*; +mod sig_info; +pub use sig_info::*; use anyhow::Context; use ddcommon::Endpoint; diff --git a/crashtracker-ffi/src/crash_info/sig_info.rs b/crashtracker-ffi/src/crash_info/sig_info.rs new file mode 100644 index 000000000..8835fb3b7 --- /dev/null +++ b/crashtracker-ffi/src/crash_info/sig_info.rs @@ -0,0 +1,27 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use datadog_crashtracker::rfc5_crash_info::{SiCodes, SignalNames}; +use ddcommon_ffi::{slice::AsBytes, CharSlice}; + +#[repr(C)] +pub struct SigInfo<'a> { + pub si_addr: CharSlice<'a>, + pub si_code: libc::c_int, + pub si_code_human_readable: SiCodes, + pub si_signo: libc::c_int, + pub si_signo_human_readable: SignalNames, +} + +impl<'a> TryFrom> for datadog_crashtracker::rfc5_crash_info::SigInfo { + type Error = anyhow::Error; + fn try_from(value: SigInfo<'a>) -> anyhow::Result { + Ok(Self { + si_addr: value.si_addr.try_to_string_option()?, + si_code: value.si_code, + si_code_human_readable: value.si_code_human_readable, + si_signo: value.si_signo, + si_signo_human_readable: value.si_signo_human_readable, + }) + } +} diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index 2d0f6c16e..17b3f7adb 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -17,7 +17,7 @@ pub use error_data::*; pub use metadata::Metadata; pub use os_info::*; pub use proc_info::*; -use sig_info::SigInfo; +pub use sig_info::*; pub use stacktrace::*; use crate::rfc5_crash_info::spans::Span; diff --git a/crashtracker/src/rfc5_crash_info/sig_info.rs b/crashtracker/src/rfc5_crash_info/sig_info.rs index e318d36d6..5b160a0d6 100644 --- a/crashtracker/src/rfc5_crash_info/sig_info.rs +++ b/crashtracker/src/rfc5_crash_info/sig_info.rs @@ -34,6 +34,7 @@ impl From for SigInfo { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] #[allow(clippy::upper_case_acronyms, non_camel_case_types)] +#[repr(C)] /// See https://man7.org/linux/man-pages/man7/signal.7.html pub enum SignalNames { SIGABRT, @@ -64,6 +65,7 @@ impl From for SignalNames { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] #[allow(clippy::upper_case_acronyms, non_camel_case_types)] +#[repr(C)] /// See https://man7.org/linux/man-pages/man2/sigaction.2.html pub enum SiCodes { BUS_ADRALN, From 924637a4a5a1cd37af8fe6b5ca35883ff9bfed23 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Tue, 3 Dec 2024 17:52:59 -0500 Subject: [PATCH 13/36] all builder implemented --- crashtracker-ffi/src/crash_info/builder.rs | 42 ++++++++++++++++++--- crashtracker-ffi/src/crash_info/mod.rs | 23 ++++++----- crashtracker-ffi/src/crash_info/span.rs | 20 ++++++++++ crashtracker/src/rfc5_crash_info/builder.rs | 18 +++++++++ crashtracker/src/rfc5_crash_info/mod.rs | 2 +- 5 files changed, 89 insertions(+), 16 deletions(-) create mode 100644 crashtracker-ffi/src/crash_info/span.rs diff --git a/crashtracker-ffi/src/crash_info/builder.rs b/crashtracker-ffi/src/crash_info/builder.rs index b81d93f6e..b06039879 100644 --- a/crashtracker-ffi/src/crash_info/builder.rs +++ b/crashtracker-ffi/src/crash_info/builder.rs @@ -1,7 +1,7 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use super::{to_inner::ToInner, Metadata, OsInfo, ProcInfo, SigInfo, StackTrace, ThreadData}; +use super::{to_inner::ToInner, Metadata, OsInfo, ProcInfo, SigInfo, Span, StackTrace, ThreadData}; use ::function_name::named; use anyhow::Context; use datadog_crashtracker::rfc5_crash_info::ErrorKind; @@ -291,6 +291,23 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_sig_info( }) } +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// All arguments must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_span_id( + mut builder: *mut CrashInfoBuilder, + span_id: Span, +) -> VoidResult { + wrap_with_ffi_result!({ + builder.to_inner_mut()?.with_span_id(span_id.try_into()?)?; + anyhow::Ok(()) + }) +} + /// # Safety /// The `builder` can be null, but if non-null it must point to a Builder made by this module, /// which has not previously been dropped. @@ -362,6 +379,25 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp_now( }) } +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// All arguments must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_trace_id( + mut builder: *mut CrashInfoBuilder, + trace_id: Span, +) -> VoidResult { + wrap_with_ffi_result!({ + builder + .to_inner_mut()? + .with_trace_id(trace_id.try_into()?)?; + anyhow::Ok(()) + }) +} + /// # Safety /// The `builder` can be null, but if non-null it must point to a Builder made by this module, /// which has not previously been dropped. @@ -398,7 +434,3 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid_random( anyhow::Ok(()) }) } - -// with_sig_info -// with_span_ids -// with_trace_ids diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index f8411d36b..68a0f5b22 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -1,25 +1,28 @@ // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 +mod builder; mod datatypes; -pub use datatypes::*; +mod metadata; +mod os_info; +mod proc_info; +mod sig_info; +mod span; mod stackframe; -pub use stackframe::*; mod stacktrace; -pub use stacktrace::*; -mod builder; +mod thread_data; + pub mod to_inner; pub use builder::*; -mod metadata; +pub use datatypes::*; pub use metadata::*; -mod os_info; pub use os_info::*; -mod proc_info; pub use proc_info::*; -mod thread_data; -pub use thread_data::*; -mod sig_info; pub use sig_info::*; +pub use span::*; +pub use stackframe::*; +pub use stacktrace::*; +pub use thread_data::*; use anyhow::Context; use ddcommon::Endpoint; diff --git a/crashtracker-ffi/src/crash_info/span.rs b/crashtracker-ffi/src/crash_info/span.rs new file mode 100644 index 000000000..70e822697 --- /dev/null +++ b/crashtracker-ffi/src/crash_info/span.rs @@ -0,0 +1,20 @@ +use anyhow::Context; +use ddcommon_ffi::{slice::AsBytes, CharSlice}; +#[repr(C)] +pub struct Span<'a> { + pub id: CharSlice<'a>, + pub thread_name: CharSlice<'a>, +} + +impl<'a> TryFrom> for datadog_crashtracker::rfc5_crash_info::Span { + type Error = anyhow::Error; + fn try_from(value: Span<'a>) -> anyhow::Result { + Ok(Self { + id: value + .id + .try_to_string_option()? + .context("id cannot be empty")?, + thread_name: value.thread_name.try_to_string_option()?, + }) + } +} diff --git a/crashtracker/src/rfc5_crash_info/builder.rs b/crashtracker/src/rfc5_crash_info/builder.rs index 682425145..62e471f53 100644 --- a/crashtracker/src/rfc5_crash_info/builder.rs +++ b/crashtracker/src/rfc5_crash_info/builder.rs @@ -217,6 +217,15 @@ impl CrashInfoBuilder { self } + pub fn with_span_id(&mut self, span_id: Span) -> anyhow::Result<&mut Self> { + if let Some(ref mut span_ids) = &mut self.span_ids { + span_ids.push(span_id); + } else { + self.span_ids = Some(vec![span_id]); + } + Ok(self) + } + pub fn with_span_ids(&mut self, span_ids: Vec) -> &mut Self { self.span_ids = Some(span_ids); self @@ -250,6 +259,15 @@ impl CrashInfoBuilder { self.with_timestamp(Utc::now()) } + pub fn with_trace_id(&mut self, trace_id: Span) -> anyhow::Result<&mut Self> { + if let Some(ref mut trace_ids) = &mut self.trace_ids { + trace_ids.push(trace_id); + } else { + self.trace_ids = Some(vec![trace_id]); + } + Ok(self) + } + pub fn with_trace_ids(&mut self, trace_ids: Vec) -> &mut Self { self.trace_ids = Some(trace_ids); self diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index 17b3f7adb..dfb62137a 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -18,9 +18,9 @@ pub use metadata::Metadata; pub use os_info::*; pub use proc_info::*; pub use sig_info::*; +pub use spans::*; pub use stacktrace::*; -use crate::rfc5_crash_info::spans::Span; use anyhow::Context; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; From d5000dc80fd0595a8acc2950932c3675ceaa796c Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Wed, 4 Dec 2024 13:45:25 -0500 Subject: [PATCH 14/36] Add handle type, and use it --- crashtracker-ffi/cbindgen.toml | 1 + crashtracker-ffi/src/crash_info/builder.rs | 102 +++++------------- crashtracker-ffi/src/crash_info/mod.rs | 1 - crashtracker-ffi/src/crash_info/stackframe.rs | 79 +++----------- crashtracker-ffi/src/crash_info/stacktrace.rs | 65 ++--------- .../src/crash_info/thread_data.rs | 6 +- crashtracker-ffi/src/crash_info/to_inner.rs | 20 ---- .../libdatadog-crashtracking-receiver.c | 4 +- ddcommon-ffi/src/handle.rs | 60 +++++++++++ ddcommon-ffi/src/lib.rs | 2 + 10 files changed, 118 insertions(+), 222 deletions(-) delete mode 100644 crashtracker-ffi/src/crash_info/to_inner.rs create mode 100644 ddcommon-ffi/src/handle.rs diff --git a/crashtracker-ffi/cbindgen.toml b/crashtracker-ffi/cbindgen.toml index b5571f563..b6267bc93 100644 --- a/crashtracker-ffi/cbindgen.toml +++ b/crashtracker-ffi/cbindgen.toml @@ -34,6 +34,7 @@ renaming_overrides_prefixing = true "Timespec" = "ddog_Timespec" "Vec_Tag" = "ddog_Vec_Tag" "Vec_U8" = "ddog_Vec_U8" +"VoidResult" = "ddog_VoidResult" [export.mangle] rename_types = "PascalCase" diff --git a/crashtracker-ffi/src/crash_info/builder.rs b/crashtracker-ffi/src/crash_info/builder.rs index b06039879..d53ae5ab6 100644 --- a/crashtracker-ffi/src/crash_info/builder.rs +++ b/crashtracker-ffi/src/crash_info/builder.rs @@ -1,63 +1,15 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use super::{to_inner::ToInner, Metadata, OsInfo, ProcInfo, SigInfo, Span, StackTrace, ThreadData}; +use super::{Metadata, OsInfo, ProcInfo, SigInfo, Span, ThreadData}; use ::function_name::named; use anyhow::Context; -use datadog_crashtracker::rfc5_crash_info::ErrorKind; +use datadog_crashtracker::rfc5_crash_info::{CrashInfoBuilder, ErrorKind, StackTrace}; use ddcommon_ffi::{ - slice::AsBytes, wrap_with_ffi_result, CharSlice, Result, Slice, Timespec, VoidResult, + slice::AsBytes, wrap_with_ffi_result, CharSlice, Handle, Result, Slice, Timespec, ToInner, + VoidResult, }; -/// Represents a CrashInfoBuilder. Do not access its member for any reason, only use -/// the C API functions on this struct. -#[repr(C)] -pub struct CrashInfoBuilder { - // This may be null, but if not it will point to a valid CrashInfoBuilder. - inner: *mut datadog_crashtracker::rfc5_crash_info::CrashInfoBuilder, -} - -impl ToInner for CrashInfoBuilder { - type Inner = datadog_crashtracker::rfc5_crash_info::CrashInfoBuilder; - - unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut Self::Inner> { - self.inner - .as_mut() - .context("inner pointer was null, indicates use after free") - } -} - -impl CrashInfoBuilder { - pub(super) fn new() -> Self { - CrashInfoBuilder { - inner: Box::into_raw(Box::new( - datadog_crashtracker::rfc5_crash_info::CrashInfoBuilder::new(), - )), - } - } - - pub(super) fn take( - &mut self, - ) -> Option> { - // Leaving a null will help with double-free issues that can - // arise in C. Of course, it's best to never get there in the - // first place! - let raw = std::mem::replace(&mut self.inner, std::ptr::null_mut()); - - if raw.is_null() { - None - } else { - Some(unsafe { Box::from_raw(raw) }) - } - } -} - -impl Drop for CrashInfoBuilder { - fn drop(&mut self) { - drop(self.take()) - } -} - //////////////////////////////////////////////////////////////////////////////////////////////////// // FFI API // //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -67,15 +19,15 @@ impl Drop for CrashInfoBuilder { /// No safety issues. #[no_mangle] #[must_use] -pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_new() -> Result { - Ok(CrashInfoBuilder::new()).into() +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_new() -> Result> { + ddcommon_ffi::Result::Ok(CrashInfoBuilder::new().into()) } /// # 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. #[no_mangle] -pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_drop(builder: *mut CrashInfoBuilder) { +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_drop(builder: *mut Handle) { // Technically, this function has been designed so if it's double-dropped // then it's okay, but it's not something that should be relied on. if !builder.is_null() { @@ -91,7 +43,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_drop(builder: *mut CrashIn #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_counter( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, name: CharSlice, value: i64, ) -> VoidResult { @@ -111,7 +63,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_counter( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_kind( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, kind: ErrorKind, ) -> VoidResult { wrap_with_ffi_result!({ @@ -128,7 +80,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_kind( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_file( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, filename: CharSlice, contents: Slice, ) -> VoidResult { @@ -158,7 +110,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_file( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_fingerprint( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, fingerprint: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ @@ -177,7 +129,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_fingerprint( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_incomplete( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, incomplete: bool, ) -> VoidResult { wrap_with_ffi_result!({ @@ -194,7 +146,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_incomplete( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_log_message( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, message: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ @@ -213,7 +165,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_log_message( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_metadata( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, metadata: Metadata, ) -> VoidResult { wrap_with_ffi_result!({ @@ -230,7 +182,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_metadata( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_os_info( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, os_info: OsInfo, ) -> VoidResult { wrap_with_ffi_result!({ @@ -247,7 +199,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_os_info( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_os_info_this_machine( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, ) -> VoidResult { wrap_with_ffi_result!({ builder.to_inner_mut()?.with_os_info_this_machine(); @@ -263,7 +215,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_os_info_this_machine( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_proc_info( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, proc_info: ProcInfo, ) -> VoidResult { wrap_with_ffi_result!({ @@ -282,7 +234,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_proc_info( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_sig_info( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, sig_info: SigInfo, ) -> VoidResult { wrap_with_ffi_result!({ @@ -299,7 +251,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_sig_info( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_span_id( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, span_id: Span, ) -> VoidResult { wrap_with_ffi_result!({ @@ -317,8 +269,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_span_id( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_stack( - mut builder: *mut CrashInfoBuilder, - mut stack: StackTrace, + mut builder: *mut Handle, + mut stack: Handle, ) -> VoidResult { wrap_with_ffi_result!({ builder @@ -337,7 +289,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_stack( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_thread( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, thread: ThreadData, ) -> VoidResult { wrap_with_ffi_result!({ @@ -354,7 +306,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_thread( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, ts: Timespec, ) -> VoidResult { wrap_with_ffi_result!({ @@ -371,7 +323,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp_now( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, ) -> VoidResult { wrap_with_ffi_result!({ builder.to_inner_mut()?.with_timestamp_now(); @@ -387,7 +339,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp_now( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_trace_id( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, trace_id: Span, ) -> VoidResult { wrap_with_ffi_result!({ @@ -406,7 +358,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_trace_id( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, uuid: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ @@ -427,7 +379,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid_random( - mut builder: *mut CrashInfoBuilder, + mut builder: *mut Handle, ) -> VoidResult { wrap_with_ffi_result!({ builder.to_inner_mut()?.with_uuid_random(); diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index 68a0f5b22..751662a6f 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -12,7 +12,6 @@ mod stackframe; mod stacktrace; mod thread_data; -pub mod to_inner; pub use builder::*; pub use datatypes::*; pub use metadata::*; diff --git a/crashtracker-ffi/src/crash_info/stackframe.rs b/crashtracker-ffi/src/crash_info/stackframe.rs index 72afb8f91..f45733d5e 100644 --- a/crashtracker-ffi/src/crash_info/stackframe.rs +++ b/crashtracker-ffi/src/crash_info/stackframe.rs @@ -1,59 +1,12 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use super::to_inner::ToInner; use ::function_name::named; use anyhow::Context; -use ddcommon_ffi::{slice::AsBytes, wrap_with_ffi_result, CharSlice, Result, VoidResult}; - -/// Represents a StackFrame. Do not access its member for any reason, only use -/// the C API functions on this struct. -#[repr(C)] -pub struct StackFrame { - // This may be null, but if not it will point to a valid StackFrame. - inner: *mut datadog_crashtracker::rfc5_crash_info::StackFrame, -} - -impl ToInner for StackFrame { - type Inner = datadog_crashtracker::rfc5_crash_info::StackFrame; - - unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut Self::Inner> { - self.inner - .as_mut() - .context("inner pointer was null, indicates use after free") - } -} - -impl StackFrame { - pub(super) fn new() -> Self { - StackFrame { - inner: Box::into_raw(Box::new( - datadog_crashtracker::rfc5_crash_info::StackFrame::new(), - )), - } - } - - pub(super) fn take( - &mut self, - ) -> Option> { - // Leaving a null will help with double-free issues that can - // arise in C. Of course, it's best to never get there in the - // first place! - let raw = std::mem::replace(&mut self.inner, std::ptr::null_mut()); - - if raw.is_null() { - None - } else { - Some(unsafe { Box::from_raw(raw) }) - } - } -} - -impl Drop for StackFrame { - fn drop(&mut self) { - drop(self.take()) - } -} +use datadog_crashtracker::rfc5_crash_info::StackFrame; +use ddcommon_ffi::{ + slice::AsBytes, wrap_with_ffi_result, CharSlice, Handle, Result, ToInner, VoidResult, +}; //////////////////////////////////////////////////////////////////////////////////////////////////// // FFI API // @@ -64,15 +17,15 @@ impl Drop for StackFrame { /// No safety issues. #[no_mangle] #[must_use] -pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> Result { - Ok(StackFrame::new()).into() +pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> Result> { + ddcommon_ffi::Result::Ok(StackFrame::new().into()) } /// # Safety /// The `frame` can be null, but if non-null it must point to a Frame /// made by this module, which has not previously been dropped. #[no_mangle] -pub unsafe extern "C" fn ddog_crasht_StackFrame_drop(frame: *mut StackFrame) { +pub unsafe extern "C" fn ddog_crasht_StackFrame_drop(frame: *mut Handle) { // Technically, this function has been designed so if it's double-dropped // then it's okay, but it's not something that should be relied on. if !frame.is_null() { @@ -88,7 +41,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_drop(frame: *mut StackFrame) { #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip( - mut frame: *mut StackFrame, + mut frame: *mut Handle, ip: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ @@ -106,7 +59,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address( - mut frame: *mut StackFrame, + mut frame: *mut Handle, module_base_address: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ @@ -124,7 +77,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp( - mut frame: *mut StackFrame, + mut frame: *mut Handle, sp: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ @@ -142,7 +95,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address( - mut frame: *mut StackFrame, + mut frame: *mut Handle, symbol_address: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ @@ -160,7 +113,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( - mut frame: *mut StackFrame, + mut frame: *mut Handle, build_id: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ @@ -178,7 +131,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path( - mut frame: *mut StackFrame, + mut frame: *mut Handle, path: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ @@ -196,7 +149,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( - mut frame: *mut StackFrame, + mut frame: *mut Handle, relative_address: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ @@ -214,7 +167,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_file( - mut frame: *mut StackFrame, + mut frame: *mut Handle, file: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ @@ -232,7 +185,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_file( #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackFrame_with_function( - mut frame: *mut StackFrame, + mut frame: *mut Handle, function: CharSlice, ) -> VoidResult { wrap_with_ffi_result!({ diff --git a/crashtracker-ffi/src/crash_info/stacktrace.rs b/crashtracker-ffi/src/crash_info/stacktrace.rs index 845dd74f6..80e318dba 100644 --- a/crashtracker-ffi/src/crash_info/stacktrace.rs +++ b/crashtracker-ffi/src/crash_info/stacktrace.rs @@ -1,58 +1,10 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use super::to_inner::ToInner; -use crate::StackFrame; use ::function_name::named; use anyhow::Context; -use ddcommon_ffi::{wrap_with_ffi_result, Result, VoidResult}; - -/// Represents a StackTrace. Do not access its member for any reason, only use -/// the C API functions on this struct. -#[repr(C)] -pub struct StackTrace { - // This may be null, but if not it will point to a valid StackTrace. - inner: *mut datadog_crashtracker::rfc5_crash_info::StackTrace, -} - -impl ToInner for StackTrace { - type Inner = datadog_crashtracker::rfc5_crash_info::StackTrace; - - unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut Self::Inner> { - self.inner - .as_mut() - .context("inner pointer was null, indicates use after free") - } -} - -impl StackTrace { - pub(super) fn new(crash_info: datadog_crashtracker::rfc5_crash_info::StackTrace) -> Self { - StackTrace { - inner: Box::into_raw(Box::new(crash_info)), - } - } - - pub(super) fn take( - &mut self, - ) -> Option> { - // Leaving a null will help with double-free issues that can - // arise in C. Of course, it's best to never get there in the - // first place! - let raw = std::mem::replace(&mut self.inner, std::ptr::null_mut()); - - if raw.is_null() { - None - } else { - Some(unsafe { Box::from_raw(raw) }) - } - } -} - -impl Drop for StackTrace { - fn drop(&mut self) { - drop(self.take()) - } -} +use datadog_crashtracker::rfc5_crash_info::{StackFrame, StackTrace}; +use ddcommon_ffi::{wrap_with_ffi_result, Handle, Result, ToInner, VoidResult}; //////////////////////////////////////////////////////////////////////////////////////////////////// // FFI API // @@ -63,18 +15,15 @@ impl Drop for StackTrace { /// No safety issues. #[no_mangle] #[must_use] -pub unsafe extern "C" fn ddog_crasht_StackTrace_new() -> Result { - Ok(StackTrace::new( - datadog_crashtracker::rfc5_crash_info::StackTrace::new(), - )) - .into() +pub unsafe extern "C" fn ddog_crasht_StackTrace_new() -> Result> { + ddcommon_ffi::Result::Ok(StackTrace::new().into()) } /// # Safety /// The `frame` can be null, but if non-null it must point to a Frame /// made by this module, which has not previously been dropped. #[no_mangle] -pub unsafe extern "C" fn ddog_crasht_StackTrace_drop(trace: *mut StackTrace) { +pub unsafe extern "C" fn ddog_crasht_StackTrace_drop(trace: *mut Handle) { // Technically, this function has been designed so if it's double-dropped // then it's okay, but it's not something that should be relied on. if !trace.is_null() { @@ -91,8 +40,8 @@ pub unsafe extern "C" fn ddog_crasht_StackTrace_drop(trace: *mut StackTrace) { #[must_use] #[named] pub unsafe extern "C" fn ddog_crasht_StackTrace_push_frame( - mut trace: *mut StackTrace, - frame: *mut StackFrame, + mut trace: *mut Handle, + frame: *mut Handle, ) -> VoidResult { wrap_with_ffi_result!({ let trace = trace.to_inner_mut()?; diff --git a/crashtracker-ffi/src/crash_info/thread_data.rs b/crashtracker-ffi/src/crash_info/thread_data.rs index c0c50a6cb..f4d96abde 100644 --- a/crashtracker-ffi/src/crash_info/thread_data.rs +++ b/crashtracker-ffi/src/crash_info/thread_data.rs @@ -2,15 +2,15 @@ // SPDX-License-Identifier: Apache-2.0 use anyhow::Context; -use ddcommon_ffi::{slice::AsBytes, CharSlice}; +use ddcommon_ffi::{slice::AsBytes, CharSlice, Handle}; -use super::StackTrace; +use datadog_crashtracker::rfc5_crash_info::StackTrace; #[repr(C)] pub struct ThreadData<'a> { pub crashed: bool, pub name: CharSlice<'a>, - pub stack: StackTrace, + pub stack: Handle, pub state: CharSlice<'a>, } diff --git a/crashtracker-ffi/src/crash_info/to_inner.rs b/crashtracker-ffi/src/crash_info/to_inner.rs deleted file mode 100644 index 5eb142426..000000000 --- a/crashtracker-ffi/src/crash_info/to_inner.rs +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ -// SPDX-License-Identifier: Apache-2.0 - -use anyhow::Context; - -pub(crate) trait ToInner { - type Inner; - unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut Self::Inner>; -} - -impl ToInner for *mut T -where - T: ToInner, -{ - type Inner = T::Inner; - unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut Self::Inner> { - let inner = self.as_mut().context("Null pointer")?; - inner.to_inner_mut() - } -} diff --git a/crashtracker/libdatadog-crashtracking-receiver.c b/crashtracker/libdatadog-crashtracking-receiver.c index 3f284e368..bc9e241fd 100644 --- a/crashtracker/libdatadog-crashtracking-receiver.c +++ b/crashtracker/libdatadog-crashtracking-receiver.c @@ -7,8 +7,8 @@ #include int main(void) { - ddog_crasht_VoidResult new_result = ddog_crasht_receiver_entry_point_stdin(); - if (new_result.tag != DDOG_CRASHT_VOID_RESULT_OK) { + ddog_VoidResult new_result = ddog_crasht_receiver_entry_point_stdin(); + if (new_result.tag != DDOG_VOID_RESULT_OK) { ddog_CharSlice message = ddog_Error_message(&new_result.err); fprintf(stderr, "%.*s", (int)message.len, message.ptr); ddog_Error_drop(&new_result.err); diff --git a/ddcommon-ffi/src/handle.rs b/ddcommon-ffi/src/handle.rs new file mode 100644 index 000000000..98e7782a0 --- /dev/null +++ b/ddcommon-ffi/src/handle.rs @@ -0,0 +1,60 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use anyhow::Context; + +/// Represents an object that should only be referred to by its handle. +/// Do not access its member for any reason, only use the C API functions on this struct. +#[repr(C)] +pub struct Handle { + // This may be null, but if not it will point to a valid . + inner: *mut T, +} + +pub trait ToInner { + /// # Safety + /// The Handle must hold a valid `inner` which has been allocated and not freed. + unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut T>; +} + +impl ToInner for *mut Handle { + unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut T> { + self.as_mut().context("Null pointer")?.to_inner_mut() + } +} + +impl ToInner for Handle { + unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut T> { + self.inner + .as_mut() + .context("inner pointer was null, indicates use after free") + } +} + +impl Handle { + pub fn take(&mut self) -> Option> { + // Leaving a null will help with double-free issues that can arise in C. + // Of course, it's best to never get there in the first place! + let raw = std::mem::replace(&mut self.inner, std::ptr::null_mut()); + + if raw.is_null() { + None + } else { + Some(unsafe { Box::from_raw(raw) }) + } + } +} + +impl From for Handle { + fn from(value: T) -> Self { + Self { + inner: Box::into_raw(Box::new(value)), + } + } +} + +impl Drop for Handle { + fn drop(&mut self) { + drop(self.take()) + } +} diff --git a/ddcommon-ffi/src/lib.rs b/ddcommon-ffi/src/lib.rs index 173d96738..98d59957f 100644 --- a/ddcommon-ffi/src/lib.rs +++ b/ddcommon-ffi/src/lib.rs @@ -5,6 +5,7 @@ mod error; pub mod array_queue; pub mod endpoint; +pub mod handle; pub mod option; pub mod result; pub mod slice; @@ -15,6 +16,7 @@ pub mod utils; pub mod vec; pub use error::*; +pub use handle::*; pub use option::*; pub use result::*; pub use slice::{CharSlice, Slice}; From e5c70321ed86dafbe8ae14bbea2bb86c24d6e98e Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Wed, 4 Dec 2024 15:05:20 -0500 Subject: [PATCH 15/36] make handle.take() a little nicer --- crashtracker-ffi/src/crash_info/builder.rs | 23 ++++-- crashtracker-ffi/src/crash_info/datatypes.rs | 57 +------------ crashtracker-ffi/src/crash_info/mod.rs | 82 +++++-------------- crashtracker-ffi/src/crash_info/stacktrace.rs | 10 +-- .../src/crash_info/thread_data.rs | 8 +- ddcommon-ffi/src/handle.rs | 24 +++--- 6 files changed, 57 insertions(+), 147 deletions(-) diff --git a/crashtracker-ffi/src/crash_info/builder.rs b/crashtracker-ffi/src/crash_info/builder.rs index d53ae5ab6..8608ab997 100644 --- a/crashtracker-ffi/src/crash_info/builder.rs +++ b/crashtracker-ffi/src/crash_info/builder.rs @@ -4,7 +4,7 @@ use super::{Metadata, OsInfo, ProcInfo, SigInfo, Span, ThreadData}; use ::function_name::named; use anyhow::Context; -use datadog_crashtracker::rfc5_crash_info::{CrashInfoBuilder, ErrorKind, StackTrace}; +use datadog_crashtracker::rfc5_crash_info::{CrashInfo, CrashInfoBuilder, ErrorKind, StackTrace}; use ddcommon_ffi::{ slice::AsBytes, wrap_with_ffi_result, CharSlice, Handle, Result, Slice, Timespec, ToInner, VoidResult, @@ -35,6 +35,21 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_drop(builder: *mut Handle< } } +/// # Safety +/// The `builder` 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_CrashInfoBuilder_build( + mut builder: *mut Handle, +) -> Result> { + wrap_with_ffi_result!({ + anyhow::ensure!(!builder.is_null()); + Ok(builder.take()?.build()?.into()) + }) +} + /// # Safety /// The `builder` can be null, but if non-null it must point to a Builder made by this module, /// which has not previously been dropped. @@ -270,12 +285,10 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_span_id( #[named] pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_stack( mut builder: *mut Handle, - mut stack: Handle, + mut stack: *mut Handle, ) -> VoidResult { wrap_with_ffi_result!({ - builder - .to_inner_mut()? - .with_stack(*stack.take().context("Stack was empty. Use after free?")?); + builder.to_inner_mut()?.with_stack(*stack.take()?); anyhow::Ok(()) }) } diff --git a/crashtracker-ffi/src/crash_info/datatypes.rs b/crashtracker-ffi/src/crash_info/datatypes.rs index 76c25d165..2139d9db2 100644 --- a/crashtracker-ffi/src/crash_info/datatypes.rs +++ b/crashtracker-ffi/src/crash_info/datatypes.rs @@ -3,64 +3,9 @@ use ddcommon_ffi::{ slice::{AsBytes, ByteSlice}, - CharSlice, Error, Slice, + CharSlice, Slice, }; -/// Represents a CrashInfo. Do not access its member for any reason, only use -/// the C API functions on this struct. -#[repr(C)] -pub struct CrashInfo { - // This may be null, but if not it will point to a valid CrashInfo. - inner: *mut datadog_crashtracker::CrashInfo, -} - -impl CrashInfo { - pub(super) fn new(crash_info: datadog_crashtracker::CrashInfo) -> Self { - CrashInfo { - inner: Box::into_raw(Box::new(crash_info)), - } - } - - pub(super) fn take(&mut self) -> Option> { - // Leaving a null will help with double-free issues that can - // arise in C. Of course, it's best to never get there in the - // first place! - let raw = std::mem::replace(&mut self.inner, std::ptr::null_mut()); - - if raw.is_null() { - None - } else { - Some(unsafe { Box::from_raw(raw) }) - } - } -} - -impl Drop for CrashInfo { - fn drop(&mut self) { - drop(self.take()) - } -} - -pub(crate) unsafe fn crashinfo_ptr_to_inner<'a>( - crashinfo_ptr: *mut CrashInfo, -) -> anyhow::Result<&'a mut datadog_crashtracker::CrashInfo> { - match crashinfo_ptr.as_mut() { - None => anyhow::bail!("crashinfo pointer was null"), - Some(inner_ptr) => match inner_ptr.inner.as_mut() { - Some(crashinfo) => Ok(crashinfo), - None => anyhow::bail!("crashinfo's inner pointer was null (indicates use-after-free)"), - }, - } -} - -/// Returned by [ddog_prof_Profile_new]. -#[repr(C)] -pub enum CrashInfoNewResult { - Ok(CrashInfo), - #[allow(dead_code)] - Err(Error), -} - #[repr(C)] #[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] pub enum NormalizedAddressTypes { diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index 751662a6f..20f213d4c 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -23,66 +23,22 @@ pub use stackframe::*; pub use stacktrace::*; pub use thread_data::*; -use anyhow::Context; -use ddcommon::Endpoint; -use ddcommon_ffi::VoidResult; - -/// Create a new crashinfo, and returns an opaque reference to it. -/// # Safety -/// No safety issues. -#[no_mangle] -#[must_use] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_new() -> CrashInfoNewResult { - CrashInfoNewResult::Ok(CrashInfo::new(datadog_crashtracker::CrashInfo::new())) -} - -/// # Safety -/// The `crash_info` can be null, but if non-null it must point to a CrashInfo -/// made by this module, which has not previously been dropped. -#[no_mangle] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_drop(crashinfo: *mut CrashInfo) { - // Technically, this function has been designed so if it's double-dropped - // then it's okay, but it's not something that should be relied on. - if !crashinfo.is_null() { - drop((*crashinfo).take()) - } -} - -/// 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_crasht_CrashInfo_normalize_ips( - crashinfo: *mut CrashInfo, - pid: u32, -) -> VoidResult { - (|| { - let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; - crashinfo.normalize_ips(pid) - })() - .context("ddog_crasht_CrashInfo_normalize_ips failed") - .into() -} - -/// Exports `crashinfo` to the backend at `endpoint` -/// Note that we support the "file://" endpoint for local file output. -/// # Safety -/// `crashinfo` must be a valid pointer to a `CrashInfo` object. -#[no_mangle] -#[must_use] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_upload_to_endpoint( - crashinfo: *mut CrashInfo, - endpoint: Option<&Endpoint>, -) -> VoidResult { - (|| { - let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; - let endpoint = endpoint.cloned(); - crashinfo.upload_to_endpoint(&endpoint) - })() - .context("ddog_crasht_CrashInfo_upload_to_endpoint failed") - .into() -} +// /// 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_crasht_CrashInfo_normalize_ips( +// crashinfo: *mut CrashInfo, +// pid: u32, +// ) -> VoidResult { +// (|| { +// let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; +// crashinfo.normalize_ips(pid) +// })() +// .context("ddog_crasht_CrashInfo_normalize_ips failed") +// .into() +// } diff --git a/crashtracker-ffi/src/crash_info/stacktrace.rs b/crashtracker-ffi/src/crash_info/stacktrace.rs index 80e318dba..e6283e769 100644 --- a/crashtracker-ffi/src/crash_info/stacktrace.rs +++ b/crashtracker-ffi/src/crash_info/stacktrace.rs @@ -41,16 +41,10 @@ pub unsafe extern "C" fn ddog_crasht_StackTrace_drop(trace: *mut Handle, - frame: *mut Handle, + mut frame: *mut Handle, ) -> VoidResult { wrap_with_ffi_result!({ - let trace = trace.to_inner_mut()?; - let frame = *frame - .as_mut() - .context("Null frame pointer")? - .take() - .context("Frame had null inner pointer. Use after free?")?; - trace.frames.push(frame); + trace.to_inner_mut()?.frames.push(*frame.take()?); anyhow::Ok(()) }) } diff --git a/crashtracker-ffi/src/crash_info/thread_data.rs b/crashtracker-ffi/src/crash_info/thread_data.rs index f4d96abde..f451ea892 100644 --- a/crashtracker-ffi/src/crash_info/thread_data.rs +++ b/crashtracker-ffi/src/crash_info/thread_data.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use anyhow::Context; -use ddcommon_ffi::{slice::AsBytes, CharSlice, Handle}; +use ddcommon_ffi::{slice::AsBytes, CharSlice, Handle, ToInner}; use datadog_crashtracker::rfc5_crash_info::StackTrace; @@ -22,10 +22,8 @@ impl<'a> TryFrom> for datadog_crashtracker::rfc5_crash_info::Thre .name .try_to_string_option()? .context("Name cannot be empty")?; - let stack = *value - .stack - .take() - .context("missing stack, use after free?")?; + // Safety: Handles should only be created using functions that leave them in a valid state. + let stack = unsafe { *value.stack.take()? }; let state = value.state.try_to_string_option()?; Ok(Self { diff --git a/ddcommon-ffi/src/handle.rs b/ddcommon-ffi/src/handle.rs index 98e7782a0..c71cca5d5 100644 --- a/ddcommon-ffi/src/handle.rs +++ b/ddcommon-ffi/src/handle.rs @@ -15,12 +15,19 @@ pub trait ToInner { /// # Safety /// The Handle must hold a valid `inner` which has been allocated and not freed. unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut T>; + /// # Safety + /// The Handle must hold a valid `inner` [return OK(inner)], or null [returns Error]. + unsafe fn take(&mut self) -> anyhow::Result>; } impl ToInner for *mut Handle { unsafe fn to_inner_mut(&mut self) -> anyhow::Result<&mut T> { self.as_mut().context("Null pointer")?.to_inner_mut() } + + unsafe fn take(&mut self) -> anyhow::Result> { + self.as_mut().context("Null pointer")?.take() + } } impl ToInner for Handle { @@ -29,19 +36,16 @@ impl ToInner for Handle { .as_mut() .context("inner pointer was null, indicates use after free") } -} -impl Handle { - pub fn take(&mut self) -> Option> { + unsafe fn take(&mut self) -> anyhow::Result> { // Leaving a null will help with double-free issues that can arise in C. // Of course, it's best to never get there in the first place! let raw = std::mem::replace(&mut self.inner, std::ptr::null_mut()); - - if raw.is_null() { - None - } else { - Some(unsafe { Box::from_raw(raw) }) - } + anyhow::ensure!( + !raw.is_null(), + "inner pointer was null, indicates use after free" + ); + Ok(Box::from_raw(raw)) } } @@ -55,6 +59,6 @@ impl From for Handle { impl Drop for Handle { fn drop(&mut self) { - drop(self.take()) + drop(unsafe { self.take() }) } } From 00ae5ea8010bc5e3214353bba1cc1673d92f46cb Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Wed, 4 Dec 2024 15:18:25 -0500 Subject: [PATCH 16/36] sharpen the wrapper macros --- crashtracker-ffi/src/crash_info/builder.rs | 62 +++++++------------ crashtracker-ffi/src/crash_info/stackframe.rs | 57 ++++++----------- crashtracker-ffi/src/crash_info/stacktrace.rs | 6 +- ddcommon-ffi/src/utils.rs | 24 +++++-- 4 files changed, 62 insertions(+), 87 deletions(-) diff --git a/crashtracker-ffi/src/crash_info/builder.rs b/crashtracker-ffi/src/crash_info/builder.rs index 8608ab997..e0091db71 100644 --- a/crashtracker-ffi/src/crash_info/builder.rs +++ b/crashtracker-ffi/src/crash_info/builder.rs @@ -3,11 +3,10 @@ use super::{Metadata, OsInfo, ProcInfo, SigInfo, Span, ThreadData}; use ::function_name::named; -use anyhow::Context; use datadog_crashtracker::rfc5_crash_info::{CrashInfo, CrashInfoBuilder, ErrorKind, StackTrace}; use ddcommon_ffi::{ - slice::AsBytes, wrap_with_ffi_result, CharSlice, Handle, Result, Slice, Timespec, ToInner, - VoidResult, + slice::AsBytes, wrap_with_ffi_result, wrap_with_void_ffi_result, CharSlice, Handle, Result, + Slice, Timespec, ToInner, VoidResult, }; //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -62,11 +61,10 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_counter( name: CharSlice, value: i64, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder .to_inner_mut()? .with_counter(name.try_to_utf8()?.to_string(), value)?; - anyhow::Ok(()) }) } @@ -81,9 +79,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_kind( mut builder: *mut Handle, kind: ErrorKind, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_kind(kind)?; - anyhow::Ok(()) }) } @@ -99,7 +96,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_file( filename: CharSlice, contents: Slice, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ let filename = filename .try_to_string_option()? .context("filename cannot be empty string")?; @@ -113,7 +110,6 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_file( }; builder.to_inner_mut()?.with_file(filename, contents); - anyhow::Ok(()) }) } @@ -128,11 +124,10 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_fingerprint( mut builder: *mut Handle, fingerprint: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder .to_inner_mut()? .with_fingerprint(fingerprint.try_to_utf8()?.to_string())?; - anyhow::Ok(()) }) } @@ -147,9 +142,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_incomplete( mut builder: *mut Handle, incomplete: bool, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_incomplete(incomplete); - anyhow::Ok(()) }) } @@ -164,11 +158,10 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_log_message( mut builder: *mut Handle, message: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder .to_inner_mut()? .with_log_message(message.try_to_utf8()?.to_string())?; - anyhow::Ok(()) }) } @@ -183,9 +176,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_metadata( mut builder: *mut Handle, metadata: Metadata, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_metadata(metadata.try_into()?); - anyhow::Ok(()) }) } @@ -200,9 +192,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_os_info( mut builder: *mut Handle, os_info: OsInfo, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_os_info(os_info.try_into()?); - anyhow::Ok(()) }) } @@ -216,9 +207,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_os_info( pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_os_info_this_machine( mut builder: *mut Handle, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_os_info_this_machine(); - anyhow::Ok(()) }) } @@ -233,11 +223,10 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_proc_info( mut builder: *mut Handle, proc_info: ProcInfo, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder .to_inner_mut()? .with_proc_info(proc_info.try_into()?); - anyhow::Ok(()) }) } @@ -252,9 +241,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_sig_info( mut builder: *mut Handle, sig_info: SigInfo, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_sig_info(sig_info.try_into()?); - anyhow::Ok(()) }) } @@ -269,9 +257,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_span_id( mut builder: *mut Handle, span_id: Span, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_span_id(span_id.try_into()?)?; - anyhow::Ok(()) }) } @@ -287,9 +274,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_stack( mut builder: *mut Handle, mut stack: *mut Handle, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_stack(*stack.take()?); - anyhow::Ok(()) }) } @@ -305,9 +291,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_thread( mut builder: *mut Handle, thread: ThreadData, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_thread(thread.try_into()?)?; - anyhow::Ok(()) }) } @@ -322,9 +307,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp( mut builder: *mut Handle, ts: Timespec, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_timestamp(ts.into()); - anyhow::Ok(()) }) } @@ -338,9 +322,8 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp( pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp_now( mut builder: *mut Handle, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_timestamp_now(); - anyhow::Ok(()) }) } @@ -355,11 +338,10 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_trace_id( mut builder: *mut Handle, trace_id: Span, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder .to_inner_mut()? .with_trace_id(trace_id.try_into()?)?; - anyhow::Ok(()) }) } @@ -374,13 +356,12 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid( mut builder: *mut Handle, uuid: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ let builder = builder.to_inner_mut()?; let uuid = uuid .try_to_string_option()? .context("UUID cannot be empty string")?; builder.with_uuid(uuid); - anyhow::Ok(()) }) } @@ -394,8 +375,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid( pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid_random( mut builder: *mut Handle, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ builder.to_inner_mut()?.with_uuid_random(); - anyhow::Ok(()) }) } diff --git a/crashtracker-ffi/src/crash_info/stackframe.rs b/crashtracker-ffi/src/crash_info/stackframe.rs index f45733d5e..7ae448dcf 100644 --- a/crashtracker-ffi/src/crash_info/stackframe.rs +++ b/crashtracker-ffi/src/crash_info/stackframe.rs @@ -2,10 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 use ::function_name::named; -use anyhow::Context; use datadog_crashtracker::rfc5_crash_info::StackFrame; use ddcommon_ffi::{ - slice::AsBytes, wrap_with_ffi_result, CharSlice, Handle, Result, ToInner, VoidResult, + slice::AsBytes, wrap_with_void_ffi_result, CharSlice, Handle, Result, ToInner, VoidResult, }; //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -44,10 +43,8 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip( mut frame: *mut Handle, ip: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ - let frame = frame.to_inner_mut()?; - frame.ip = ip.try_to_string_option()?; - anyhow::Ok(()) + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.ip = ip.try_to_string_option()?; }) } @@ -62,10 +59,8 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address( mut frame: *mut Handle, module_base_address: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ - let frame = frame.to_inner_mut()?; - frame.module_base_address = module_base_address.try_to_string_option()?; - anyhow::Ok(()) + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.module_base_address = module_base_address.try_to_string_option()?; }) } @@ -80,10 +75,8 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp( mut frame: *mut Handle, sp: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ - let frame = frame.to_inner_mut()?; - frame.sp = sp.try_to_string_option()?; - anyhow::Ok(()) + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.sp = sp.try_to_string_option()?; }) } @@ -98,10 +91,8 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address( mut frame: *mut Handle, symbol_address: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ - let frame = frame.to_inner_mut()?; - frame.symbol_address = symbol_address.try_to_string_option()?; - anyhow::Ok(()) + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.symbol_address = symbol_address.try_to_string_option()?; }) } @@ -116,10 +107,8 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( mut frame: *mut Handle, build_id: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ - let frame = frame.to_inner_mut()?; - frame.build_id = build_id.try_to_string_option()?; - anyhow::Ok(()) + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.build_id = build_id.try_to_string_option()?; }) } @@ -134,10 +123,8 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path( mut frame: *mut Handle, path: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ - let frame = frame.to_inner_mut()?; - frame.path = path.try_to_string_option()?; - anyhow::Ok(()) + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.path = path.try_to_string_option()?; }) } @@ -152,10 +139,8 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( mut frame: *mut Handle, relative_address: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ - let frame = frame.to_inner_mut()?; - frame.relative_address = relative_address.try_to_string_option()?; - anyhow::Ok(()) + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.relative_address = relative_address.try_to_string_option()?; }) } @@ -170,10 +155,8 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_file( mut frame: *mut Handle, file: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ - let frame = frame.to_inner_mut()?; - frame.file = file.try_to_string_option()?; - anyhow::Ok(()) + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.file = file.try_to_string_option()?; }) } @@ -188,9 +171,7 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_function( mut frame: *mut Handle, function: CharSlice, ) -> VoidResult { - wrap_with_ffi_result!({ - let frame = frame.to_inner_mut()?; - frame.function = function.try_to_string_option()?; - anyhow::Ok(()) + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.function = function.try_to_string_option()?; }) } diff --git a/crashtracker-ffi/src/crash_info/stacktrace.rs b/crashtracker-ffi/src/crash_info/stacktrace.rs index e6283e769..6505a565d 100644 --- a/crashtracker-ffi/src/crash_info/stacktrace.rs +++ b/crashtracker-ffi/src/crash_info/stacktrace.rs @@ -2,9 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 use ::function_name::named; -use anyhow::Context; use datadog_crashtracker::rfc5_crash_info::{StackFrame, StackTrace}; -use ddcommon_ffi::{wrap_with_ffi_result, Handle, Result, ToInner, VoidResult}; +use ddcommon_ffi::{wrap_with_void_ffi_result, Handle, Result, ToInner, VoidResult}; //////////////////////////////////////////////////////////////////////////////////////////////////// // FFI API // @@ -43,8 +42,7 @@ pub unsafe extern "C" fn ddog_crasht_StackTrace_push_frame( mut trace: *mut Handle, mut frame: *mut Handle, ) -> VoidResult { - wrap_with_ffi_result!({ + wrap_with_void_ffi_result!({ trace.to_inner_mut()?.frames.push(*frame.take()?); - anyhow::Ok(()) }) } diff --git a/ddcommon-ffi/src/utils.rs b/ddcommon-ffi/src/utils.rs index 4a6bbe3db..e2366581b 100644 --- a/ddcommon-ffi/src/utils.rs +++ b/ddcommon-ffi/src/utils.rs @@ -1,11 +1,27 @@ /// Wraps a C-FFI function in standard form -/// Expects the function to return a result type that implements into -/// and to be decorated with #[named]. +/// Expects the function to return a result type that implements into and to be decorated with +/// #[named]. #[macro_export] macro_rules! wrap_with_ffi_result { - ($body:block) => { + ($body:block) => {{ + use anyhow::Context; (|| $body)() .context(concat!(function_name!(), " failed")) .into() - }; + }}; +} + +/// Wraps a C-FFI function in standard form. +/// Expects the function to return a VoidResult and to be decorated with #[named]. +#[macro_export] +macro_rules! wrap_with_void_ffi_result { + ($body:block) => {{ + use anyhow::Context; + (|| { + $body; + anyhow::Ok(()) + })() + .context(concat!(function_name!(), " failed")) + .into() + }}; } From 5969cb0183ca60ae0a98739de2b6e1daee807bde Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Wed, 4 Dec 2024 16:01:37 -0500 Subject: [PATCH 17/36] add to_endpoint functions --- crashtracker-ffi/src/crash_info/api.rs | 33 ++++++++++++++++++++ crashtracker-ffi/src/crash_info/builder.rs | 8 ++--- crashtracker-ffi/src/crash_info/mod.rs | 2 ++ crashtracker/src/rfc5_crash_info/mod.rs | 36 ++++++++++++++++++++++ 4 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 crashtracker-ffi/src/crash_info/api.rs diff --git a/crashtracker-ffi/src/crash_info/api.rs b/crashtracker-ffi/src/crash_info/api.rs new file mode 100644 index 000000000..baefabebc --- /dev/null +++ b/crashtracker-ffi/src/crash_info/api.rs @@ -0,0 +1,33 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +use datadog_crashtracker::rfc5_crash_info::CrashInfo; +use ddcommon_ffi::{wrap_with_void_ffi_result, Handle, ToInner, VoidResult}; +use function_name::named; + +/// # 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. +#[no_mangle] +pub unsafe extern "C" fn ddog_crasht_CrashInfo_drop(builder: *mut Handle) { + // Technically, this function has been designed so if it's double-dropped + // then it's okay, but it's not something that should be relied on. + if !builder.is_null() { + drop((*builder).take()) + } +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfo_upload_to_telemetry( + mut info: *mut Handle, +) -> VoidResult { + wrap_with_void_ffi_result!({ + let _info = info.to_inner_mut()?; + }) +} diff --git a/crashtracker-ffi/src/crash_info/builder.rs b/crashtracker-ffi/src/crash_info/builder.rs index e0091db71..f7e62efa0 100644 --- a/crashtracker-ffi/src/crash_info/builder.rs +++ b/crashtracker-ffi/src/crash_info/builder.rs @@ -43,10 +43,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_drop(builder: *mut Handle< pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_build( mut builder: *mut Handle, ) -> Result> { - wrap_with_ffi_result!({ - anyhow::ensure!(!builder.is_null()); - Ok(builder.take()?.build()?.into()) - }) + wrap_with_ffi_result!({ anyhow::Ok(builder.take()?.build()?.into()) }) } /// # Safety @@ -357,11 +354,10 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid( uuid: CharSlice, ) -> VoidResult { wrap_with_void_ffi_result!({ - let builder = builder.to_inner_mut()?; let uuid = uuid .try_to_string_option()? .context("UUID cannot be empty string")?; - builder.with_uuid(uuid); + builder.to_inner_mut()?.with_uuid(uuid); }) } diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index 20f213d4c..cc73a28cd 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -1,6 +1,7 @@ // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 +mod api; mod builder; mod datatypes; mod metadata; @@ -12,6 +13,7 @@ mod stackframe; mod stacktrace; mod thread_data; +pub use api::*; pub use builder::*; pub use datatypes::*; pub use metadata::*; diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index dfb62137a..458bcb0be 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -9,10 +9,12 @@ mod proc_info; mod sig_info; mod spans; mod stacktrace; +mod telemetry; mod test_utils; mod unknown_value; pub use builder::*; +use ddcommon::Endpoint; pub use error_data::*; pub use metadata::Metadata; pub use os_info::*; @@ -20,6 +22,7 @@ pub use proc_info::*; pub use sig_info::*; pub use spans::*; pub use stacktrace::*; +pub use telemetry::*; use anyhow::Context; use schemars::JsonSchema; @@ -135,6 +138,39 @@ impl CrashInfo { .with_context(|| format!("Failed to write json to {}", path.display()))?; Ok(()) } + + pub fn upload_to_endpoint(&self, endpoint: &Option) -> anyhow::Result<()> { + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build()?; + + rt.block_on(async { self.async_upload_to_endpoint(endpoint).await }) + } + + pub async fn async_upload_to_endpoint( + &self, + endpoint: &Option, + ) -> anyhow::Result<()> { + // If we're debugging to a file, dump the actual crashinfo into a json + if let Some(endpoint) = endpoint { + if Some("file") == endpoint.url.scheme_str() { + let path = ddcommon::decode_uri_path_in_authority(&endpoint.url) + .context("crash output file was not correctly formatted")?; + self.to_file(&path)?; + let new_path = path.with_extension("rfc5.json"); + let rfc5: crate::rfc5_crash_info::CrashInfo = self.clone().into(); + rfc5.to_file(&new_path)?; + } + } + + self.upload_to_telemetry(endpoint).await + } + + async fn upload_to_telemetry(&self, endpoint: &Option) -> anyhow::Result<()> { + let uploader = TelemetryCrashUploader::new(&self.metadata, endpoint)?; + uploader.upload_to_telemetry(self).await?; + Ok(()) + } } #[cfg(test)] From c747f5f4b4eea0d28e025bd9ecb6febfd8910885 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Wed, 4 Dec 2024 18:09:23 -0500 Subject: [PATCH 18/36] normalizer --- crashtracker-ffi/src/crash_info/api.rs | 27 ++++++++++-- crashtracker-ffi/src/crash_info/mod.rs | 20 --------- .../src/rfc5_crash_info/error_data.rs | 13 ++++++ crashtracker/src/rfc5_crash_info/mod.rs | 9 ++-- .../src/rfc5_crash_info/stacktrace.rs | 43 +++++++++++++++++++ 5 files changed, 84 insertions(+), 28 deletions(-) diff --git a/crashtracker-ffi/src/crash_info/api.rs b/crashtracker-ffi/src/crash_info/api.rs index baefabebc..e8d297a99 100644 --- a/crashtracker-ffi/src/crash_info/api.rs +++ b/crashtracker-ffi/src/crash_info/api.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use datadog_crashtracker::rfc5_crash_info::CrashInfo; +use ddcommon::Endpoint; use ddcommon_ffi::{wrap_with_void_ffi_result, Handle, ToInner, VoidResult}; use function_name::named; @@ -18,16 +19,34 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfo_drop(builder: *mut Handle, + pid: u32, +) -> VoidResult { + wrap_with_void_ffi_result!({ + crash_info.to_inner_mut()?.normalize_ips(pid)?; + }) +} + +/// # 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. /// The CharSlice must be valid. #[no_mangle] #[must_use] #[named] -pub unsafe extern "C" fn ddog_crasht_CrashInfo_upload_to_telemetry( - mut info: *mut Handle, +pub unsafe extern "C" fn ddog_crasht_CrashInfo_upload_to_endpoint( + mut crash_info: *mut Handle, + endpoint: Option<&Endpoint>, ) -> VoidResult { wrap_with_void_ffi_result!({ - let _info = info.to_inner_mut()?; + crash_info + .to_inner_mut()? + .upload_to_endpoint(&endpoint.cloned())?; }) } diff --git a/crashtracker-ffi/src/crash_info/mod.rs b/crashtracker-ffi/src/crash_info/mod.rs index cc73a28cd..3d1a56f36 100644 --- a/crashtracker-ffi/src/crash_info/mod.rs +++ b/crashtracker-ffi/src/crash_info/mod.rs @@ -24,23 +24,3 @@ pub use span::*; pub use stackframe::*; pub use stacktrace::*; pub use thread_data::*; - -// /// 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_crasht_CrashInfo_normalize_ips( -// crashinfo: *mut CrashInfo, -// pid: u32, -// ) -> VoidResult { -// (|| { -// let crashinfo = crashinfo_ptr_to_inner(crashinfo)?; -// crashinfo.normalize_ips(pid) -// })() -// .context("ddog_crasht_CrashInfo_normalize_ips failed") -// .into() -// } diff --git a/crashtracker/src/rfc5_crash_info/error_data.rs b/crashtracker/src/rfc5_crash_info/error_data.rs index c2bfde670..a7a4688bd 100644 --- a/crashtracker/src/rfc5_crash_info/error_data.rs +++ b/crashtracker/src/rfc5_crash_info/error_data.rs @@ -17,6 +17,19 @@ pub struct ErrorData { pub threads: Vec, } +impl ErrorData { + pub fn normalize_ips(&mut self, pid: u32) -> anyhow::Result<()> { + let normalizer = blazesym::normalize::Normalizer::new(); + let pid = pid.into(); + // TODO, should we continue after error or just exit? + self.stack.normalize_ips(&normalizer, pid)?; + for thread in &mut self.threads { + thread.stack.normalize_ips(&normalizer, pid)?; + } + Ok(()) + } +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] pub enum SourceType { Crashtracking, diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index 458bcb0be..b44fb2e0d 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -58,6 +58,10 @@ impl CrashInfo { pub fn current_schema_version() -> String { "1.0".to_string() } + + pub fn normalize_ips(&mut self, pid: u32) -> anyhow::Result<()> { + self.error.normalize_ips(pid) + } } impl From for CrashInfo { @@ -155,11 +159,8 @@ impl CrashInfo { if let Some(endpoint) = endpoint { if Some("file") == endpoint.url.scheme_str() { let path = ddcommon::decode_uri_path_in_authority(&endpoint.url) - .context("crash output file was not correctly formatted")?; + .context("crash output file path was not correctly formatted")?; self.to_file(&path)?; - let new_path = path.with_extension("rfc5.json"); - let rfc5: crate::rfc5_crash_info::CrashInfo = self.clone().into(); - rfc5.to_file(&new_path)?; } } diff --git a/crashtracker/src/rfc5_crash_info/stacktrace.rs b/crashtracker/src/rfc5_crash_info/stacktrace.rs index 38248aaa4..be9129b0a 100644 --- a/crashtracker/src/rfc5_crash_info/stacktrace.rs +++ b/crashtracker/src/rfc5_crash_info/stacktrace.rs @@ -1,6 +1,8 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 +use anyhow::Context; +use blazesym::{helper::ElfResolver, normalize::Normalizer, symbolize::TranslateFileOffset, Pid}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -19,6 +21,14 @@ impl StackTrace { frames: vec![], } } + + pub fn normalize_ips(&mut self, normalizer: &Normalizer, pid: Pid) -> anyhow::Result<()> { + for frame in &mut self.frames { + // TODO: Should this keep going on failure, and report at the end? + frame.normalize_ip(normalizer, pid)?; + } + Ok(()) + } } impl Default for StackTrace { @@ -164,6 +174,29 @@ impl StackFrame { pub fn new() -> Self { Self::default() } + + 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]; + let elf = meta.as_elf().context("Not elf")?; + let resolver = ElfResolver::open(&elf.path)?; + let virt_address = resolver + .file_offset_to_virt_offset(file_offset)? + .context("No matching segment found")?; + + self.build_id = elf.build_id.as_ref().map(|x| byte_slice_as_hex(x.as_ref())); + self.build_id_type = Some(BuildIdType::GNU); + self.file_type = Some(FileType::ELF); + self.path = Some(elf.path.to_string_lossy().to_string()); + self.relative_address = Some(format!("{virt_address:#018x}")); + } + Ok(()) + } } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] @@ -200,6 +233,16 @@ fn byte_vec_as_hex(bv: Option>) -> Option { } } +fn byte_slice_as_hex(bv: &[u8]) -> String { + use std::fmt::Write; + + let mut s = String::new(); + for byte in bv { + let _ = write!(&mut s, "{byte:X}"); + } + s +} + #[cfg(test)] impl super::test_utils::TestInstance for StackTrace { fn test_instance(_seed: u64) -> Self { From e1d6cd79f345f383650bf2f151fb05cd4ec51217 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Thu, 5 Dec 2024 12:11:44 -0500 Subject: [PATCH 19/36] resolve names --- crashtracker-ffi/src/crash_info/api.rs | 15 ++++++++ .../src/rfc5_crash_info/error_data.rs | 14 +++++++ crashtracker/src/rfc5_crash_info/mod.rs | 4 ++ .../src/rfc5_crash_info/stacktrace.rs | 37 ++++++++++++++++++- 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/crashtracker-ffi/src/crash_info/api.rs b/crashtracker-ffi/src/crash_info/api.rs index e8d297a99..00b93aea4 100644 --- a/crashtracker-ffi/src/crash_info/api.rs +++ b/crashtracker-ffi/src/crash_info/api.rs @@ -33,6 +33,21 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfo_normalize_ips( }) } +/// # 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_resolve_names( + mut crash_info: *mut Handle, + pid: u32, +) -> VoidResult { + wrap_with_void_ffi_result!({ + crash_info.to_inner_mut()?.resolve_names(pid)?; + }) +} + /// # 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. diff --git a/crashtracker/src/rfc5_crash_info/error_data.rs b/crashtracker/src/rfc5_crash_info/error_data.rs index a7a4688bd..77ea64c57 100644 --- a/crashtracker/src/rfc5_crash_info/error_data.rs +++ b/crashtracker/src/rfc5_crash_info/error_data.rs @@ -28,6 +28,20 @@ impl ErrorData { } Ok(()) } + + pub fn resolve_names(&mut self, pid: u32) -> anyhow::Result<()> { + let mut process = blazesym::symbolize::Process::new(pid.into()); + // https://github.com/libbpf/blazesym/issues/518 + process.map_files = false; + let src = blazesym::symbolize::Source::Process(process); + let symbolizer = blazesym::symbolize::Symbolizer::new(); + self.stack.resolve_names(&src, &symbolizer)?; + + for thread in &mut self.threads { + thread.stack.resolve_names(&src, &symbolizer)?; + } + Ok(()) + } } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index b44fb2e0d..13a425f98 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -62,6 +62,10 @@ impl CrashInfo { pub fn normalize_ips(&mut self, pid: u32) -> anyhow::Result<()> { self.error.normalize_ips(pid) } + + pub fn resolve_names(&mut self, pid: u32) -> anyhow::Result<()> { + self.error.resolve_names(pid) + } } impl From for CrashInfo { diff --git a/crashtracker/src/rfc5_crash_info/stacktrace.rs b/crashtracker/src/rfc5_crash_info/stacktrace.rs index be9129b0a..6ea3cf5aa 100644 --- a/crashtracker/src/rfc5_crash_info/stacktrace.rs +++ b/crashtracker/src/rfc5_crash_info/stacktrace.rs @@ -2,7 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 use anyhow::Context; -use blazesym::{helper::ElfResolver, normalize::Normalizer, symbolize::TranslateFileOffset, Pid}; +use blazesym::{ + helper::ElfResolver, + normalize::Normalizer, + symbolize::{Input, Source, Symbolized, Symbolizer, TranslateFileOffset}, + Pid, +}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -29,6 +34,14 @@ impl StackTrace { } Ok(()) } + + pub fn resolve_names(&mut self, src: &Source, symbolizer: &Symbolizer) -> anyhow::Result<()> { + for frame in &mut self.frames { + // TODO: Should this keep going on failure, and report at the end? + frame.resolve_names(src, symbolizer)?; + } + Ok(()) + } } impl Default for StackTrace { @@ -197,6 +210,28 @@ impl StackFrame { } Ok(()) } + + pub fn resolve_names(&mut self, src: &Source, symbolizer: &Symbolizer) -> anyhow::Result<()> { + if let Some(ip) = &self.ip { + let ip = ip.trim_start_matches("0x"); + let ip = u64::from_str_radix(ip, 16)?; + let input = Input::AbsAddr(ip); + match symbolizer.symbolize_single(src, input)? { + Symbolized::Sym(s) => { + if let Some(c) = s.code_info { + self.column = c.column.map(u32::from); + self.file = Some(c.to_path().display().to_string()); + self.line = c.line; + } + self.function = Some(s.name.into_owned()); + } + Symbolized::Unknown(reason) => { + anyhow::bail!("Couldn't symbolize {ip}: {reason}"); + } + } + } + Ok(()) + } } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] From 525dbce787300095c483d65a8299c9d88a9acc9c Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Thu, 5 Dec 2024 12:26:14 -0500 Subject: [PATCH 20/36] make the builder consistently return a result --- crashtracker-ffi/src/collector/datatypes.rs | 11 +-- crashtracker-ffi/src/crash_info/builder.rs | 36 ++++---- crashtracker-ffi/src/crash_info/metadata.rs | 12 +-- crashtracker/src/rfc5_crash_info/builder.rs | 94 +++++++++++---------- ddcommon-ffi/src/slice.rs | 4 + 5 files changed, 83 insertions(+), 74 deletions(-) diff --git a/crashtracker-ffi/src/collector/datatypes.rs b/crashtracker-ffi/src/collector/datatypes.rs index 5535a3ee9..94de57955 100644 --- a/crashtracker-ffi/src/collector/datatypes.rs +++ b/crashtracker-ffi/src/collector/datatypes.rs @@ -29,21 +29,18 @@ impl<'a> TryFrom> for datadog_crashtracker::CrashtrackerRecei let args = { let mut vec = Vec::with_capacity(value.args.len()); for x in value.args.iter() { - vec.push(x.try_to_utf8()?.to_string()); + vec.push(x.try_to_string()?); } vec }; let env = { let mut vec = Vec::with_capacity(value.env.len()); for x in value.env.iter() { - vec.push(( - x.key.try_to_utf8()?.to_string(), - x.val.try_to_utf8()?.to_string(), - )); + vec.push((x.key.try_to_string()?, x.val.try_to_string()?)); } vec }; - let path_to_receiver_binary = value.path_to_receiver_binary.try_to_utf8()?.to_string(); + let path_to_receiver_binary = value.path_to_receiver_binary.try_to_string()?; let stderr_filename = value.optional_stderr_filename.try_to_string_option()?; let stdout_filename = value.optional_stdout_filename.try_to_string_option()?; Self::new( @@ -79,7 +76,7 @@ impl<'a> TryFrom> for datadog_crashtracker::CrashtrackerConfiguration let additional_files = { let mut vec = Vec::with_capacity(value.additional_files.len()); for x in value.additional_files.iter() { - vec.push(x.try_to_utf8()?.to_string()); + vec.push(x.try_to_string()?); } vec }; diff --git a/crashtracker-ffi/src/crash_info/builder.rs b/crashtracker-ffi/src/crash_info/builder.rs index f7e62efa0..79696f57f 100644 --- a/crashtracker-ffi/src/crash_info/builder.rs +++ b/crashtracker-ffi/src/crash_info/builder.rs @@ -61,7 +61,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_counter( wrap_with_void_ffi_result!({ builder .to_inner_mut()? - .with_counter(name.try_to_utf8()?.to_string(), value)?; + .with_counter(name.try_to_string()?, value)?; }) } @@ -100,13 +100,13 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_file( let contents = { let mut accum = Vec::with_capacity(contents.len()); for line in contents.iter() { - let line = line.try_to_utf8()?.to_string(); + let line = line.try_to_string()?; accum.push(line); } accum }; - builder.to_inner_mut()?.with_file(filename, contents); + builder.to_inner_mut()?.with_file(filename, contents)?; }) } @@ -124,7 +124,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_fingerprint( wrap_with_void_ffi_result!({ builder .to_inner_mut()? - .with_fingerprint(fingerprint.try_to_utf8()?.to_string())?; + .with_fingerprint(fingerprint.try_to_string()?)?; }) } @@ -140,7 +140,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_incomplete( incomplete: bool, ) -> VoidResult { wrap_with_void_ffi_result!({ - builder.to_inner_mut()?.with_incomplete(incomplete); + builder.to_inner_mut()?.with_incomplete(incomplete)?; }) } @@ -158,7 +158,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_log_message( wrap_with_void_ffi_result!({ builder .to_inner_mut()? - .with_log_message(message.try_to_utf8()?.to_string())?; + .with_log_message(message.try_to_string()?)?; }) } @@ -174,7 +174,9 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_metadata( metadata: Metadata, ) -> VoidResult { wrap_with_void_ffi_result!({ - builder.to_inner_mut()?.with_metadata(metadata.try_into()?); + builder + .to_inner_mut()? + .with_metadata(metadata.try_into()?)?; }) } @@ -190,7 +192,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_os_info( os_info: OsInfo, ) -> VoidResult { wrap_with_void_ffi_result!({ - builder.to_inner_mut()?.with_os_info(os_info.try_into()?); + builder.to_inner_mut()?.with_os_info(os_info.try_into()?)?; }) } @@ -205,7 +207,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_os_info_this_machine( mut builder: *mut Handle, ) -> VoidResult { wrap_with_void_ffi_result!({ - builder.to_inner_mut()?.with_os_info_this_machine(); + builder.to_inner_mut()?.with_os_info_this_machine()?; }) } @@ -223,7 +225,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_proc_info( wrap_with_void_ffi_result!({ builder .to_inner_mut()? - .with_proc_info(proc_info.try_into()?); + .with_proc_info(proc_info.try_into()?)?; }) } @@ -239,7 +241,9 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_sig_info( sig_info: SigInfo, ) -> VoidResult { wrap_with_void_ffi_result!({ - builder.to_inner_mut()?.with_sig_info(sig_info.try_into()?); + builder + .to_inner_mut()? + .with_sig_info(sig_info.try_into()?)?; }) } @@ -272,7 +276,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_stack( mut stack: *mut Handle, ) -> VoidResult { wrap_with_void_ffi_result!({ - builder.to_inner_mut()?.with_stack(*stack.take()?); + builder.to_inner_mut()?.with_stack(*stack.take()?)?; }) } @@ -305,7 +309,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp( ts: Timespec, ) -> VoidResult { wrap_with_void_ffi_result!({ - builder.to_inner_mut()?.with_timestamp(ts.into()); + builder.to_inner_mut()?.with_timestamp(ts.into())?; }) } @@ -320,7 +324,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_timestamp_now( mut builder: *mut Handle, ) -> VoidResult { wrap_with_void_ffi_result!({ - builder.to_inner_mut()?.with_timestamp_now(); + builder.to_inner_mut()?.with_timestamp_now()?; }) } @@ -357,7 +361,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid( let uuid = uuid .try_to_string_option()? .context("UUID cannot be empty string")?; - builder.to_inner_mut()?.with_uuid(uuid); + builder.to_inner_mut()?.with_uuid(uuid)?; }) } @@ -372,6 +376,6 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_uuid_random( mut builder: *mut Handle, ) -> VoidResult { wrap_with_void_ffi_result!({ - builder.to_inner_mut()?.with_uuid_random(); + builder.to_inner_mut()?.with_uuid_random()?; }) } diff --git a/crashtracker-ffi/src/crash_info/metadata.rs b/crashtracker-ffi/src/crash_info/metadata.rs index 790213253..6a29cb251 100644 --- a/crashtracker-ffi/src/crash_info/metadata.rs +++ b/crashtracker-ffi/src/crash_info/metadata.rs @@ -13,9 +13,9 @@ pub struct Metadata<'a> { impl<'a> TryFrom> for datadog_crashtracker::rfc5_crash_info::Metadata { type Error = anyhow::Error; fn try_from(value: Metadata<'a>) -> anyhow::Result { - let library_name = value.library_name.try_to_utf8()?.to_string(); - let library_version = value.library_version.try_to_utf8()?.to_string(); - let family = value.family.try_to_utf8()?.to_string(); + let library_name = value.library_name.try_to_string()?; + let library_version = value.library_version.try_to_string()?; + let family = value.family.try_to_string()?; let tags = if let Some(tags) = value.tags { tags.into_iter().map(|t| t.to_string()).collect() } else { @@ -33,9 +33,9 @@ impl<'a> TryFrom> for datadog_crashtracker::rfc5_crash_info::Metada impl<'a> TryFrom> for datadog_crashtracker::CrashtrackerMetadata { type Error = anyhow::Error; fn try_from(value: Metadata<'a>) -> anyhow::Result { - let library_name = value.library_name.try_to_utf8()?.to_string(); - let library_version = value.library_version.try_to_utf8()?.to_string(); - let family = value.family.try_to_utf8()?.to_string(); + let library_name = value.library_name.try_to_string()?; + let library_version = value.library_version.try_to_string()?; + let family = value.family.try_to_string()?; let tags = value .tags .map(|tags| tags.iter().cloned().collect()) diff --git a/crashtracker/src/rfc5_crash_info/builder.rs b/crashtracker/src/rfc5_crash_info/builder.rs index 62e471f53..b775037c7 100644 --- a/crashtracker/src/rfc5_crash_info/builder.rs +++ b/crashtracker/src/rfc5_crash_info/builder.rs @@ -46,24 +46,24 @@ impl ErrorDataBuilder { Self::default() } - pub fn with_kind(&mut self, kind: ErrorKind) -> &mut Self { + pub fn with_kind(&mut self, kind: ErrorKind) -> anyhow::Result<&mut Self> { self.kind = Some(kind); - self + Ok(self) } - pub fn with_message(&mut self, message: String) -> &mut Self { + pub fn with_message(&mut self, message: String) -> anyhow::Result<&mut Self> { self.message = Some(message); - self + Ok(self) } - pub fn with_stack(&mut self, stack: StackTrace) -> &mut Self { + pub fn with_stack(&mut self, stack: StackTrace) -> anyhow::Result<&mut Self> { self.stack = Some(stack); - self + Ok(self) } - pub fn with_threads(&mut self, threads: Vec) -> &mut Self { + pub fn with_threads(&mut self, threads: Vec) -> anyhow::Result<&mut Self> { self.threads = Some(threads); - self + Ok(self) } } @@ -136,30 +136,34 @@ impl CrashInfoBuilder { Ok(self) } - pub fn with_counters(&mut self, counters: HashMap) -> &mut Self { + pub fn with_counters(&mut self, counters: HashMap) -> anyhow::Result<&mut Self> { self.counters = Some(counters); - self + Ok(self) } pub fn with_kind(&mut self, kind: ErrorKind) -> anyhow::Result<&mut Self> { - self.error.with_kind(kind); + self.error.with_kind(kind)?; Ok(self) } /// Appends the given file to the current set of files in the builder. - pub fn with_file(&mut self, filename: String, contents: Vec) -> &mut Self { + pub fn with_file( + &mut self, + filename: String, + contents: Vec, + ) -> anyhow::Result<&mut Self> { if let Some(ref mut files) = &mut self.files { files.insert(filename, contents); } else { self.files = Some(HashMap::from([(filename, contents)])); } - self + Ok(self) } /// Sets the current set of files in the builder. - pub fn with_files(&mut self, files: HashMap>) -> &mut Self { + pub fn with_files(&mut self, files: HashMap>) -> anyhow::Result<&mut Self> { self.files = Some(files); - self + Ok(self) } pub fn with_fingerprint(&mut self, fingerprint: String) -> anyhow::Result<&mut Self> { @@ -168,9 +172,9 @@ impl CrashInfoBuilder { Ok(self) } - pub fn with_incomplete(&mut self, incomplete: bool) -> &mut Self { + pub fn with_incomplete(&mut self, incomplete: bool) -> anyhow::Result<&mut Self> { self.incomplete = Some(incomplete); - self + Ok(self) } /// Appends the given message to the current set of messages in the builder. @@ -183,38 +187,38 @@ impl CrashInfoBuilder { Ok(self) } - pub fn with_log_messages(&mut self, log_messages: Vec) -> &mut Self { + pub fn with_log_messages(&mut self, log_messages: Vec) -> anyhow::Result<&mut Self> { self.log_messages = Some(log_messages); - self + Ok(self) } - pub fn with_message(&mut self, message: String) -> &mut Self { - self.error.with_message(message); - self + pub fn with_message(&mut self, message: String) -> anyhow::Result<&mut Self> { + self.error.with_message(message)?; + Ok(self) } - pub fn with_metadata(&mut self, metadata: Metadata) -> &mut Self { + pub fn with_metadata(&mut self, metadata: Metadata) -> anyhow::Result<&mut Self> { self.metadata = Some(metadata); - self + Ok(self) } - pub fn with_os_info(&mut self, os_info: OsInfo) -> &mut Self { + pub fn with_os_info(&mut self, os_info: OsInfo) -> anyhow::Result<&mut Self> { self.os_info = Some(os_info); - self + Ok(self) } - pub fn with_os_info_this_machine(&mut self) -> &mut Self { + pub fn with_os_info_this_machine(&mut self) -> anyhow::Result<&mut Self> { self.with_os_info(::os_info::get().into()) } - pub fn with_proc_info(&mut self, proc_info: ProcInfo) -> &mut Self { + pub fn with_proc_info(&mut self, proc_info: ProcInfo) -> anyhow::Result<&mut Self> { self.proc_info = Some(proc_info); - self + Ok(self) } - pub fn with_sig_info(&mut self, sig_info: SigInfo) -> &mut Self { + pub fn with_sig_info(&mut self, sig_info: SigInfo) -> anyhow::Result<&mut Self> { self.sig_info = Some(sig_info); - self + Ok(self) } pub fn with_span_id(&mut self, span_id: Span) -> anyhow::Result<&mut Self> { @@ -226,14 +230,14 @@ impl CrashInfoBuilder { Ok(self) } - pub fn with_span_ids(&mut self, span_ids: Vec) -> &mut Self { + pub fn with_span_ids(&mut self, span_ids: Vec) -> anyhow::Result<&mut Self> { self.span_ids = Some(span_ids); - self + Ok(self) } - pub fn with_stack(&mut self, stack: StackTrace) -> &mut Self { - self.error.with_stack(stack); - self + pub fn with_stack(&mut self, stack: StackTrace) -> anyhow::Result<&mut Self> { + self.error.with_stack(stack)?; + Ok(self) } pub fn with_thread(&mut self, thread: ThreadData) -> anyhow::Result<&mut Self> { @@ -246,16 +250,16 @@ impl CrashInfoBuilder { } pub fn with_threads(&mut self, threads: Vec) -> anyhow::Result<&mut Self> { - self.error.with_threads(threads); + self.error.with_threads(threads)?; Ok(self) } - pub fn with_timestamp(&mut self, timestamp: DateTime) -> &mut Self { + pub fn with_timestamp(&mut self, timestamp: DateTime) -> anyhow::Result<&mut Self> { self.timestamp = Some(timestamp); - self + Ok(self) } - pub fn with_timestamp_now(&mut self) -> &mut Self { + pub fn with_timestamp_now(&mut self) -> anyhow::Result<&mut Self> { self.with_timestamp(Utc::now()) } @@ -268,17 +272,17 @@ impl CrashInfoBuilder { Ok(self) } - pub fn with_trace_ids(&mut self, trace_ids: Vec) -> &mut Self { + pub fn with_trace_ids(&mut self, trace_ids: Vec) -> anyhow::Result<&mut Self> { self.trace_ids = Some(trace_ids); - self + Ok(self) } - pub fn with_uuid(&mut self, uuid: String) -> &mut Self { + pub fn with_uuid(&mut self, uuid: String) -> anyhow::Result<&mut Self> { self.uuid = Some(uuid); - self + Ok(self) } - pub fn with_uuid_random(&mut self) -> &mut Self { + pub fn with_uuid_random(&mut self) -> anyhow::Result<&mut Self> { self.with_uuid(Uuid::new_v4().to_string()) } } diff --git a/ddcommon-ffi/src/slice.rs b/ddcommon-ffi/src/slice.rs index 862f84dcc..cc5c1b26a 100644 --- a/ddcommon-ffi/src/slice.rs +++ b/ddcommon-ffi/src/slice.rs @@ -72,6 +72,10 @@ pub trait AsBytes<'a> { std::str::from_utf8(self.as_bytes()) } + fn try_to_string(&self) -> Result { + Ok(self.try_to_utf8()?.to_string()) + } + #[inline] fn try_to_string_option(&self) -> Result, Utf8Error> { let s = std::str::from_utf8(self.as_bytes())?.to_string(); From 5c0a8ad9924d14a4afec41d64725772bfbfff43a Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Thu, 5 Dec 2024 14:46:48 -0500 Subject: [PATCH 21/36] make crashtracker.c compile --- examples/ffi/crashtracking.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/ffi/crashtracking.c b/examples/ffi/crashtracking.c index 8555ab133..a803f583c 100644 --- a/examples/ffi/crashtracking.c +++ b/examples/ffi/crashtracking.c @@ -12,8 +12,8 @@ void example_segfault_handler(int signal) { exit(-1); } -void handle_result(ddog_crasht_Result result) { - if (result.tag == DDOG_CRASHT_RESULT_ERR) { +void handle_result(ddog_VoidResult result) { + if (result.tag == DDOG_VOID_RESULT_ERR) { ddog_CharSlice message = ddog_Error_message(&result.err); fprintf(stderr, "%.*s\n", (int)message.len, message.ptr); ddog_Error_drop(&result.err); From 5ce5a12aa4ad8434721e83061aed2cf41a4e2cb1 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Thu, 5 Dec 2024 22:37:56 -0500 Subject: [PATCH 22/36] example works, need to add a stack trace --- crashtracker-ffi/src/crash_info/builder.rs | 24 +- crashtracker/src/rfc5_crash_info/builder.rs | 9 +- examples/ffi/crashinfo.cpp | 252 +++++++++++--------- 3 files changed, 170 insertions(+), 115 deletions(-) diff --git a/crashtracker-ffi/src/crash_info/builder.rs b/crashtracker-ffi/src/crash_info/builder.rs index 79696f57f..97b450a9e 100644 --- a/crashtracker-ffi/src/crash_info/builder.rs +++ b/crashtracker-ffi/src/crash_info/builder.rs @@ -91,6 +91,26 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_kind( pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_file( mut builder: *mut Handle, filename: CharSlice, +) -> VoidResult { + wrap_with_void_ffi_result!({ + builder.to_inner_mut()?.with_file( + filename + .try_to_string_option()? + .context("filename cannot be empty string")?, + )?; + }) +} + +/// # Safety +/// The `builder` can be null, but if non-null it must point to a Builder made by this module, +/// which has not previously been dropped. +/// The CharSlice must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_file_and_contents( + mut builder: *mut Handle, + filename: CharSlice, contents: Slice, ) -> VoidResult { wrap_with_void_ffi_result!({ @@ -106,7 +126,9 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_file( accum }; - builder.to_inner_mut()?.with_file(filename, contents)?; + builder + .to_inner_mut()? + .with_file_and_contents(filename, contents)?; }) } diff --git a/crashtracker/src/rfc5_crash_info/builder.rs b/crashtracker/src/rfc5_crash_info/builder.rs index b775037c7..74ef8c370 100644 --- a/crashtracker/src/rfc5_crash_info/builder.rs +++ b/crashtracker/src/rfc5_crash_info/builder.rs @@ -4,6 +4,7 @@ use chrono::{DateTime, Utc}; use error_data::ThreadData; use stacktrace::StackTrace; +use std::io::{BufRead, BufReader}; use unknown_value::UnknownValue; use uuid::Uuid; @@ -146,8 +147,14 @@ impl CrashInfoBuilder { Ok(self) } + pub fn with_file(&mut self, filename: String) -> anyhow::Result<&mut Self> { + let file = File::open(&filename).with_context(|| format!("filename: {filename}"))?; + let lines: std::io::Result> = BufReader::new(file).lines().collect(); + self.with_file_and_contents(filename, lines?) + } + /// Appends the given file to the current set of files in the builder. - pub fn with_file( + pub fn with_file_and_contents( &mut self, filename: String, contents: Vec, diff --git a/examples/ffi/crashinfo.cpp b/examples/ffi/crashinfo.cpp index a5bd63857..2c1ed0018 100644 --- a/examples/ffi/crashinfo.cpp +++ b/examples/ffi/crashinfo.cpp @@ -15,129 +15,148 @@ extern "C" { #include static ddog_CharSlice to_slice_c_char(const char *s) { return {.ptr = s, .len = strlen(s)}; } -static ddog_CharSlice to_slice_c_char(const char *s, std::size_t size) { return {.ptr = s, .len = size}; } +static ddog_CharSlice to_slice_c_char(const char *s, std::size_t size) { + return {.ptr = s, .len = size}; +} static ddog_CharSlice to_slice_string(std::string const &s) { return {.ptr = s.data(), .len = s.length()}; } -template -static ddog_ByteSlice to_byte_slice(T const& c) { - return {.ptr = reinterpret_cast(c.data()), .len = c.size()}; -} +struct BuilderDeleter { + void operator()(ddog_crasht_Handle_CrashInfoBuilder *object) { + ddog_crasht_CrashInfoBuilder_drop(object); + } +}; -struct Deleter { - void operator()(ddog_crasht_CrashInfo *object) { ddog_crasht_CrashInfo_drop(object); } +struct CrashinfoDeleter { + void operator()(ddog_crasht_Handle_CrashInfo *object) { + ddog_crasht_CrashInfo_drop(object); + } }; + void print_error(const char *s, const ddog_Error &err) { auto charslice = ddog_Error_message(&err); printf("%s (%.*s)\n", s, static_cast(charslice.len), charslice.ptr); } -void check_result(ddog_crasht_Result result, const char *msg) { - if (result.tag != DDOG_CRASHT_RESULT_OK) { +void check_result(ddog_VoidResult result, const char *msg) { + if (result.tag != DDOG_VOID_RESULT_OK) { print_error(msg, result.err); ddog_Error_drop(&result.err); exit(EXIT_FAILURE); } } -void add_stacktrace(std::unique_ptr &crashinfo) { - - // Collect things into vectors so they stay alive till the function exits - constexpr std::size_t nb_elements = 20; - std::vector> functions_and_filenames{nb_elements}; - for (uintptr_t i = 0; i < nb_elements; ++i) { - functions_and_filenames.push_back({"func_" + std::to_string(i), "/path/to/code/file_" + std::to_string(i)}); - } - - std::vector names{nb_elements}; - for (auto i = 0; i < nb_elements; i++) { - auto const& [function_name, filename] = functions_and_filenames[i]; - - auto function_name_slice = to_slice_string(function_name); - auto res = ddog_crasht_demangle(function_name_slice, DDOG_CRASHT_DEMANGLE_OPTIONS_COMPLETE); - if (res.tag == DDOG_CRASHT_STRING_WRAPPER_RESULT_OK) - { - auto string_result = res.ok.message; - function_name_slice = to_slice_c_char((const char*)string_result.ptr, string_result.len); - } - - names.push_back({.colno = ddog_Option_U32_some(i), - .filename = to_slice_string(filename), - .lineno = ddog_Option_U32_some(2 * i + 3), - .name = function_name_slice}); - } - - std::vector trace; - for (uintptr_t i = 0; i < 20; ++i) { - ddog_crasht_StackFrame frame = {.ip = i, - .module_base_address = 0, - .names = {.ptr = &names[i], .len = 1}, - .sp = 0, - .symbol_address = 0}; - trace.push_back(frame); +void check_result(ddog_Vec_Tag_PushResult result, const char *msg) { + if (result.tag != DDOG_VEC_TAG_PUSH_RESULT_OK) { + print_error(msg, result.err); + ddog_Error_drop(&result.err); + exit(EXIT_FAILURE); } - - std::vector build_id = {42}; - std::string filePath = "/usr/share/somewhere"; - // test with normalized - auto elfFrameWithNormalization = ddog_crasht_StackFrame{ - .ip = 42, - .module_base_address = 0, - .names = {.ptr = &names[0], .len = 1}, // just for the test - .normalized_ip = { - .file_offset = 1, - .build_id = to_byte_slice(build_id), - .path = to_slice_c_char(filePath.c_str(), filePath.size()), - .typ = DDOG_CRASHT_NORMALIZED_ADDRESS_TYPES_ELF, - }, - .sp = 0, - .symbol_address = 0, - }; - - trace.push_back(elfFrameWithNormalization); - - // Windows-kind of frame - auto dllFrameWithNormalization = ddog_crasht_StackFrame{ - .ip = 42, - .module_base_address = 0, - .names = {.ptr = &names[0], .len = 1}, // just for the test - .normalized_ip = { - .file_offset = 1, - .build_id = to_byte_slice(build_id), - .age = 21, - .path = to_slice_c_char(filePath.c_str(), filePath.size()), - .typ = DDOG_CRASHT_NORMALIZED_ADDRESS_TYPES_PDB, - }, - .sp = 0, - .symbol_address = 0, - }; - - trace.push_back(dllFrameWithNormalization); - - ddog_crasht_Slice_StackFrame trace_slice = {.ptr = trace.data(), .len = trace.size()}; - - check_result( - ddog_crasht_CrashInfo_set_stacktrace(crashinfo.get(), to_slice_c_char(""), trace_slice), - "Failed to set stacktrace"); } +// void add_stacktrace(std::unique_ptr &crashinfo) { + +// // Collect things into vectors so they stay alive till the function exits +// constexpr std::size_t nb_elements = 20; +// std::vector> functions_and_filenames{nb_elements}; +// for (uintptr_t i = 0; i < nb_elements; ++i) { +// functions_and_filenames.push_back({"func_" + std::to_string(i), "/path/to/code/file_" + +// std::to_string(i)}); +// } + +// std::vector names{nb_elements}; +// for (auto i = 0; i < nb_elements; i++) { +// auto const& [function_name, filename] = functions_and_filenames[i]; + +// auto function_name_slice = to_slice_string(function_name); +// auto res = ddog_crasht_demangle(function_name_slice, DDOG_CRASHT_DEMANGLE_OPTIONS_COMPLETE); +// if (res.tag == DDOG_CRASHT_STRING_WRAPPER_RESULT_OK) +// { +// auto string_result = res.ok.message; +// function_name_slice = to_slice_c_char((const char*)string_result.ptr, string_result.len); +// } + +// names.push_back({.colno = ddog_Option_U32_some(i), +// .filename = to_slice_string(filename), +// .lineno = ddog_Option_U32_some(2 * i + 3), +// .name = function_name_slice}); +// } + +// std::vector trace; +// for (uintptr_t i = 0; i < 20; ++i) { +// ddog_crasht_StackFrame frame = {.ip = i, +// .module_base_address = 0, +// .names = {.ptr = &names[i], .len = 1}, +// .sp = 0, +// .symbol_address = 0}; +// trace.push_back(frame); +// } + +// std::vector build_id = {42}; +// std::string filePath = "/usr/share/somewhere"; +// // test with normalized +// auto elfFrameWithNormalization = ddog_crasht_StackFrame{ +// .ip = 42, +// .module_base_address = 0, +// .names = {.ptr = &names[0], .len = 1}, // just for the test +// .normalized_ip = { +// .file_offset = 1, +// .build_id = to_byte_slice(build_id), +// .path = to_slice_c_char(filePath.c_str(), filePath.size()), +// .typ = DDOG_CRASHT_NORMALIZED_ADDRESS_TYPES_ELF, +// }, +// .sp = 0, +// .symbol_address = 0, +// }; + +// trace.push_back(elfFrameWithNormalization); + +// // Windows-kind of frame +// auto dllFrameWithNormalization = ddog_crasht_StackFrame{ +// .ip = 42, +// .module_base_address = 0, +// .names = {.ptr = &names[0], .len = 1}, // just for the test +// .normalized_ip = { +// .file_offset = 1, +// .build_id = to_byte_slice(build_id), +// .age = 21, +// .path = to_slice_c_char(filePath.c_str(), filePath.size()), +// .typ = DDOG_CRASHT_NORMALIZED_ADDRESS_TYPES_PDB, +// }, +// .sp = 0, +// .symbol_address = 0, +// }; + +// trace.push_back(dllFrameWithNormalization); + +// ddog_crasht_Slice_StackFrame trace_slice = {.ptr = trace.data(), .len = trace.size()}; + +// check_result( +// ddog_crasht_CrashInfoBuilder_with_stacktrace(crashinfo.get(), to_slice_c_char(""), +// trace_slice), "Failed to set stacktrace"); +// } + int main(void) { - auto crashinfo_new_result = ddog_crasht_CrashInfo_new(); - if (crashinfo_new_result.tag != DDOG_CRASHT_CRASH_INFO_NEW_RESULT_OK) { - print_error("Failed to make new crashinfo: ", crashinfo_new_result.err); - ddog_Error_drop(&crashinfo_new_result.err); + auto builder_new_result = ddog_crasht_CrashInfoBuilder_new(); + if (builder_new_result.tag != + DDOG_CRASHT_RESULT_HANDLE_CRASH_INFO_BUILDER_OK_HANDLE_CRASH_INFO_BUILDER) { + print_error("Failed to make new crashinfo builder: ", builder_new_result.err); + ddog_Error_drop(&builder_new_result.err); exit(EXIT_FAILURE); } - std::unique_ptr crashinfo{&crashinfo_new_result.ok}; + std::unique_ptr builder{ + &builder_new_result.ok}; - check_result( - ddog_crasht_CrashInfo_add_counter(crashinfo.get(), to_slice_c_char("my_amazing_counter"), 3), - "Failed to add counter"); + check_result(ddog_crasht_CrashInfoBuilder_with_counter(builder.get(), + to_slice_c_char("my_amazing_counter"), 3), + "Failed to add counter"); - // TODO add some tags here auto tags = ddog_Vec_Tag_new(); + check_result( + ddog_Vec_Tag_push(&tags, to_slice_c_char("best-hockey-team"), to_slice_c_char("Habs")), + "failed to add tag"); const ddog_crasht_Metadata metadata = { .library_name = to_slice_c_char("libdatadog"), .library_version = to_slice_c_char("42"), @@ -145,35 +164,42 @@ int main(void) { .tags = &tags, }; - // TODO: We should set more tags that are expected by telemetry - check_result(ddog_crasht_CrashInfo_set_metadata(crashinfo.get(), metadata), + check_result(ddog_crasht_CrashInfoBuilder_with_metadata(builder.get(), metadata), "Failed to add metadata"); - check_result(ddog_crasht_CrashInfo_add_tag(crashinfo.get(), to_slice_c_char("best hockey team"), - to_slice_c_char("Habs")), - "Failed to add tag"); + ddog_Vec_Tag_drop(tags); // This API allows one to capture useful files (e.g. /proc/pid/maps) - // For testing purposes, use `/etc/hosts` which should exist on any reasonable - // UNIX system - check_result(ddog_crasht_CrashInfo_add_file(crashinfo.get(), to_slice_c_char("/etc/hosts")), + // For testing purposes, use `/etc/hosts` which should exist on any reasonable UNIX system + check_result(ddog_crasht_CrashInfoBuilder_with_file(builder.get(), to_slice_c_char("/etc/hosts")), "Failed to add file"); - add_stacktrace(crashinfo); + + check_result(ddog_crasht_CrashInfoBuilder_with_kind(builder.get(), DDOG_CRASHT_ERROR_KIND_PANIC), + "Failed to set error kind"); + + // add_stacktrace(crashinfo); - ddog_Timespec timestamp = {.seconds = 1568899800, .nanoseconds = 0}; // Datadog IPO at 2019-09-19T13:30:00Z = 1568899800 unix - check_result(ddog_crasht_CrashInfo_set_timestamp(crashinfo.get(), timestamp), + ddog_Timespec timestamp = {.seconds = 1568899800, .nanoseconds = 0}; + check_result(ddog_crasht_CrashInfoBuilder_with_timestamp(builder.get(), timestamp), "Failed to set timestamp"); - ddog_crasht_ProcInfo procinfo = { - .pid = 42 - }; - - check_result(ddog_crasht_CrashInfo_set_procinfo(crashinfo.get(), procinfo), + ddog_crasht_ProcInfo procinfo = {.pid = 42}; + check_result(ddog_crasht_CrashInfoBuilder_with_proc_info(builder.get(), procinfo), "Failed to set procinfo"); - auto endpoint = ddog_endpoint_from_filename(to_slice_c_char("/tmp/test")); + auto crashinfo_result = ddog_crasht_CrashInfoBuilder_build(builder.release()); + ddog_crasht_Result_HandleCrashInfo d; + if (crashinfo_result.tag != DDOG_CRASHT_RESULT_HANDLE_CRASH_INFO_OK_HANDLE_CRASH_INFO) { + print_error("Failed to make new crashinfo builder: ", crashinfo_result.err); + ddog_Error_drop(&crashinfo_result.err); + exit(EXIT_FAILURE); + } + std::unique_ptr crashinfo{ + &crashinfo_result.ok}; + + auto endpoint = ddog_endpoint_from_filename(to_slice_c_char("/tmp/test")); check_result(ddog_crasht_CrashInfo_upload_to_endpoint(crashinfo.get(), endpoint), "Failed to export to file"); ddog_endpoint_drop(endpoint); From f5517b55cafb3ebed441ff97046e2187b97720f9 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Thu, 5 Dec 2024 23:02:24 -0500 Subject: [PATCH 23/36] extract result macro --- examples/ffi/crashinfo.cpp | 62 ++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/examples/ffi/crashinfo.cpp b/examples/ffi/crashinfo.cpp index 2c1ed0018..dbdab1aef 100644 --- a/examples/ffi/crashinfo.cpp +++ b/examples/ffi/crashinfo.cpp @@ -29,33 +29,48 @@ struct BuilderDeleter { }; struct CrashinfoDeleter { - void operator()(ddog_crasht_Handle_CrashInfo *object) { - ddog_crasht_CrashInfo_drop(object); - } + void operator()(ddog_crasht_Handle_CrashInfo *object) { ddog_crasht_CrashInfo_drop(object); } }; - void print_error(const char *s, const ddog_Error &err) { auto charslice = ddog_Error_message(&err); printf("%s (%.*s)\n", s, static_cast(charslice.len), charslice.ptr); } -void check_result(ddog_VoidResult result, const char *msg) { - if (result.tag != DDOG_VOID_RESULT_OK) { - print_error(msg, result.err); - ddog_Error_drop(&result.err); - exit(EXIT_FAILURE); +#define CHECK_RESULT(typ, ok_tag) \ + void check_result(typ result, const char *msg) { \ + if (result.tag != ok_tag) { \ + print_error(msg, result.err); \ + ddog_Error_drop(&result.err); \ + exit(EXIT_FAILURE); \ + } \ } -} -void check_result(ddog_Vec_Tag_PushResult result, const char *msg) { - if (result.tag != DDOG_VEC_TAG_PUSH_RESULT_OK) { - print_error(msg, result.err); - ddog_Error_drop(&result.err); - exit(EXIT_FAILURE); +CHECK_RESULT(ddog_VoidResult, DDOG_VOID_RESULT_OK) +CHECK_RESULT(ddog_Vec_Tag_PushResult, DDOG_VEC_TAG_PUSH_RESULT_OK) + +#define EXTRACT_RESULT(wrapper_typ, ok_tag, inner_typ, deleter) \ + std::unique_ptr extract_result(wrapper_typ result, const char *msg) { \ + if (result.tag != ok_tag) { \ + print_error(msg, result.err); \ + ddog_Error_drop(&result.err); \ + exit(EXIT_FAILURE); \ + } \ + std::unique_ptr rval{&result.ok}; \ + return rval; \ } -} +EXTRACT_RESULT(ddog_crasht_Result_HandleCrashInfoBuilder, DDOG_CRASHT_RESULT_HANDLE_CRASH_INFO_BUILDER_OK_HANDLE_CRASH_INFO_BUILDER, ddog_crasht_Handle_CrashInfoBuilder, BuilderDeleter) + +void add_stacktrace( + std::unique_ptr &crashinfo) { + constexpr std::size_t nb_elements = 20; + auto trace_new = ddog_crasht_StackTrace_new(); + + for (uintptr_t i = 0; i < nb_elements; ++i) { + auto new_frame = ddog_crasht_StackFrame_new(); + } +} // void add_stacktrace(std::unique_ptr &crashinfo) { // // Collect things into vectors so they stay alive till the function exits @@ -139,16 +154,7 @@ void check_result(ddog_Vec_Tag_PushResult result, const char *msg) { // } int main(void) { - auto builder_new_result = ddog_crasht_CrashInfoBuilder_new(); - if (builder_new_result.tag != - DDOG_CRASHT_RESULT_HANDLE_CRASH_INFO_BUILDER_OK_HANDLE_CRASH_INFO_BUILDER) { - print_error("Failed to make new crashinfo builder: ", builder_new_result.err); - ddog_Error_drop(&builder_new_result.err); - exit(EXIT_FAILURE); - } - std::unique_ptr builder{ - &builder_new_result.ok}; - + auto builder = extract_result(ddog_crasht_CrashInfoBuilder_new(), "failed to make builder"); check_result(ddog_crasht_CrashInfoBuilder_with_counter(builder.get(), to_slice_c_char("my_amazing_counter"), 3), "Failed to add counter"); @@ -173,7 +179,6 @@ int main(void) { check_result(ddog_crasht_CrashInfoBuilder_with_file(builder.get(), to_slice_c_char("/etc/hosts")), "Failed to add file"); - check_result(ddog_crasht_CrashInfoBuilder_with_kind(builder.get(), DDOG_CRASHT_ERROR_KIND_PANIC), "Failed to set error kind"); @@ -196,8 +201,7 @@ int main(void) { exit(EXIT_FAILURE); } - std::unique_ptr crashinfo{ - &crashinfo_result.ok}; + std::unique_ptr crashinfo{&crashinfo_result.ok}; auto endpoint = ddog_endpoint_from_filename(to_slice_c_char("/tmp/test")); check_result(ddog_crasht_CrashInfo_upload_to_endpoint(crashinfo.get(), endpoint), From 31e9f84c72e5d5faf00f7d1d2b6b308bce173b40 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Thu, 5 Dec 2024 23:20:02 -0500 Subject: [PATCH 24/36] nicer macros for extract result --- examples/ffi/crashinfo.cpp | 44 +++++++++++++------------------------- 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/examples/ffi/crashinfo.cpp b/examples/ffi/crashinfo.cpp index dbdab1aef..7521e9a10 100644 --- a/examples/ffi/crashinfo.cpp +++ b/examples/ffi/crashinfo.cpp @@ -22,16 +22,6 @@ static ddog_CharSlice to_slice_string(std::string const &s) { return {.ptr = s.data(), .len = s.length()}; } -struct BuilderDeleter { - void operator()(ddog_crasht_Handle_CrashInfoBuilder *object) { - ddog_crasht_CrashInfoBuilder_drop(object); - } -}; - -struct CrashinfoDeleter { - void operator()(ddog_crasht_Handle_CrashInfo *object) { ddog_crasht_CrashInfo_drop(object); } -}; - void print_error(const char *s, const ddog_Error &err) { auto charslice = ddog_Error_message(&err); printf("%s (%.*s)\n", s, static_cast(charslice.len), charslice.ptr); @@ -49,21 +39,26 @@ void print_error(const char *s, const ddog_Error &err) { CHECK_RESULT(ddog_VoidResult, DDOG_VOID_RESULT_OK) CHECK_RESULT(ddog_Vec_Tag_PushResult, DDOG_VEC_TAG_PUSH_RESULT_OK) -#define EXTRACT_RESULT(wrapper_typ, ok_tag, inner_typ, deleter) \ - std::unique_ptr extract_result(wrapper_typ result, const char *msg) { \ - if (result.tag != ok_tag) { \ - print_error(msg, result.err); \ - ddog_Error_drop(&result.err); \ +#define EXTRACT_RESULT(typ, ok_tag) \ + struct typ##Deleter { \ + void operator()(ddog_crasht_Handle_##typ *object) { ddog_crasht_##typ##_drop(object); } \ + }; \ + std::unique_ptr \ + extract_result(ddog_crasht_Result_Handle##typ result, const char *msg) { \ + if (result.tag != ok_tag) { \ + print_error(msg, result.err); \ + ddog_Error_drop(&result.err); \ exit(EXIT_FAILURE); \ } \ - std::unique_ptr rval{&result.ok}; \ + std::unique_ptr rval{&result.ok}; \ return rval; \ } -EXTRACT_RESULT(ddog_crasht_Result_HandleCrashInfoBuilder, DDOG_CRASHT_RESULT_HANDLE_CRASH_INFO_BUILDER_OK_HANDLE_CRASH_INFO_BUILDER, ddog_crasht_Handle_CrashInfoBuilder, BuilderDeleter) +EXTRACT_RESULT(CrashInfoBuilder, + DDOG_CRASHT_RESULT_HANDLE_CRASH_INFO_BUILDER_OK_HANDLE_CRASH_INFO_BUILDER) +EXTRACT_RESULT(CrashInfo, DDOG_CRASHT_RESULT_HANDLE_CRASH_INFO_OK_HANDLE_CRASH_INFO) -void add_stacktrace( - std::unique_ptr &crashinfo) { +void add_stacktrace(ddog_crasht_Handle_CrashInfoBuilder *crashinfo) { constexpr std::size_t nb_elements = 20; auto trace_new = ddog_crasht_StackTrace_new(); @@ -193,16 +188,7 @@ int main(void) { check_result(ddog_crasht_CrashInfoBuilder_with_proc_info(builder.get(), procinfo), "Failed to set procinfo"); - auto crashinfo_result = ddog_crasht_CrashInfoBuilder_build(builder.release()); - ddog_crasht_Result_HandleCrashInfo d; - if (crashinfo_result.tag != DDOG_CRASHT_RESULT_HANDLE_CRASH_INFO_OK_HANDLE_CRASH_INFO) { - print_error("Failed to make new crashinfo builder: ", crashinfo_result.err); - ddog_Error_drop(&crashinfo_result.err); - exit(EXIT_FAILURE); - } - - std::unique_ptr crashinfo{&crashinfo_result.ok}; - + auto crashinfo = extract_result(ddog_crasht_CrashInfoBuilder_build(builder.release()), "failed to build CrashInfo"); auto endpoint = ddog_endpoint_from_filename(to_slice_c_char("/tmp/test")); check_result(ddog_crasht_CrashInfo_upload_to_endpoint(crashinfo.get(), endpoint), "Failed to export to file"); From 8c5ae526d7e940026fbe4dbb2c119f8f3f677607 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Thu, 5 Dec 2024 23:34:22 -0500 Subject: [PATCH 25/36] stacktrace in example --- crashtracker-ffi/src/crash_info/stackframe.rs | 3 +++ examples/ffi/crashinfo.cpp | 27 ++++++++++++++----- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/crashtracker-ffi/src/crash_info/stackframe.rs b/crashtracker-ffi/src/crash_info/stackframe.rs index 7ae448dcf..a7dc6b6cb 100644 --- a/crashtracker-ffi/src/crash_info/stackframe.rs +++ b/crashtracker-ffi/src/crash_info/stackframe.rs @@ -112,6 +112,9 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( }) } +// TODO the rest of the types here + + /// # Safety /// The `frame` can be null, but if non-null it must point to a Frame made by this module, /// which has not previously been dropped. diff --git a/examples/ffi/crashinfo.cpp b/examples/ffi/crashinfo.cpp index 7521e9a10..dd1b99ea4 100644 --- a/examples/ffi/crashinfo.cpp +++ b/examples/ffi/crashinfo.cpp @@ -43,8 +43,8 @@ CHECK_RESULT(ddog_Vec_Tag_PushResult, DDOG_VEC_TAG_PUSH_RESULT_OK) struct typ##Deleter { \ void operator()(ddog_crasht_Handle_##typ *object) { ddog_crasht_##typ##_drop(object); } \ }; \ - std::unique_ptr \ - extract_result(ddog_crasht_Result_Handle##typ result, const char *msg) { \ + std::unique_ptr extract_result( \ + ddog_crasht_Result_Handle##typ result, const char *msg) { \ if (result.tag != ok_tag) { \ print_error(msg, result.err); \ ddog_Error_drop(&result.err); \ @@ -57,14 +57,26 @@ CHECK_RESULT(ddog_Vec_Tag_PushResult, DDOG_VEC_TAG_PUSH_RESULT_OK) EXTRACT_RESULT(CrashInfoBuilder, DDOG_CRASHT_RESULT_HANDLE_CRASH_INFO_BUILDER_OK_HANDLE_CRASH_INFO_BUILDER) EXTRACT_RESULT(CrashInfo, DDOG_CRASHT_RESULT_HANDLE_CRASH_INFO_OK_HANDLE_CRASH_INFO) +EXTRACT_RESULT(StackTrace, DDOG_CRASHT_RESULT_HANDLE_STACK_TRACE_OK_HANDLE_STACK_TRACE) +EXTRACT_RESULT(StackFrame, DDOG_CRASHT_RESULT_HANDLE_STACK_FRAME_OK_HANDLE_STACK_FRAME) -void add_stacktrace(ddog_crasht_Handle_CrashInfoBuilder *crashinfo) { +void add_stacktrace(ddog_crasht_Handle_CrashInfoBuilder *builder) { constexpr std::size_t nb_elements = 20; - auto trace_new = ddog_crasht_StackTrace_new(); + auto stacktrace = extract_result(ddog_crasht_StackTrace_new(), "failed to make new StackTrace"); for (uintptr_t i = 0; i < nb_elements; ++i) { - auto new_frame = ddog_crasht_StackFrame_new(); + auto new_frame = extract_result(ddog_crasht_StackFrame_new(), "failed to make StackFrame"); + std::string name = "func_" + std::to_string(i); + check_result(ddog_crasht_StackFrame_with_function(new_frame.get(), to_slice_string(name)), + "failed to add function"); + std::string filename = "/path/to/code/file_" + std::to_string(i); + check_result(ddog_crasht_StackFrame_with_file(new_frame.get(), to_slice_string(filename)), + "failed to add filename"); + check_result(ddog_crasht_StackTrace_push_frame(stacktrace.get(), new_frame.get()), + "failed to add filename"); } + check_result(ddog_crasht_CrashInfoBuilder_with_stack(builder, stacktrace.get()), + "failed to add stacktrace"); } // void add_stacktrace(std::unique_ptr &crashinfo) { @@ -177,7 +189,7 @@ int main(void) { check_result(ddog_crasht_CrashInfoBuilder_with_kind(builder.get(), DDOG_CRASHT_ERROR_KIND_PANIC), "Failed to set error kind"); - // add_stacktrace(crashinfo); + add_stacktrace(builder.get()); // Datadog IPO at 2019-09-19T13:30:00Z = 1568899800 unix ddog_Timespec timestamp = {.seconds = 1568899800, .nanoseconds = 0}; @@ -188,7 +200,8 @@ int main(void) { check_result(ddog_crasht_CrashInfoBuilder_with_proc_info(builder.get(), procinfo), "Failed to set procinfo"); - auto crashinfo = extract_result(ddog_crasht_CrashInfoBuilder_build(builder.release()), "failed to build CrashInfo"); + auto crashinfo = extract_result(ddog_crasht_CrashInfoBuilder_build(builder.release()), + "failed to build CrashInfo"); auto endpoint = ddog_endpoint_from_filename(to_slice_c_char("/tmp/test")); check_result(ddog_crasht_CrashInfo_upload_to_endpoint(crashinfo.get(), endpoint), "Failed to export to file"); From 58e1e42af1e9b5243e65ab5af3f5a19c651002cf Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 6 Dec 2024 13:46:44 -0500 Subject: [PATCH 26/36] finished C++ example --- crashtracker-ffi/src/crash_info/stackframe.rs | 63 +++++++- crashtracker-ffi/src/crash_info/stacktrace.rs | 1 + examples/ffi/crashinfo.cpp | 146 ++++++++---------- 3 files changed, 124 insertions(+), 86 deletions(-) diff --git a/crashtracker-ffi/src/crash_info/stackframe.rs b/crashtracker-ffi/src/crash_info/stackframe.rs index a7dc6b6cb..7e03b3fe4 100644 --- a/crashtracker-ffi/src/crash_info/stackframe.rs +++ b/crashtracker-ffi/src/crash_info/stackframe.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use ::function_name::named; -use datadog_crashtracker::rfc5_crash_info::StackFrame; +use datadog_crashtracker::rfc5_crash_info::{BuildIdType, FileType, StackFrame}; use ddcommon_ffi::{ slice::AsBytes, wrap_with_void_ffi_result, CharSlice, Handle, Result, ToInner, VoidResult, }; @@ -112,8 +112,37 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id( }) } -// TODO the rest of the types here +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The BuildIdType must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_build_id_type( + mut frame: *mut Handle, + build_id_type: BuildIdType, +) -> VoidResult { + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.build_id_type = Some(build_id_type); + }) +} +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +/// The FileType must be valid. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_file_type( + mut frame: *mut Handle, + file_type: FileType, +) -> VoidResult { + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.file_type = Some(file_type); + }) +} /// # Safety /// The `frame` can be null, but if non-null it must point to a Frame made by this module, @@ -147,6 +176,21 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address( }) } +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_column( + mut frame: *mut Handle, + column: u32, +) -> VoidResult { + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.column = Some(column); + }) +} + /// # Safety /// The `frame` can be null, but if non-null it must point to a Frame made by this module, /// which has not previously been dropped. @@ -178,3 +222,18 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_function( frame.to_inner_mut()?.function = function.try_to_string_option()?; }) } + +/// # Safety +/// The `frame` can be null, but if non-null it must point to a Frame made by this module, +/// which has not previously been dropped. +#[no_mangle] +#[must_use] +#[named] +pub unsafe extern "C" fn ddog_crasht_StackFrame_with_line( + mut frame: *mut Handle, + line: u32, +) -> VoidResult { + wrap_with_void_ffi_result!({ + frame.to_inner_mut()?.line = Some(line); + }) +} diff --git a/crashtracker-ffi/src/crash_info/stacktrace.rs b/crashtracker-ffi/src/crash_info/stacktrace.rs index 6505a565d..32dd4e0b5 100644 --- a/crashtracker-ffi/src/crash_info/stacktrace.rs +++ b/crashtracker-ffi/src/crash_info/stacktrace.rs @@ -35,6 +35,7 @@ pub unsafe extern "C" fn ddog_crasht_StackTrace_drop(trace: *mut Handle &crashinfo) { - -// // Collect things into vectors so they stay alive till the function exits -// constexpr std::size_t nb_elements = 20; -// std::vector> functions_and_filenames{nb_elements}; -// for (uintptr_t i = 0; i < nb_elements; ++i) { -// functions_and_filenames.push_back({"func_" + std::to_string(i), "/path/to/code/file_" + -// std::to_string(i)}); -// } - -// std::vector names{nb_elements}; -// for (auto i = 0; i < nb_elements; i++) { -// auto const& [function_name, filename] = functions_and_filenames[i]; - -// auto function_name_slice = to_slice_string(function_name); -// auto res = ddog_crasht_demangle(function_name_slice, DDOG_CRASHT_DEMANGLE_OPTIONS_COMPLETE); -// if (res.tag == DDOG_CRASHT_STRING_WRAPPER_RESULT_OK) -// { -// auto string_result = res.ok.message; -// function_name_slice = to_slice_c_char((const char*)string_result.ptr, string_result.len); -// } - -// names.push_back({.colno = ddog_Option_U32_some(i), -// .filename = to_slice_string(filename), -// .lineno = ddog_Option_U32_some(2 * i + 3), -// .name = function_name_slice}); -// } - -// std::vector trace; -// for (uintptr_t i = 0; i < 20; ++i) { -// ddog_crasht_StackFrame frame = {.ip = i, -// .module_base_address = 0, -// .names = {.ptr = &names[i], .len = 1}, -// .sp = 0, -// .symbol_address = 0}; -// trace.push_back(frame); -// } - -// std::vector build_id = {42}; -// std::string filePath = "/usr/share/somewhere"; -// // test with normalized -// auto elfFrameWithNormalization = ddog_crasht_StackFrame{ -// .ip = 42, -// .module_base_address = 0, -// .names = {.ptr = &names[0], .len = 1}, // just for the test -// .normalized_ip = { -// .file_offset = 1, -// .build_id = to_byte_slice(build_id), -// .path = to_slice_c_char(filePath.c_str(), filePath.size()), -// .typ = DDOG_CRASHT_NORMALIZED_ADDRESS_TYPES_ELF, -// }, -// .sp = 0, -// .symbol_address = 0, -// }; - -// trace.push_back(elfFrameWithNormalization); - -// // Windows-kind of frame -// auto dllFrameWithNormalization = ddog_crasht_StackFrame{ -// .ip = 42, -// .module_base_address = 0, -// .names = {.ptr = &names[0], .len = 1}, // just for the test -// .normalized_ip = { -// .file_offset = 1, -// .build_id = to_byte_slice(build_id), -// .age = 21, -// .path = to_slice_c_char(filePath.c_str(), filePath.size()), -// .typ = DDOG_CRASHT_NORMALIZED_ADDRESS_TYPES_PDB, -// }, -// .sp = 0, -// .symbol_address = 0, -// }; - -// trace.push_back(dllFrameWithNormalization); - -// ddog_crasht_Slice_StackFrame trace_slice = {.ptr = trace.data(), .len = trace.size()}; - -// check_result( -// ddog_crasht_CrashInfoBuilder_with_stacktrace(crashinfo.get(), to_slice_c_char(""), -// trace_slice), "Failed to set stacktrace"); -// } int main(void) { auto builder = extract_result(ddog_crasht_CrashInfoBuilder_new(), "failed to make builder"); From dd8a7db9205251e5656b193b05a7029ec365e036 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 6 Dec 2024 14:05:15 -0500 Subject: [PATCH 27/36] os_info --- crashtracker/src/rfc5_crash_info/mod.rs | 4 +++- examples/ffi/crashinfo.cpp | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index 13a425f98..38b2ec886 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -44,8 +44,10 @@ pub struct CrashInfo { pub log_messages: Vec, pub metadata: Metadata, pub os_info: OsInfo, + #[serde(default, skip_serializing_if = "Option::is_none")] pub proc_info: Option, //TODO, update the schema - pub sig_info: Option, //TODO, update the schema + #[serde(default, skip_serializing_if = "Option::is_none")] + pub sig_info: Option, //TODO, update the schema #[serde(default, skip_serializing_if = "Vec::is_empty")] pub span_ids: Vec, pub timestamp: String, diff --git a/examples/ffi/crashinfo.cpp b/examples/ffi/crashinfo.cpp index 93e0cf643..785aed797 100644 --- a/examples/ffi/crashinfo.cpp +++ b/examples/ffi/crashinfo.cpp @@ -61,10 +61,9 @@ EXTRACT_RESULT(StackTrace, DDOG_CRASHT_RESULT_HANDLE_STACK_TRACE_OK_HANDLE_STACK EXTRACT_RESULT(StackFrame, DDOG_CRASHT_RESULT_HANDLE_STACK_FRAME_OK_HANDLE_STACK_FRAME) void add_stacktrace(ddog_crasht_Handle_CrashInfoBuilder *builder) { - constexpr std::size_t nb_elements = 20; auto stacktrace = extract_result(ddog_crasht_StackTrace_new(), "failed to make new StackTrace"); - for (uintptr_t i = 0; i < nb_elements; ++i) { + for (uintptr_t i = 0; i < 10; ++i) { auto new_frame = extract_result(ddog_crasht_StackFrame_new(), "failed to make StackFrame"); std::string name = "func_" + std::to_string(i); check_result(ddog_crasht_StackFrame_with_function(new_frame.get(), to_slice_string(name)), @@ -178,6 +177,9 @@ int main(void) { check_result(ddog_crasht_CrashInfoBuilder_with_proc_info(builder.get(), procinfo), "Failed to set procinfo"); + check_result(ddog_crasht_CrashInfoBuilder_with_os_info_this_machine(builder.get()), + "Failed to set os_info"); + auto crashinfo = extract_result(ddog_crasht_CrashInfoBuilder_build(builder.release()), "failed to build CrashInfo"); auto endpoint = ddog_endpoint_from_filename(to_slice_c_char("/tmp/test")); From b36c3c0fd5131908e8456a608d24cd8a390c0ff8 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 6 Dec 2024 14:09:13 -0500 Subject: [PATCH 28/36] licence --- LICENSE-3rdparty.yml | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/LICENSE-3rdparty.yml b/LICENSE-3rdparty.yml index 590b23a25..f3d694237 100644 --- a/LICENSE-3rdparty.yml +++ b/LICENSE-3rdparty.yml @@ -11238,6 +11238,41 @@ third_party_libraries: DEALINGS IN THE SOFTWARE. - license: Apache-2.0 text: " Apache License\n Version 2.0, January 2004\n http://www.apache.org/licenses/\n\nTERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION\n\n1. Definitions.\n\n \"License\" shall mean the terms and conditions for use, reproduction,\n and distribution as defined by Sections 1 through 9 of this document.\n\n \"Licensor\" shall mean the copyright owner or entity authorized by\n the copyright owner that is granting the License.\n\n \"Legal Entity\" shall mean the union of the acting entity and all\n other entities that control, are controlled by, or are under common\n control with that entity. For the purposes of this definition,\n \"control\" means (i) the power, direct or indirect, to cause the\n direction or management of such entity, whether by contract or\n otherwise, or (ii) ownership of fifty percent (50%) or more of the\n outstanding shares, or (iii) beneficial ownership of such entity.\n\n \"You\" (or \"Your\") shall mean an individual or Legal Entity\n exercising permissions granted by this License.\n\n \"Source\" form shall mean the preferred form for making modifications,\n including but not limited to software source code, documentation\n source, and configuration files.\n\n \"Object\" form shall mean any form resulting from mechanical\n transformation or translation of a Source form, including but\n not limited to compiled object code, generated documentation,\n and conversions to other media types.\n\n \"Work\" shall mean the work of authorship, whether in Source or\n Object form, made available under the License, as indicated by a\n copyright notice that is included in or attached to the work\n (an example is provided in the Appendix below).\n\n \"Derivative Works\" shall mean any work, whether in Source or Object\n form, that is based on (or derived from) the Work and for which the\n editorial revisions, annotations, elaborations, or other modifications\n represent, as a whole, an original work of authorship. For the purposes\n of this License, Derivative Works shall not include works that remain\n separable from, or merely link (or bind by name) to the interfaces of,\n the Work and Derivative Works thereof.\n\n \"Contribution\" shall mean any work of authorship, including\n the original version of the Work and any modifications or additions\n to that Work or Derivative Works thereof, that is intentionally\n submitted to Licensor for inclusion in the Work by the copyright owner\n or by an individual or Legal Entity authorized to submit on behalf of\n the copyright owner. For the purposes of this definition, \"submitted\"\n means any form of electronic, verbal, or written communication sent\n to the Licensor or its representatives, including but not limited to\n communication on electronic mailing lists, source code control systems,\n and issue tracking systems that are managed by, or on behalf of, the\n Licensor for the purpose of discussing and improving the Work, but\n excluding communication that is conspicuously marked or otherwise\n designated in writing by the copyright owner as \"Not a Contribution.\"\n\n \"Contributor\" shall mean Licensor and any individual or Legal Entity\n on behalf of whom a Contribution has been received by Licensor and\n subsequently incorporated within the Work.\n\n2. Grant of Copyright License. Subject to the terms and conditions of\n this License, each Contributor hereby grants to You a perpetual,\n worldwide, non-exclusive, no-charge, royalty-free, irrevocable\n copyright license to reproduce, prepare Derivative Works of,\n publicly display, publicly perform, sublicense, and distribute the\n Work and such Derivative Works in Source or Object form.\n\n3. Grant of Patent License. Subject to the terms and conditions of\n this License, each Contributor hereby grants to You a perpetual,\n worldwide, non-exclusive, no-charge, royalty-free, irrevocable\n (except as stated in this section) patent license to make, have made,\n use, offer to sell, sell, import, and otherwise transfer the Work,\n where such license applies only to those patent claims licensable\n by such Contributor that are necessarily infringed by their\n Contribution(s) alone or by combination of their Contribution(s)\n with the Work to which such Contribution(s) was submitted. If You\n institute patent litigation against any entity (including a\n cross-claim or counterclaim in a lawsuit) alleging that the Work\n or a Contribution incorporated within the Work constitutes direct\n or contributory patent infringement, then any patent licenses\n granted to You under this License for that Work shall terminate\n as of the date such litigation is filed.\n\n4. Redistribution. You may reproduce and distribute copies of the\n Work or Derivative Works thereof in any medium, with or without\n modifications, and in Source or Object form, provided that You\n meet the following conditions:\n\n (a) You must give any other recipients of the Work or\n Derivative Works a copy of this License; and\n\n (b) You must cause any modified files to carry prominent notices\n stating that You changed the files; and\n\n (c) You must retain, in the Source form of any Derivative Works\n that You distribute, all copyright, patent, trademark, and\n attribution notices from the Source form of the Work,\n excluding those notices that do not pertain to any part of\n the Derivative Works; and\n\n (d) If the Work includes a \"NOTICE\" text file as part of its\n distribution, then any Derivative Works that You distribute must\n include a readable copy of the attribution notices contained\n within such NOTICE file, excluding those notices that do not\n pertain to any part of the Derivative Works, in at least one\n of the following places: within a NOTICE text file distributed\n as part of the Derivative Works; within the Source form or\n documentation, if provided along with the Derivative Works; or,\n within a display generated by the Derivative Works, if and\n wherever such third-party notices normally appear. The contents\n of the NOTICE file are for informational purposes only and\n do not modify the License. You may add Your own attribution\n notices within Derivative Works that You distribute, alongside\n or as an addendum to the NOTICE text from the Work, provided\n that such additional attribution notices cannot be construed\n as modifying the License.\n\n You may add Your own copyright statement to Your modifications and\n may provide additional or different license terms and conditions\n for use, reproduction, or distribution of Your modifications, or\n for any such Derivative Works as a whole, provided Your use,\n reproduction, and distribution of the Work otherwise complies with\n the conditions stated in this License.\n\n5. Submission of Contributions. Unless You explicitly state otherwise,\n any Contribution intentionally submitted for inclusion in the Work\n by You to the Licensor shall be under the terms and conditions of\n this License, without any additional terms or conditions.\n Notwithstanding the above, nothing herein shall supersede or modify\n the terms of any separate license agreement you may have executed\n with Licensor regarding such Contributions.\n\n6. Trademarks. This License does not grant permission to use the trade\n names, trademarks, service marks, or product names of the Licensor,\n except as required for reasonable and customary use in describing the\n origin of the Work and reproducing the content of the NOTICE file.\n\n7. Disclaimer of Warranty. Unless required by applicable law or\n agreed to in writing, Licensor provides the Work (and each\n Contributor provides its Contributions) on an \"AS IS\" BASIS,\n WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or\n implied, including, without limitation, any warranties or conditions\n of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A\n PARTICULAR PURPOSE. You are solely responsible for determining the\n appropriateness of using or redistributing the Work and assume any\n risks associated with Your exercise of permissions under this License.\n\n8. Limitation of Liability. In no event and under no legal theory,\n whether in tort (including negligence), contract, or otherwise,\n unless required by applicable law (such as deliberate and grossly\n negligent acts) or agreed to in writing, shall any Contributor be\n liable to You for damages, including any direct, indirect, special,\n incidental, or consequential damages of any character arising as a\n result of this License or out of the use or inability to use the\n Work (including but not limited to damages for loss of goodwill,\n work stoppage, computer failure or malfunction, or any and all\n other commercial damages or losses), even if such Contributor\n has been advised of the possibility of such damages.\n\n9. Accepting Warranty or Additional Liability. While redistributing\n the Work or Derivative Works thereof, You may choose to offer,\n and charge a fee for, acceptance of support, warranty, indemnity,\n or other liability obligations and/or rights consistent with this\n License. However, in accepting such obligations, You may act only\n on Your own behalf and on Your sole responsibility, not on behalf\n of any other Contributor, and only if You agree to indemnify,\n defend, and hold each Contributor harmless for any liability\n incurred by, or claims asserted against, such Contributor by reason\n of your accepting any such warranty or additional liability.\n\nEND OF TERMS AND CONDITIONS\n\nAPPENDIX: How to apply the Apache License to your work.\n\n To apply the Apache License to your work, attach the following\n boilerplate notice, with the fields enclosed by brackets \"[]\"\n replaced with your own identifying information. (Don't include\n the brackets!) The text should be enclosed in the appropriate\n comment syntax for the file format. We also recommend that a\n file or class name and description of purpose be included on the\n same \"printed page\" as the copyright notice for easier\n identification within third-party archives.\n\nCopyright [yyyy] [name of copyright owner]\n\nLicensed under the Apache License, Version 2.0 (the \"License\");\nyou may not use this file except in compliance with the License.\nYou may obtain a copy of the License at\n\n\thttp://www.apache.org/licenses/LICENSE-2.0\n\nUnless required by applicable law or agreed to in writing, software\ndistributed under the License is distributed on an \"AS IS\" BASIS,\nWITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\nSee the License for the specific language governing permissions and\nlimitations under the License.\n" +- package_name: function_name + package_version: 0.3.0 + repository: https://github.com/danielhenrymantilla/rust-function_name + license: MIT + licenses: + - license: MIT + text: | + MIT License + + Copyright (c) 2019 Daniel Henry-Mantilla + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. +- package_name: function_name-proc-macro + package_version: 0.3.0 + repository: https://github.com/danielhenrymantilla/rust-function_name + license: MIT + licenses: + - license: MIT + text: NOT FOUND - package_name: futures package_version: 0.3.31 repository: https://github.com/rust-lang/futures-rs @@ -15890,7 +15925,7 @@ third_party_libraries: - license: MIT text: NOT FOUND - package_name: libc - package_version: 0.2.161 + package_version: 0.2.167 repository: https://github.com/rust-lang/libc license: MIT OR Apache-2.0 licenses: From 6cffa8c36bee5819f4f7dff37f6fddef648f84f2 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 6 Dec 2024 14:46:22 -0500 Subject: [PATCH 29/36] headers --- crashtracker-ffi/src/crash_info/metadata.rs | 3 +++ crashtracker-ffi/src/crash_info/span.rs | 3 +++ ddcommon-ffi/src/utils.rs | 3 +++ 3 files changed, 9 insertions(+) diff --git a/crashtracker-ffi/src/crash_info/metadata.rs b/crashtracker-ffi/src/crash_info/metadata.rs index 6a29cb251..7f2af08de 100644 --- a/crashtracker-ffi/src/crash_info/metadata.rs +++ b/crashtracker-ffi/src/crash_info/metadata.rs @@ -1,3 +1,6 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + use ddcommon::tag::Tag; use ddcommon_ffi::{slice::AsBytes, CharSlice}; diff --git a/crashtracker-ffi/src/crash_info/span.rs b/crashtracker-ffi/src/crash_info/span.rs index 70e822697..4f912e49e 100644 --- a/crashtracker-ffi/src/crash_info/span.rs +++ b/crashtracker-ffi/src/crash_info/span.rs @@ -1,3 +1,6 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + use anyhow::Context; use ddcommon_ffi::{slice::AsBytes, CharSlice}; #[repr(C)] diff --git a/ddcommon-ffi/src/utils.rs b/ddcommon-ffi/src/utils.rs index e2366581b..94cd8f8eb 100644 --- a/ddcommon-ffi/src/utils.rs +++ b/ddcommon-ffi/src/utils.rs @@ -1,3 +1,6 @@ +// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + /// Wraps a C-FFI function in standard form /// Expects the function to return a result type that implements into and to be decorated with /// #[named]. From 778611626af40cd4e2ace8b5dfa9a6f45b2f60b8 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 6 Dec 2024 15:06:21 -0500 Subject: [PATCH 30/36] blazesym is only for unix --- crashtracker/src/rfc5_crash_info/error_data.rs | 1 + crashtracker/src/rfc5_crash_info/stacktrace.rs | 7 +++++-- ddcommon-ffi/src/slice.rs | 7 +------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/crashtracker/src/rfc5_crash_info/error_data.rs b/crashtracker/src/rfc5_crash_info/error_data.rs index 77ea64c57..d6926d020 100644 --- a/crashtracker/src/rfc5_crash_info/error_data.rs +++ b/crashtracker/src/rfc5_crash_info/error_data.rs @@ -17,6 +17,7 @@ pub struct ErrorData { pub threads: Vec, } +#[cfg(unix)] impl ErrorData { pub fn normalize_ips(&mut self, pid: u32) -> anyhow::Result<()> { let normalizer = blazesym::normalize::Normalizer::new(); diff --git a/crashtracker/src/rfc5_crash_info/stacktrace.rs b/crashtracker/src/rfc5_crash_info/stacktrace.rs index 6ea3cf5aa..bf92f0c9d 100644 --- a/crashtracker/src/rfc5_crash_info/stacktrace.rs +++ b/crashtracker/src/rfc5_crash_info/stacktrace.rs @@ -1,7 +1,9 @@ // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 +use crate::NormalizedAddress; use anyhow::Context; +#[cfg(unix)] use blazesym::{ helper::ElfResolver, normalize::Normalizer, @@ -11,8 +13,6 @@ use blazesym::{ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use crate::NormalizedAddress; - #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] pub struct StackTrace { pub format: String, @@ -187,7 +187,10 @@ impl StackFrame { pub fn new() -> Self { Self::default() } +} +#[cfg(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"); diff --git a/ddcommon-ffi/src/slice.rs b/ddcommon-ffi/src/slice.rs index cc5c1b26a..8e84e21d9 100644 --- a/ddcommon-ffi/src/slice.rs +++ b/ddcommon-ffi/src/slice.rs @@ -78,12 +78,7 @@ pub trait AsBytes<'a> { #[inline] fn try_to_string_option(&self) -> Result, Utf8Error> { - let s = std::str::from_utf8(self.as_bytes())?.to_string(); - if s.is_empty() { - Ok(None) - } else { - Ok(Some(s)) - } + Ok(Some(self.try_to_string()?).filter(|x| !x.is_empty())) } #[inline] From 48c29beed35ad8924d43cc0b93280916f31839d1 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 6 Dec 2024 15:16:15 -0500 Subject: [PATCH 31/36] more cfg unix --- crashtracker-ffi/src/crash_info/api.rs | 2 ++ crashtracker/src/rfc5_crash_info/mod.rs | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/crashtracker-ffi/src/crash_info/api.rs b/crashtracker-ffi/src/crash_info/api.rs index 00b93aea4..4ced394f8 100644 --- a/crashtracker-ffi/src/crash_info/api.rs +++ b/crashtracker-ffi/src/crash_info/api.rs @@ -24,6 +24,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfo_drop(builder: *mut Handle, pid: u32, @@ -39,6 +40,7 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfo_normalize_ips( #[no_mangle] #[must_use] #[named] +#[cfg(unix)] pub unsafe extern "C" fn ddog_crasht_CrashInfo_resolve_names( mut crash_info: *mut Handle, pid: u32, diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index 38b2ec886..5a906dffd 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -61,6 +61,11 @@ impl CrashInfo { "1.0".to_string() } + +} + +#[cfg(unix)] +impl CrashInfo { pub fn normalize_ips(&mut self, pid: u32) -> anyhow::Result<()> { self.error.normalize_ips(pid) } From 7e79d856d2c719e26555d84cba0fb768513aaa33 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 6 Dec 2024 15:19:40 -0500 Subject: [PATCH 32/36] format --- crashtracker/src/rfc5_crash_info/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crashtracker/src/rfc5_crash_info/mod.rs b/crashtracker/src/rfc5_crash_info/mod.rs index 5a906dffd..68494d48a 100644 --- a/crashtracker/src/rfc5_crash_info/mod.rs +++ b/crashtracker/src/rfc5_crash_info/mod.rs @@ -60,8 +60,6 @@ impl CrashInfo { pub fn current_schema_version() -> String { "1.0".to_string() } - - } #[cfg(unix)] From d83b284689d638d4eb4843e4b7064ee0572a1b82 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 6 Dec 2024 15:26:34 -0500 Subject: [PATCH 33/36] even more cfg unix --- crashtracker/src/rfc5_crash_info/stacktrace.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crashtracker/src/rfc5_crash_info/stacktrace.rs b/crashtracker/src/rfc5_crash_info/stacktrace.rs index bf92f0c9d..16994d987 100644 --- a/crashtracker/src/rfc5_crash_info/stacktrace.rs +++ b/crashtracker/src/rfc5_crash_info/stacktrace.rs @@ -26,7 +26,10 @@ impl StackTrace { frames: vec![], } } +} +#[cfg(unix)] +impl StackTrace { pub fn normalize_ips(&mut self, normalizer: &Normalizer, pid: Pid) -> anyhow::Result<()> { for frame in &mut self.frames { // TODO: Should this keep going on failure, and report at the end? From 954a577be646dee70f39b6e906fe38f30037a910 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 6 Dec 2024 15:41:06 -0500 Subject: [PATCH 34/36] unix --- crashtracker/src/rfc5_crash_info/stacktrace.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crashtracker/src/rfc5_crash_info/stacktrace.rs b/crashtracker/src/rfc5_crash_info/stacktrace.rs index 16994d987..ee9829365 100644 --- a/crashtracker/src/rfc5_crash_info/stacktrace.rs +++ b/crashtracker/src/rfc5_crash_info/stacktrace.rs @@ -274,6 +274,7 @@ fn byte_vec_as_hex(bv: Option>) -> Option { } } +#[cfg(unix)] fn byte_slice_as_hex(bv: &[u8]) -> String { use std::fmt::Write; From 6bbefaf869cf821f5f2ffeabb25261d3e2a68c1e Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 6 Dec 2024 15:47:56 -0500 Subject: [PATCH 35/36] hopefully the last unix move: --- crashtracker/src/rfc5_crash_info/stacktrace.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crashtracker/src/rfc5_crash_info/stacktrace.rs b/crashtracker/src/rfc5_crash_info/stacktrace.rs index ee9829365..34d45d3ae 100644 --- a/crashtracker/src/rfc5_crash_info/stacktrace.rs +++ b/crashtracker/src/rfc5_crash_info/stacktrace.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 use crate::NormalizedAddress; -use anyhow::Context; #[cfg(unix)] use blazesym::{ helper::ElfResolver, @@ -195,6 +194,7 @@ impl StackFrame { #[cfg(unix)] impl StackFrame { pub fn normalize_ip(&mut self, normalizer: &Normalizer, pid: Pid) -> anyhow::Result<()> { + use anyhow::Context; if let Some(ip) = &self.ip { let ip = ip.trim_start_matches("0x"); let ip = u64::from_str_radix(ip, 16)?; From 4c58d321e6810e5ad8dfa590aa126c5d3b59189e Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Fri, 6 Dec 2024 23:57:36 -0500 Subject: [PATCH 36/36] EXTRACT_RESULT that heap allocates --- examples/ffi/crashinfo.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/examples/ffi/crashinfo.cpp b/examples/ffi/crashinfo.cpp index 785aed797..337ab3be1 100644 --- a/examples/ffi/crashinfo.cpp +++ b/examples/ffi/crashinfo.cpp @@ -41,7 +41,10 @@ CHECK_RESULT(ddog_Vec_Tag_PushResult, DDOG_VEC_TAG_PUSH_RESULT_OK) #define EXTRACT_RESULT(typ, ok_tag) \ struct typ##Deleter { \ - void operator()(ddog_crasht_Handle_##typ *object) { ddog_crasht_##typ##_drop(object); } \ + void operator()(ddog_crasht_Handle_##typ *object) { \ + ddog_crasht_##typ##_drop(object); \ + delete object; \ + } \ }; \ std::unique_ptr extract_result( \ ddog_crasht_Result_Handle##typ result, const char *msg) { \ @@ -50,7 +53,8 @@ CHECK_RESULT(ddog_Vec_Tag_PushResult, DDOG_VEC_TAG_PUSH_RESULT_OK) ddog_Error_drop(&result.err); \ exit(EXIT_FAILURE); \ } \ - std::unique_ptr rval{&result.ok}; \ + std::unique_ptr rval{ \ + new ddog_crasht_Handle_##typ{result.ok}}; \ return rval; \ }