Add crashtracker support for sidecar (#1265)

Add crashtracker support for sidecar

This also switches from a stack pointer based approach to an instruction pointer based approach to remove stack frames.

It seems like in rust code - at least in my test rust code - the crashing frame was always omitted; e.g.:

datadog_crashtracker::collector::crash_handler::handle_posix_sigaction -> sp: 0xffffb763ed20
__kernel_rt_sigreturn -> sp: 0xffffb763eda0
datadog_sidecar::unix::test -> sp: 0xffffb763eed0 < dummy function to test a crash - here it crashes, but it's omitted, because the sp is right below __kernel_rt_sigreturn frame... for reasons?
ddog_daemon_entry_point -> sp: 0xffffdb23c560 < and that's now above the fault_sp.

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>

Author: bwoebi

Cargo.lock

@@ -200,17 +200,13 @@ fn test_crash_tracking_callstack() {
 
     // Note: in Release, we do not have the crate and module name prepended to the function name
     // Here we compile the crashing app in Debug.
-    let mut expected_functions = Vec::new();
-    // It seems that on arm/arm64, fn3 is inlined in fn2, so not present.
-    // Add fn3 only for x86_64 arch
-    #[cfg(target_arch = "x86_64")]
-    expected_functions.push("crashing_test_app::unix::fn3");
-    expected_functions.extend_from_slice(&[
+    let expected_functions = [
+        "crashing_test_app::unix::fn3",
         "crashing_test_app::unix::fn2",
         "crashing_test_app::unix::fn1",
         "crashing_test_app::unix::main",
         "crashing_test_app::main",
-    ]);
+    ];
 
     let crashing_callstack = &crash_payload["error"]["stack"]["frames"];
     assert!(

bin_tests/tests/crashtracker_bin_test.rs

@@ -45,7 +45,7 @@ pub enum EmitterError {
 unsafe fn emit_backtrace_by_frames(
     w: &mut impl Write,
     resolve_frames: StacktraceCollection,
-    fault_rsp: usize,
+    fault_ip: usize,
 ) -> Result<(), EmitterError> {
     // https://docs.rs/backtrace/latest/backtrace/index.html
     writeln!(w, "{DD_CRASHTRACK_BEGIN_STACKTRACE}")?;
@@ -61,56 +61,68 @@ unsafe fn emit_backtrace_by_frames(
         Ok(())
     }
 
-    backtrace::trace_unsynchronized(|frame| {
-        // Skip all stack frames whose stack pointer is less than to the determined crash stack
-        // pointer (fault_rsp). These frames belong exclusively to the crash tracker and the
-        // backtrace functionality and are therefore not relevant for troubleshooting.
-        let sp = frame.sp();
-        if !sp.is_null() && (sp as usize) < fault_rsp {
-            return true;
-        }
-        if resolve_frames == StacktraceCollection::EnabledWithInprocessSymbols {
-            backtrace::resolve_frame_unsynchronized(frame, |symbol| {
-                #[allow(clippy::unwrap_used)]
-                write!(w, "{{").unwrap();
-                #[allow(clippy::unwrap_used)]
-                emit_absolute_addresses(w, frame).unwrap();
-                if let Some(column) = symbol.colno() {
+    let mut ip_found = false;
+    loop {
+        backtrace::trace_unsynchronized(|frame| {
+            // Skip all stack frames until we encounter the determined crash instruction pointer
+            // (fault_ip). These initial frames belong exclusively to the crash tracker and the
+            // backtrace functionality and are therefore not relevant for troubleshooting.
+            let ip = frame.ip();
+            if ip as usize == fault_ip {
+                ip_found = true;
+            }
+            if !ip_found {
+                return true;
+            }
+            if resolve_frames == StacktraceCollection::EnabledWithInprocessSymbols {
+                backtrace::resolve_frame_unsynchronized(frame, |symbol| {
                     #[allow(clippy::unwrap_used)]
-                    write!(w, ", \"column\": {column}").unwrap();
-                }
-                if let Some(file) = symbol.filename() {
-                    // The debug printer for path already wraps it in `"` marks.
+                    write!(w, "{{").unwrap();
                     #[allow(clippy::unwrap_used)]
-                    write!(w, ", \"file\": {file:?}").unwrap();
-                }
-                if let Some(function) = symbol.name() {
+                    emit_absolute_addresses(w, frame).unwrap();
+                    if let Some(column) = symbol.colno() {
+                        #[allow(clippy::unwrap_used)]
+                        write!(w, ", \"column\": {column}").unwrap();
+                    }
+                    if let Some(file) = symbol.filename() {
+                        // The debug printer for path already wraps it in `"` marks.
+                        #[allow(clippy::unwrap_used)]
+                        write!(w, ", \"file\": {file:?}").unwrap();
+                    }
+                    if let Some(function) = symbol.name() {
+                        #[allow(clippy::unwrap_used)]
+                        write!(w, ", \"function\": \"{function}\"").unwrap();
+                    }
+                    if let Some(line) = symbol.lineno() {
+                        #[allow(clippy::unwrap_used)]
+                        write!(w, ", \"line\": {line}").unwrap();
+                    }
                     #[allow(clippy::unwrap_used)]
-                    write!(w, ", \"function\": \"{function}\"").unwrap();
-                }
-                if let Some(line) = symbol.lineno() {
+                    writeln!(w, "}}").unwrap();
+                    // Flush eagerly to ensure that each frame gets emitted even if the next one
+                    // fails
                     #[allow(clippy::unwrap_used)]
-                    write!(w, ", \"line\": {line}").unwrap();
-                }
+                    w.flush().unwrap();
+                });
+            } else {
+                #[allow(clippy::unwrap_used)]
+                write!(w, "{{").unwrap();
+                #[allow(clippy::unwrap_used)]
+                emit_absolute_addresses(w, frame).unwrap();
                 #[allow(clippy::unwrap_used)]
                 writeln!(w, "}}").unwrap();
                 // Flush eagerly to ensure that each frame gets emitted even if the next one fails
                 #[allow(clippy::unwrap_used)]
                 w.flush().unwrap();
-            });
-        } else {
-            #[allow(clippy::unwrap_used)]
-            write!(w, "{{").unwrap();
-            #[allow(clippy::unwrap_used)]
-            emit_absolute_addresses(w, frame).unwrap();
-            #[allow(clippy::unwrap_used)]
-            writeln!(w, "}}").unwrap();
-            // Flush eagerly to ensure that each frame gets emitted even if the next one fails
-            #[allow(clippy::unwrap_used)]
-            w.flush().unwrap();
+            }
+            true // keep going to the next frame
+        });
+        if ip_found {
+            break;
         }
-        true // keep going to the next frame
-    });
+        // emit anything at all, if the crashing frame is not found for some reason
+        ip_found = true;
+    }
     writeln!(w, "{DD_CRASHTRACK_END_STACKTRACE}")?;
     w.flush()?;
     Ok(())
@@ -145,8 +157,8 @@ pub(crate) fn emit_crashreport(
     // https://doc.rust-lang.org/src/std/backtrace.rs.html#332
     // Do this last, so even if it crashes, we still get the other info.
     if config.resolve_frames() != StacktraceCollection::Disabled {
-        let fault_rsp = extract_rsp(ucontext);
-        unsafe { emit_backtrace_by_frames(pipe, config.resolve_frames(), fault_rsp)? };
+        let fault_ip = extract_ip(ucontext);
+        unsafe { emit_backtrace_by_frames(pipe, config.resolve_frames(), fault_ip)? };
     }
     writeln!(pipe, "{DD_CRASHTRACK_DONE}")?;
     pipe.flush()?;
@@ -307,17 +319,17 @@ fn emit_text_file(w: &mut impl Write, path: &str) -> Result<(), EmitterError> {
     Ok(())
 }
 
-fn extract_rsp(ucontext: *const ucontext_t) -> usize {
+fn extract_ip(ucontext: *const ucontext_t) -> usize {
     unsafe {
         #[cfg(all(target_os = "macos", target_arch = "x86_64"))]
-        return (*(*ucontext).uc_mcontext).__ss.__rsp as usize;
+        return (*(*ucontext).uc_mcontext).__ss.__rip as usize;
         #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
-        return (*(*ucontext).uc_mcontext).__ss.__sp as usize;
+        return (*(*ucontext).uc_mcontext).__ss.__pc as usize;
 
         #[cfg(all(target_os = "linux", target_arch = "x86_64"))]
-        return (*ucontext).uc_mcontext.gregs[libc::REG_RSP as usize] as usize;
+        return (*ucontext).uc_mcontext.gregs[libc::REG_RIP as usize] as usize;
         #[cfg(all(target_os = "linux", target_arch = "aarch64"))]
-        return (*ucontext).uc_mcontext.sp as usize;
+        return (*ucontext).uc_mcontext.pc as usize;
     }
 }
 
@@ -326,14 +338,28 @@ mod tests {
     use super::*;
     use std::str;
 
-    #[test]
-    #[cfg_attr(miri, ignore)]
-    fn test_emit_backtrace_disabled() {
+    #[inline(never)]
+    fn inner_test_emit_backtrace_with_symbols(collection: StacktraceCollection) -> Vec<u8> {
+        let mut ip_of_test_fn = 0;
+        let mut skip = 3;
+        unsafe {
+            backtrace::trace_unsynchronized(|frame| {
+                ip_of_test_fn = frame.ip() as usize;
+                skip -= 1;
+                skip > 0
+            })
+        };
         let mut buf = Vec::new();
         unsafe {
-            emit_backtrace_by_frames(&mut buf, StacktraceCollection::Disabled, 0)
-                .expect("to work ;-)");
+            emit_backtrace_by_frames(&mut buf, collection, ip_of_test_fn).expect("to work ;-)");
         }
+        buf
+    }
+
+    #[test]
+    #[cfg_attr(miri, ignore)]
+    fn test_emit_backtrace_disabled() {
+        let buf = inner_test_emit_backtrace_with_symbols(StacktraceCollection::Disabled);
         let out = str::from_utf8(&buf).expect("to be valid UTF8");
         assert!(out.contains("BEGIN_STACKTRACE"));
         assert!(out.contains("END_STACKTRACE"));
@@ -353,18 +379,10 @@ mod tests {
     #[test]
     #[cfg_attr(miri, ignore)]
     fn test_emit_backtrace_with_symbols() {
-        let dummy = 0u8;
+        let buf = inner_test_emit_backtrace_with_symbols(
+            StacktraceCollection::EnabledWithInprocessSymbols,
+        );
         // retrieve stack pointer for this function
-        let sp_of_test_fn = &dummy as *const u8 as usize;
-        let mut buf = Vec::new();
-        unsafe {
-            emit_backtrace_by_frames(
-                &mut buf,
-                StacktraceCollection::EnabledWithInprocessSymbols,
-                sp_of_test_fn,
-            )
-            .expect("to work ;-)");
-        }
         let out = str::from_utf8(&buf).expect("to be valid UTF8");
         assert!(out.contains("BEGIN_STACKTRACE"));
         assert!(out.contains("END_STACKTRACE"));

datadog-crashtracker/src/collector/emitters.rs

@@ -9,13 +9,15 @@ use std::{
 };
 
 use datadog_ipc::platform::locks::FLock;
-use spawn_worker::{assert_child_exit, entrypoint, fork::set_default_child_panic_handler, Stdio};
+use spawn_worker::{
+    assert_child_exit, entrypoint, fork::set_default_child_panic_handler, Stdio, TrampolineData,
+};
 use tempfile::tempdir;
 
 static ENV_LOCK_PATH: &str = "__LOCK_PATH";
 
 #[no_mangle]
-pub extern "C" fn flock_test_entrypoint() {
+pub extern "C" fn flock_test_entrypoint(_trampoline_data: &TrampolineData) {
     set_default_child_panic_handler();
     let lock_path = std::env::var(ENV_LOCK_PATH).unwrap();
 

datadog-ipc/tests/flock.rs

@@ -309,7 +309,7 @@ pub extern "C" fn ddog_sidecar_transport_drop(_: Box<SidecarTransport>) {}
 /// Caller must ensure the process is safe to fork, at the time when this method is called
 #[no_mangle]
 pub extern "C" fn ddog_sidecar_connect(connection: &mut *mut SidecarTransport) -> MaybeError {
-    let cfg = datadog_sidecar::config::Config::get();
+    let cfg = datadog_sidecar::config::FromEnv::config();
 
     let stream = Box::new(try_c!(datadog_sidecar::start_or_connect_to_sidecar(cfg)));
     *connection = Box::into_raw(stream);

datadog-sidecar-ffi/src/lib.rs

@@ -1,12 +1,12 @@
 // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/
 // SPDX-License-Identifier: Apache-2.0
 
+use ddcommon::Endpoint;
 use http::uri::{PathAndQuery, Scheme};
 use serde::{Deserialize, Serialize};
-use std::{collections::HashMap, path::PathBuf, time::Duration};
-
-use ddcommon::Endpoint;
 use spawn_worker::LibDependency;
+use std::sync::LazyLock;
+use std::{collections::HashMap, path::PathBuf, time::Duration};
 
 const ENV_SIDECAR_IPC_MODE: &str = "_DD_DEBUG_SIDECAR_IPC_MODE";
 const SIDECAR_IPC_MODE_SHARED: &str = "shared";
@@ -28,6 +28,8 @@ const ENV_SIDECAR_SELF_TELEMETRY: &str = "_DD_SIDECAR_SELF_TELEMETRY";
 
 const ENV_SIDECAR_WATCHDOG_MAX_MEMORY: &str = "_DD_SIDECAR_WATCHDOG_MAX_MEMORY";
 
+const ENV_SIDECAR_CRASHTRACKER_ENDPOINT: &str = "_DD_SIDECAR_CRASHTRACKER_ENDPOINT";
+
 const ENV_SIDECAR_APPSEC_SHARED_LIB_PATH: &str = "_DD_SIDECAR_APPSEC_SHARED_LIB_PATH";
 const ENV_SIDECAR_APPSEC_SOCKET_FILE_PATH: &str = "_DD_SIDECAR_APPSEC_SOCKET_FILE_PATH";
 const ENV_SIDECAR_APPSEC_LOCK_FILE_PATH: &str = "_DD_SIDECAR_APPSEC_LOCK_FILE_PATH";
@@ -89,6 +91,7 @@ pub struct Config {
     pub self_telemetry: bool,
     pub library_dependencies: Vec<LibDependency>,
     pub child_env: HashMap<std::ffi::OsString, std::ffi::OsString>,
+    pub crashtracker_endpoint: Option<Endpoint>,
     pub appsec_config: Option<AppSecConfig>,
     pub max_memory: usize,
 }
@@ -102,9 +105,11 @@ pub struct AppSecConfig {
     pub log_level: String,
 }
 
+static ENV_CONFIG: LazyLock<Config> = LazyLock::new(FromEnv::config);
+
 impl Config {
-    pub fn get() -> Self {
-        FromEnv::config()
+    pub fn get() -> &'static Self {
+        &ENV_CONFIG
     }
 
     pub fn to_env(&self) -> HashMap<&'static str, std::ffi::OsString> {
@@ -120,6 +125,9 @@ impl Config {
                 self.self_telemetry.to_string().into(),
             ),
         ]);
+        if let Ok(json) = serde_json::to_string(&self.crashtracker_endpoint) {
+            res.insert(ENV_SIDECAR_CRASHTRACKER_ENDPOINT, json.into());
+        }
         if self.appsec_config.is_some() {
             #[allow(clippy::unwrap_used)]
             res.extend(self.appsec_config.as_ref().unwrap().to_env());
@@ -225,6 +233,12 @@ impl FromEnv {
             .unwrap_or(0)
     }
 
+    fn crashtracker_endpoint() -> Option<Endpoint> {
+        std::env::var(ENV_SIDECAR_CRASHTRACKER_ENDPOINT)
+            .ok()
+            .and_then(|json| serde_json::from_str(&json).ok())
+    }
+
     pub fn config() -> Config {
         Config {
             ipc_mode: Self::ipc_mode(),
@@ -234,12 +248,13 @@ impl FromEnv {
             self_telemetry: Self::self_telemetry(),
             library_dependencies: vec![],
             child_env: std::env::vars_os().collect(),
+            crashtracker_endpoint: Self::crashtracker_endpoint(),
             appsec_config: Self::appsec_config(),
             max_memory: Self::max_memory(),
         }
     }
 
-    pub fn appsec_config() -> Option<AppSecConfig> {
+    fn appsec_config() -> Option<AppSecConfig> {
         let shared_lib_path = std::env::var_os(ENV_SIDECAR_APPSEC_SHARED_LIB_PATH)?;
         let socket_file_path = std::env::var_os(ENV_SIDECAR_APPSEC_SOCKET_FILE_PATH)?;
         let lock_file_path = std::env::var_os(ENV_SIDECAR_APPSEC_LOCK_FILE_PATH)?;

datadog-sidecar/src/config.rs

@@ -410,7 +410,7 @@ pub(crate) fn enable_logging() -> anyhow::Result<()> {
         let filter = MULTI_LOG_FILTER.add(config.log_level.clone());
         _ = PERMANENT_MIN_LOG_LEVEL.set(filter);
     }
-    MULTI_LOG_WRITER.add(config.log_method); // same than MULTI_LOG_FILTER
+    MULTI_LOG_WRITER.add(config.log_method.clone()); // same than MULTI_LOG_FILTER
 
     LogTracer::init()?;