Merge branch 'julio/remove-sed-from-builder' of github.com:DataDog/li…

…bdatadog into julio/remove-sed-from-builder

.github/workflows/stale.yml

@@ -0,0 +1,27 @@
+name: "Mark stale pull requests"
+
+on:
+  schedule:
+    - cron: "0 0 * * *"
+  workflow_dispatch:
+
+permissions:
+  issues: write
+  pull-requests: write
+
+jobs:
+  stale:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/stale@v8
+        with:
+          stale-pr-message: >
+            This pull request has been automatically marked as stale because it has not had recent activity.
+            It will be closed if no further activity occurs. To override this behavior, add the keep-open
+            label or update the PR.
+          days-before-issue-stale: -1
+          days-before-issue-close: -1
+          days-before-pr-stale: 90
+          days-before-pr-close: 14
+          stale-pr-label: "stale"
+          exempt-pr-labels: "keep-open"

.github/workflows/test.yml

@@ -25,7 +25,7 @@ jobs:
           android: true
           dotnet: true
           haskell: true
-          large-packages: false
+          large-packages: true
           docker-images: false
           swap-storage: true
 

bin_tests/tests/crashtracker_bin_test.rs

@@ -104,7 +104,8 @@ fn test_crash_tracking_bin(
     assert_eq!(
         serde_json::json!({
           "signum": 11,
-          "signame": "SIGSEGV"
+          "signame": "SIGSEGV",
+          "faulting_address": 0,
         }),
         crash_payload["siginfo"]
     );
@@ -146,7 +147,8 @@ fn assert_telemetry_message(crash_telemetry: &[u8]) {
             "profiler_collecting_sample:1",
             "profiler_inactive:0",
             "profiler_serializing:0",
-            "signame:SIGSEGV"
+            "signame:SIGSEGV",
+            "faulting_address:0x0000000000000000",
         ]),
         tags
     );

crashtracker-ffi/src/crash_info/datatypes.rs

@@ -237,7 +237,12 @@ impl<'a> TryFrom<SigInfo<'a>> for datadog_crashtracker::SigInfo {
     fn try_from(value: SigInfo<'a>) -> Result<Self, Self::Error> {
         let signum = value.signum;
         let signame = option_from_char_slice(value.signame)?;
-        Ok(Self { signum, signame })
+        let faulting_address = None; // TODO: Expose this to FFI
+        Ok(Self {
+            signum,
+            signame,
+            faulting_address,
+        })
     }
 }
 

crashtracker/src/collector/crash_handler.rs

@@ -254,7 +254,7 @@ pub fn shutdown_receiver() -> anyhow::Result<()> {
 extern "C" fn handle_posix_sigaction(signum: i32, sig_info: *mut siginfo_t, ucontext: *mut c_void) {
     // Handle the signal.  Note this has a guard to ensure that we only generate
     // one crash report per process.
-    let _ = handle_posix_signal_impl(signum);
+    let _ = handle_posix_signal_impl(signum, sig_info);
 
     // Once we've handled the signal, chain to any previous handlers.
     // SAFETY: This was created by [register_crash_handlers].  There is a tiny
@@ -303,7 +303,7 @@ extern "C" fn handle_posix_sigaction(signum: i32, sig_info: *mut siginfo_t, ucon
     };
 }
 
-fn handle_posix_signal_impl(signum: i32) -> anyhow::Result<()> {
+fn handle_posix_signal_impl(signum: i32, sig_info: *mut siginfo_t) -> anyhow::Result<()> {
     static NUM_TIMES_CALLED: AtomicU64 = AtomicU64::new(0);
     if NUM_TIMES_CALLED.fetch_add(1, SeqCst) > 0 {
         // In the case where some lower-level signal handler recovered the error
@@ -324,13 +324,27 @@ fn handle_posix_signal_impl(signum: i32) -> anyhow::Result<()> {
     anyhow::ensure!(!metadata_ptr.is_null(), "No crashtracking metadata");
     let (_metadata, metadata_string) = unsafe { metadata_ptr.as_ref().context("metadata ptr")? };
 
+    let faulting_address: Option<usize> =
+        if !sig_info.is_null() && (signum == libc::SIGSEGV || signum == libc::SIGBUS) {
+            unsafe { Some((*sig_info).si_addr() as usize) }
+        } else {
+            None
+        };
+
     match unsafe { receiver.as_mut().context("No crashtracking receiver")? } {
         ReceiverType::ForkedProcess(child) => {
             let pipe = child
                 .stdin
                 .as_mut()
                 .context("Crashtracker: Can't get pipe")?;
-            let res = emit_crashreport(pipe, config, config_str, metadata_string, signum);
+            let res = emit_crashreport(
+                pipe,
+                config,
+                config_str,
+                metadata_string,
+                signum,
+                faulting_address,
+            );
             let _ = pipe.flush();
             if config.wait_for_receiver {
                 // https://doc.rust-lang.org/std/process/struct.Child.html#method.wait
@@ -362,6 +376,7 @@ fn handle_posix_signal_impl(signum: i32) -> anyhow::Result<()> {
                 config_str,
                 metadata_string,
                 signum,
+                faulting_address,
             );
             let _ = unix_stream.flush();
             unix_stream
@@ -464,10 +479,15 @@ unsafe fn set_alt_stack() -> anyhow::Result<()> {
         return Ok(());
     }
 
+    // Ensure that the altstack size is the greater of 16 pages or SIGSTKSZ. This is necessary
+    // because the default SIGSTKSZ is 8KB, which we're starting to run into. This new size is
+    // arbitrary, but at least it's large enough for our purposes, and yet a small enough part of
+    // the process RSS that it shouldn't be a problem.
     let page_size = page_size::get();
+    let sigalstack_base_size = std::cmp::max(SIGSTKSZ, 16 * page_size);
     let stackp = mmap(
         ptr::null_mut(),
-        SIGSTKSZ + page_size::get(),
+        sigalstack_base_size + page_size,
         PROT_READ | PROT_WRITE,
         MAP_PRIVATE | MAP_ANON,
         -1,
@@ -487,7 +507,7 @@ unsafe fn set_alt_stack() -> anyhow::Result<()> {
     let stack = libc::stack_t {
         ss_sp: stackp,
         ss_flags: 0,
-        ss_size: SIGSTKSZ,
+        ss_size: sigalstack_base_size,
     };
     let rval = sigaltstack(&stack, ptr::null_mut());
     anyhow::ensure!(rval == 0, "sigaltstack failed {rval}");

crashtracker/src/collector/emitters.rs

@@ -101,10 +101,11 @@ pub(crate) fn emit_crashreport(
     config_str: &str,
     metadata_string: &str,
     signum: i32,
+    faulting_address: Option<usize>,
 ) -> anyhow::Result<()> {
     emit_metadata(pipe, metadata_string)?;
     emit_config(pipe, config_str)?;
-    emit_siginfo(pipe, signum)?;
+    emit_siginfo(pipe, signum, faulting_address)?;
     emit_procinfo(pipe)?;
     pipe.flush()?;
     emit_counters(pipe)?;
@@ -163,7 +164,11 @@ fn emit_proc_self_maps(w: &mut impl Write) -> anyhow::Result<()> {
     Ok(())
 }
 
-fn emit_siginfo(w: &mut impl Write, signum: i32) -> anyhow::Result<()> {
+fn emit_siginfo(
+    w: &mut impl Write,
+    signum: i32,
+    faulting_address: Option<usize>,
+) -> anyhow::Result<()> {
     let signame = if signum == libc::SIGSEGV {
         "SIGSEGV"
     } else if signum == libc::SIGBUS {
@@ -173,7 +178,14 @@ fn emit_siginfo(w: &mut impl Write, signum: i32) -> anyhow::Result<()> {
     };
 
     writeln!(w, "{DD_CRASHTRACK_BEGIN_SIGINFO}")?;
-    writeln!(w, "{{\"signum\": {signum}, \"signame\": \"{signame}\"}}")?;
+    if let Some(addr) = faulting_address {
+        writeln!(
+            w,
+            "{{\"signum\": {signum}, \"signame\": \"{signame}\", \"faulting_address\": {addr}}}"
+        )?;
+    } else {
+        writeln!(w, "{{\"signum\": {signum}, \"signame\": \"{signame}\"}}")?;
+    };
     writeln!(w, "{DD_CRASHTRACK_END_SIGINFO}")?;
     Ok(())
 }

crashtracker/src/crash_info/mod.rs

@@ -23,6 +23,9 @@ pub struct SigInfo {
     #[serde(skip_serializing_if = "Option::is_none")]
     #[serde(default)]
     pub signame: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    #[serde(default)]
+    pub faulting_address: Option<usize>,
 }
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]

crashtracker/src/crash_info/telemetry.rs

@@ -191,6 +191,9 @@ fn extract_crash_info_tags(crash_info: &CrashInfo) -> anyhow::Result<String> {
         if let Some(signame) = &siginfo.signame {
             write!(&mut tags, ",signame:{}", signame)?;
         }
+        if let Some(faulting_address) = &siginfo.faulting_address {
+            write!(&mut tags, ",faulting_address:{:#018x}", faulting_address)?;
+        }
     }
     for (counter, value) in &crash_info.counters {
         write!(&mut tags, ",{}:{}", counter, value)?;
@@ -276,6 +279,7 @@ mod tests {
             siginfo: Some(SigInfo {
                 signum: 11,
                 signame: Some("SIGSEGV".to_owned()),
+                faulting_address: Some(0x1234),
             }),
             proc_info: None,
             stacktrace: vec![],
@@ -308,7 +312,8 @@ mod tests {
                 "signum:11",
                 "signame:SIGSEGV",
                 "collecting_sample:1",
-                "not_profiling:0"
+                "not_profiling:0",
+                "faulting_address:0x0000000000001234",
             ]),
             tags
         );