diff --git a/Cargo.lock b/Cargo.lock index 3fd5276f74..8057cea510 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3299,14 +3299,17 @@ dependencies = [ "libc", "libdd-common-ffi", "libdd-profiling-heap-gotter", + "libdd-profiling-heap-sampler", ] [[package]] name = "libdd-profiling-heap-sampler" version = "0.1.0" dependencies = [ + "anyhow", "bindgen 0.71.1", "cc", + "elf", ] [[package]] diff --git a/libdd-profiling-heap-allocator/benches/sampler_overhead.rs b/libdd-profiling-heap-allocator/benches/sampler_overhead.rs index c71f476a01..22aaff25bf 100644 --- a/libdd-profiling-heap-allocator/benches/sampler_overhead.rs +++ b/libdd-profiling-heap-allocator/benches/sampler_overhead.rs @@ -38,6 +38,10 @@ mod linux_bench { // Return a stable aligned pointer with mapped bytes before it. The sampler's free path // may inspect header-sized bytes immediately before the user pointer when // checking for sampled allocations. + // + // Always returns the same fixed pointer. This allocator isn't tracking real + // capacity or state; it exists purely to eliminate the real allocator's cost + // from the benchmark so it measures the sampler's own overhead. unsafe { ptr::addr_of_mut!(NOOP_BUFFER.0).cast::().add(4096) } } @@ -48,10 +52,21 @@ mod linux_bench { unsafe { ptr::addr_of_mut!(NOOP_BUFFER.0).cast::().add(4096) } } + /// # Safety + /// + /// Must be called on a thread that isn't concurrently tearing down its + /// TLS (i.e. not from a destructor); otherwise identical to + /// `dd_tl_state_get_or_init`'s own safety contract. unsafe fn sampler_tl_state() -> *mut libdd_profiling_heap_sampler::dd_tl_state_t { unsafe { dd_tl_state_get_or_init() } } + /// Pins this thread's sampler state onto the fast (unsampled) path for + /// the rest of the benchmark. `remaining_bytes` starts at a huge + /// negative value and `sampling_interval` at a huge positive one, so + /// benchmark-sized allocations can never drive `remaining_bytes` + /// non-negative and trigger the slow/sampled path. Dividing by 4 keeps + /// headroom against overflow while summing allocation sizes. unsafe fn pin_sampler_to_fast_path() { let tl = unsafe { sampler_tl_state() }; if !tl.is_null() { @@ -64,6 +79,10 @@ mod linux_bench { } } + /// Forces the next allocation on this thread onto the slow/sampled + /// path. `512 * 1024` matches `DD_SAMPLING_INTERVAL_DEFAULT` (see + /// tl_state.h), so this benchmarks the sampled path against the same + /// interval used in production rather than an arbitrary value. unsafe fn force_next_allocation_to_sample() { let tl = unsafe { sampler_tl_state() }; if !tl.is_null() { diff --git a/libdd-profiling-heap-allocator/src/allocator.rs b/libdd-profiling-heap-allocator/src/allocator.rs index 1965d3bee9..17e7434c49 100644 --- a/libdd-profiling-heap-allocator/src/allocator.rs +++ b/libdd-profiling-heap-allocator/src/allocator.rs @@ -71,6 +71,13 @@ unsafe impl GlobalAlloc for SampledAllocator { return self.inner.dealloc(ptr, layout); } let freed = dd_allocation_freed(ptr.cast(), layout.size(), layout.align()); + // `layout.align()` is reused here rather than anything derived from + // `freed`: alignment never changes between the original `alloc` and + // this `dealloc`, so pairing `freed.size` with the caller's own + // alignment is exactly what `GlobalAlloc::dealloc`'s safety contract + // already requires (the layout passed here must match the one used + // for the original allocation). This isn't a guarantee `SampledAllocator` + // adds — it's just satisfying the contract our caller is on the hook for. let inner_layout = Layout::from_size_align_unchecked(freed.size, layout.align()); self.inner.dealloc(freed.ptr.cast(), inner_layout); } diff --git a/libdd-profiling-heap-allocator/src/lib.rs b/libdd-profiling-heap-allocator/src/lib.rs index 453afbdec2..d550860a55 100644 --- a/libdd-profiling-heap-allocator/src/lib.rs +++ b/libdd-profiling-heap-allocator/src/lib.rs @@ -14,8 +14,9 @@ //! //! # Features //! -//! * `live-heap` (off by default) — enables live-heap tracking: allocations are flagged and frees -//! are sampled, so a profiler can balance allocs against frees. Off = allocation profiling only. +//! * `live-heap` (off by default) - enables live-heap tracking: sampled allocations are flagged at +//! alloc time, and that flag is detected again on free, so a profiler can pair each free back to +//! its sample and balance allocs against frees. Off = allocation profiling only. //! //! # Example //! diff --git a/libdd-profiling-heap-gotter-ffi/Cargo.toml b/libdd-profiling-heap-gotter-ffi/Cargo.toml index ee5b35b50c..062a64c20b 100644 --- a/libdd-profiling-heap-gotter-ffi/Cargo.toml +++ b/libdd-profiling-heap-gotter-ffi/Cargo.toml @@ -16,6 +16,8 @@ bench = false [features] default = ["cbindgen"] cbindgen = ["build_common/cbindgen", "libdd-common-ffi/cbindgen"] +# See libdd-profiling-heap-gotter's test-support feature. Not for production use. +test-support = ["libdd-profiling-heap-gotter/test-support"] [build-dependencies] build_common = { path = "../build-common" } @@ -31,3 +33,6 @@ libdd-profiling-heap-gotter = { path = "../libdd-profiling-heap-gotter" } [target.'cfg(target_os = "linux")'.dev-dependencies] libc = "0.2" +# For the USDT-note ELF inspection test (tests/usdt_notes.rs); mirrors how +# libdd-otel-thread-ctx-ffi pulls in its dep's `sanity-check` feature. +libdd-profiling-heap-sampler = { path = "../libdd-profiling-heap-sampler", features = ["sanity-check"] } diff --git a/libdd-profiling-heap-gotter-ffi/examples/cdylib_demo.rs b/libdd-profiling-heap-gotter-ffi/examples/cdylib_demo.rs index dc11a56896..598b6c7a7b 100644 --- a/libdd-profiling-heap-gotter-ffi/examples/cdylib_demo.rs +++ b/libdd-profiling-heap-gotter-ffi/examples/cdylib_demo.rs @@ -17,6 +17,13 @@ //! The gotter-ffi crate is Linux-only; on other targets the example //! compiles to a no-op `main` so clippy/test on non-Linux don't fail //! with "configured out". +//! +//! This demo deliberately loads the library via `dlopen`/`dlsym` even +//! though a real Rust binary would just link the crate directly. That's +//! contrived for Rust specifically, but it's the pattern every other +//! language's SDK actually has to use to consume this FFI surface, so +//! exercising it here catches issues (symbol visibility, ABI mismatches) +//! that a direct `extern crate` link would hide. #[cfg(not(target_os = "linux"))] fn main() -> Result<(), String> { @@ -54,6 +61,11 @@ mod linux { Ok(Self(handle)) } + /// # Safety + /// + /// `T` must be exactly the function pointer type of the symbol named + /// `name` in the loaded library; a mismatch is transmuted silently + /// and will call into the wrong signature. unsafe fn symbol(&self, name: &CStr) -> Result where T: Copy, @@ -85,6 +97,11 @@ mod linux { } } + /// Infers the built cdylib's path from `cargo run --example`'s layout: + /// the example binary lands at `target//examples/`, and + /// the cdylib is a sibling of `examples/` at `target//`. + /// `DDOG_HEAP_GOTTER_FFI_CDYLIB` overrides this for anyone driving the + /// build differently. fn cdylib_path() -> Result { if let Some(path) = std::env::var_os("DDOG_HEAP_GOTTER_FFI_CDYLIB") { return Ok(PathBuf::from(path)); diff --git a/libdd-profiling-heap-gotter-ffi/src/lib.rs b/libdd-profiling-heap-gotter-ffi/src/lib.rs index f92092e4d8..c2374d8d1d 100644 --- a/libdd-profiling-heap-gotter-ffi/src/lib.rs +++ b/libdd-profiling-heap-gotter-ffi/src/lib.rs @@ -62,3 +62,14 @@ pub extern "C" fn ddog_heap_gotter_update() -> VoidResult { pub extern "C" fn ddog_heap_gotter_is_installed() -> bool { libdd_profiling_heap_gotter::heap_overrides_are_installed() } + +/// Test-only: number of times a patched hook (`malloc`/`free`) has run in +/// this process. Lets integration tests prove the patched GOT was actually +/// exercised, not just that nothing crashed. Not part of the production API +/// surface; only compiled in with the `test-support` feature. +#[cfg(feature = "test-support")] +#[no_mangle] +#[must_use] +pub extern "C" fn ddog_heap_gotter_test_hook_hits() -> u64 { + libdd_profiling_heap_gotter::test_hook_hits() +} diff --git a/libdd-profiling-heap-gotter-ffi/tests/install.rs b/libdd-profiling-heap-gotter-ffi/tests/install.rs index 8fe21f2c7c..46fa1eda10 100644 --- a/libdd-profiling-heap-gotter-ffi/tests/install.rs +++ b/libdd-profiling-heap-gotter-ffi/tests/install.rs @@ -14,6 +14,8 @@ #![cfg(all(target_os = "linux", target_pointer_width = "64", not(miri)))] use libdd_common_ffi::VoidResult; +#[cfg(feature = "test-support")] +use libdd_profiling_heap_gotter_ffi::ddog_heap_gotter_test_hook_hits; use libdd_profiling_heap_gotter_ffi::{ddog_heap_gotter_install, ddog_heap_gotter_is_installed}; #[track_caller] @@ -37,9 +39,19 @@ fn install_patches_the_got() { "is_installed should be true after install" ); - // Touch the heap while installed so the patched GOT actually gets - // used. We just need the process to still be alive after this. + // Touch the heap while installed so the patched GOT actually gets used. + #[cfg(feature = "test-support")] + let hits_before = ddog_heap_gotter_test_hook_hits(); + let v: Vec = vec![0; 128]; assert_eq!(v.len(), 128); drop(v); + + // With test-support, prove the hooks actually ran rather than just that + // the process survived. + #[cfg(feature = "test-support")] + assert!( + ddog_heap_gotter_test_hook_hits() > hits_before, + "expected the malloc/free hooks to run" + ); } diff --git a/libdd-profiling-heap-gotter-ffi/tests/usdt_notes.rs b/libdd-profiling-heap-gotter-ffi/tests/usdt_notes.rs new file mode 100644 index 0000000000..33213c2eb4 --- /dev/null +++ b/libdd-profiling-heap-gotter-ffi/tests/usdt_notes.rs @@ -0,0 +1,33 @@ +// Copyright 2026-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +//! Verify the ddheap USDT probes appear exactly once in the built shared +//! library's `.note.stapsdt`. A duplicate entry (e.g. from `static inline` + +//! bindgen's `wrap_static_fns`, or LTO inlining across TUs) would make an +//! attached consumer fire twice, so this guards the "one note per probe" +//! invariant documented in the sampler's `probes.c`. +//! +//! Inspecting the linked cdylib (rather than running the check in-process) +//! validates the real shipped artifact. Mirrors +//! libdd-otel-thread-ctx-ffi/tests/elf_properties.rs. +//! +//! The cdylib path is derived at runtime from the test executable location. +//! Both the test binary and the cdylib live in `target/<[triple/]profile>/deps/`. + +#![cfg(target_os = "linux")] + +use std::path::PathBuf; + +fn cdylib_path() -> PathBuf { + let exe = std::env::current_exe().expect("failed to read current executable path"); + exe.parent() + .expect("unexpected test executable path structure") + .join("liblibdd_profiling_heap_gotter_ffi.so") +} + +#[test] +#[cfg_attr(miri, ignore)] +fn ddheap_probes_have_one_note_each() { + let path = cdylib_path(); + libdd_profiling_heap_sampler::usdt_check::check_usdt_probes_in(&path).unwrap(); +} diff --git a/libdd-profiling-heap-gotter/Cargo.toml b/libdd-profiling-heap-gotter/Cargo.toml index 6b76597bbe..38cd0d0b97 100644 --- a/libdd-profiling-heap-gotter/Cargo.toml +++ b/libdd-profiling-heap-gotter/Cargo.toml @@ -19,6 +19,10 @@ bench = false [features] # Forward live-heap (retained-heap) tracking down to the sampler live-heap = ["libdd-profiling-heap-sampler/live-heap"] +# Exposes a hook-hit counter so integration tests in other crates (e.g. +# libdd-profiling-heap-gotter-ffi) can prove the patched GOT was actually +# used, not just that nothing crashed. Not for production use. +test-support = [] [target.'cfg(target_os = "linux")'.dependencies] libc = "0.2" diff --git a/libdd-profiling-heap-gotter/src/elf.rs b/libdd-profiling-heap-gotter/src/elf.rs index 8d9afd2cd9..27e786b2a5 100644 --- a/libdd-profiling-heap-gotter/src/elf.rs +++ b/libdd-profiling-heap-gotter/src/elf.rs @@ -531,6 +531,13 @@ struct PatchedLibrary { /// Identifies the library at this base address, so we can detect /// base-address reuse after a `dlclose` + `dlopen` places a /// different library at the same load address. + /// + /// Known limitation: detection keys on this name, not on library + /// contents. A different version of the same library reloaded at the + /// same base (identical path, changed contents, same base despite + /// ASLR) would slip through and we would restore stale GOT values. + /// This is judged unlikely enough in practice to document rather than + /// guard against with additional fingerprinting. library_name: String, /// Set each pass in which this library was seen; used to drop entries /// for libraries that have since been unloaded. diff --git a/libdd-profiling-heap-gotter/src/hooks.rs b/libdd-profiling-heap-gotter/src/hooks.rs index 15602db001..c7111c95b0 100644 --- a/libdd-profiling-heap-gotter/src/hooks.rs +++ b/libdd-profiling-heap-gotter/src/hooks.rs @@ -39,9 +39,27 @@ pub(crate) static ORIG_DLOPEN: AtomicUsize = AtomicUsize::new(0); /// Resolved address of the real `pthread_create`. pub(crate) static ORIG_PTHREAD_CREATE: AtomicUsize = AtomicUsize::new(0); +/// Counts hook invocations so integration tests outside this crate can +/// prove the patched GOT was actually exercised, not just that nothing +/// crashed. Only `malloc`/`free` increment it: that's enough to prove the +/// hooks ran without adding bookkeeping to every symbol. +#[cfg(feature = "test-support")] +pub(crate) static HOOK_HITS: AtomicUsize = AtomicUsize::new(0); + /// Load a resolved function pointer from one of the `ORIG_*` slots. +/// +/// # Safety +/// +/// `T` must be exactly the `extern "C" fn(...)` pointer type that +/// `apply_overrides` writes into `slot` (see the `ORIG_*` docs above); if +/// `slot` is still `0`, this returns `None` rather than transmuting. #[inline] unsafe fn load_fn(slot: &AtomicUsize) -> Option { + // `Acquire` pairs with the `store(Release)` in elf.rs's + // `apply_overrides`, which runs before the GOT is ever patched to + // route calls into these hooks. That gives a real happens-before + // edge: once this observes a non-zero slot, it's guaranteed to see + // the fully-published address, not just "no torn read". let v = slot.load(Ordering::Acquire); if v == 0 { None @@ -95,17 +113,21 @@ type PthreadCreateFn = unsafe extern "C" fn( #[no_mangle] pub unsafe extern "C" fn gotter_malloc(size: usize) -> *mut c_void { + #[cfg(feature = "test-support")] + HOOK_HITS.fetch_add(1, Ordering::Relaxed); let Some(real): Option = load_fn(&ORIG_MALLOC) else { return std::ptr::null_mut(); }; // Default alignment for malloc on glibc is 2*sizeof(void*) == 16. - let req = dd_allocation_requested(size, core::mem::align_of::() * 2); + let req = dd_allocation_requested(size, core::mem::align_of::<*mut c_void>() * 2); let raw = real(req.size); dd_allocation_created(raw, req) } #[no_mangle] pub unsafe extern "C" fn gotter_free(ptr: *mut c_void) { + #[cfg(feature = "test-support")] + HOOK_HITS.fetch_add(1, Ordering::Relaxed); let Some(real): Option = load_fn(&ORIG_FREE) else { return; }; @@ -125,13 +147,14 @@ pub unsafe extern "C" fn gotter_calloc(nmemb: usize, size: usize) -> *mut c_void let Some(total) = nmemb.checked_mul(size) else { return real(nmemb, size); }; - let req = dd_allocation_requested(total, core::mem::align_of::() * 2); + let req = dd_allocation_requested(total, core::mem::align_of::<*mut c_void>() * 2); // calloc takes (nmemb, size); when the sampler bumps `req.size` we // funnel the extra bytes into the size argument (nmemb stays 1's // worth conceptually). The simplest robust path is to switch to a // single (1, req.size) allocation when sampling kicks in, so the // underlying allocator zeroes everything we hand back. Unsampled // path keeps the user's (nmemb, size) verbatim. + // `req.weight == 0` is `!dd_alloc_req_is_sampled(req)`, inlined here to avoid a cross-FFI call. let raw = if req.weight == 0 { real(nmemb, size) } else { diff --git a/libdd-profiling-heap-gotter/src/lib.rs b/libdd-profiling-heap-gotter/src/lib.rs index aecd4b9092..9e15385ae2 100644 --- a/libdd-profiling-heap-gotter/src/lib.rs +++ b/libdd-profiling-heap-gotter/src/lib.rs @@ -62,7 +62,7 @@ mod elf; mod hooks; #[cfg(all(target_os = "linux", target_pointer_width = "64"))] -use std::sync::{Mutex, MutexGuard, TryLockError}; +use std::sync::{Mutex, MutexGuard}; #[cfg(all(target_os = "linux", target_pointer_width = "64"))] use elf::SymbolOverrides; @@ -77,6 +77,9 @@ static GLOBAL_OVERRIDES: Mutex> = Mutex::new(None); fn lock_global_overrides() -> MutexGuard<'static, Option> { GLOBAL_OVERRIDES .lock() + // Recover from poison: if a thread panicked during apply/update, the + // registry may be partially applied, but the next scan re-walks all + // libraries idempotently, making partial state harmless. .unwrap_or_else(|poisoned| poisoned.into_inner()) } @@ -109,8 +112,7 @@ pub fn install_heap_overrides() -> bool { register_all(&mut so); *guard = Some(so); } - let so = guard.as_mut().unwrap(); - so.apply_overrides(); + guard.as_mut().unwrap().apply_overrides(); // Heuristic: at least one ORIG slot resolved. any_orig_resolved() } @@ -126,14 +128,7 @@ pub fn install_heap_overrides() -> bool { /// doesn't need to call this directly. No-op on non-Linux targets. #[cfg(all(target_os = "linux", target_pointer_width = "64"))] pub fn update_heap_overrides() { - // `try_lock` so a dlopen happening on the same thread that owns the - // install lock doesn't deadlock - that thread will finish its - // outer apply_overrides, which already walks every library. - let mut guard = match GLOBAL_OVERRIDES.try_lock() { - Ok(guard) => guard, - Err(TryLockError::Poisoned(poisoned)) => poisoned.into_inner(), - Err(TryLockError::WouldBlock) => return, - }; + let mut guard = lock_global_overrides(); if let Some(so) = guard.as_mut() { so.update_overrides(); } @@ -157,59 +152,53 @@ pub fn heap_overrides_are_installed() -> bool { false } +/// Number of times a `gotter_*` hook has run in this process. Test-only: +/// lets integration tests in other crates (see +/// `libdd-profiling-heap-gotter-ffi/tests/install.rs`) prove the patched +/// GOT was actually exercised, not just that nothing crashed. Always `0` +/// on non-64-bit-Linux targets, where hooks never run. +#[cfg(feature = "test-support")] +#[cfg(all(target_os = "linux", target_pointer_width = "64"))] +pub fn test_hook_hits() -> u64 { + hooks::HOOK_HITS.load(std::sync::atomic::Ordering::Relaxed) as u64 +} + +/// See the Linux variant above. +#[cfg(feature = "test-support")] +#[cfg(not(all(target_os = "linux", target_pointer_width = "64")))] +pub fn test_hook_hits() -> u64 { + 0 +} + /// Register GOT overrides for every symbol this crate currently hooks. #[cfg(all(target_os = "linux", target_pointer_width = "64"))] fn register_all(so: &mut SymbolOverrides) { use hooks::*; - use std::sync::atomic::AtomicUsize; // Register one entry per supported symbol. The install path stores // via `store(Release)` and hooks read via `load(Acquire)`; both go // through the typed atomic to avoid racing plain writes against // atomic loads. - fn reg(so: &mut SymbolOverrides, name: &str, hook_addr: usize, slot: &'static AtomicUsize) { - so.register(name, hook_addr, slot); - } - - reg( - so, - "malloc", - gotter_malloc as *const () as usize, - &ORIG_MALLOC, - ); - reg(so, "free", gotter_free as *const () as usize, &ORIG_FREE); - reg( - so, - "calloc", - gotter_calloc as *const () as usize, - &ORIG_CALLOC, - ); - reg( - so, + so.register("malloc", gotter_malloc as *const () as usize, &ORIG_MALLOC); + so.register("free", gotter_free as *const () as usize, &ORIG_FREE); + so.register("calloc", gotter_calloc as *const () as usize, &ORIG_CALLOC); + so.register( "realloc", gotter_realloc as *const () as usize, &ORIG_REALLOC, ); - reg( - so, + so.register( "posix_memalign", gotter_posix_memalign as *const () as usize, &ORIG_POSIX_MEMALIGN, ); - reg( - so, + so.register( "aligned_alloc", gotter_aligned_alloc as *const () as usize, &ORIG_ALIGNED_ALLOC, ); - reg( - so, - "dlopen", - gotter_dlopen as *const () as usize, - &ORIG_DLOPEN, - ); - reg( - so, + so.register("dlopen", gotter_dlopen as *const () as usize, &ORIG_DLOPEN); + so.register( "pthread_create", gotter_pthread_create as *const () as usize, &ORIG_PTHREAD_CREATE, diff --git a/libdd-profiling-heap-gotter/tests/install.rs b/libdd-profiling-heap-gotter/tests/install.rs index ba5bd03a05..a5cacbe4a4 100644 --- a/libdd-profiling-heap-gotter/tests/install.rs +++ b/libdd-profiling-heap-gotter/tests/install.rs @@ -24,11 +24,30 @@ use std::ffi::c_void; use libdd_profiling_heap_sampler::{dd_sample_flag_peek, dd_tl_state_get_or_init}; use serial_test::serial; +/// Warn (once) when these tests aren't running under nextest. The GOT install +/// is permanent, so isolation depends on each test getting its own process; +/// nextest sets `NEXTEST=1`, plain `cargo test` shares one process and leaks +/// a prior test's install into later ones. +fn warn_if_not_isolated() { + use std::sync::Once; + static ONCE: Once = Once::new(); + ONCE.call_once(|| { + if std::env::var_os("NEXTEST").is_none() { + eprintln!( + "warning: heap-gotter install tests need per-test process isolation; \ + run with `cargo nextest run`. Under `cargo test` a prior test's \ + permanent GOT install leaks into later ones." + ); + } + }); +} + /// After install the heap should still be functional and no recursive /// crash should occur when malloc/free go through the patched GOT. #[test] #[serial] fn install_keeps_heap_functional() { + warn_if_not_isolated(); extern "C" { fn malloc(size: usize) -> *mut c_void; } @@ -40,9 +59,17 @@ fn install_keeps_heap_functional() { ); unsafe { - let p = malloc(64); + let p = malloc(64) as *mut u8; assert!(!p.is_null(), "malloc returned NULL post-install"); - libc::free(p); + // Write then read back the whole buffer to prove it's real, usable + // memory, not just a non-NULL (possibly mis-offset) pointer. + for i in 0..64 { + p.add(i).write((i as u8) ^ 0xA5); + } + for i in 0..64 { + assert_eq!(p.add(i).read(), (i as u8) ^ 0xA5, "byte {i} corrupted"); + } + libc::free(p as *mut c_void); } } @@ -54,6 +81,7 @@ fn install_keeps_heap_functional() { #[test] #[serial] fn install_produces_sampled_allocations() { + warn_if_not_isolated(); let installed = libdd_profiling_heap_gotter::install_heap_overrides(); assert!(installed); @@ -97,6 +125,7 @@ fn install_produces_sampled_allocations() { #[test] #[serial] fn realloc_null_produces_sampled_allocation() { + warn_if_not_isolated(); let installed = libdd_profiling_heap_gotter::install_heap_overrides(); assert!(installed); @@ -130,6 +159,7 @@ fn realloc_null_produces_sampled_allocation() { #[test] #[serial] fn page_aligned_allocations_are_unsampled() { + warn_if_not_isolated(); let installed = libdd_profiling_heap_gotter::install_heap_overrides(); assert!(installed); @@ -162,6 +192,7 @@ fn page_aligned_allocations_are_unsampled() { #[test] #[serial] fn realloc_of_sampled_allocation_preserves_data() { + warn_if_not_isolated(); let installed = libdd_profiling_heap_gotter::install_heap_overrides(); assert!(installed); @@ -246,6 +277,7 @@ unsafe fn alloc_aligned(align: usize, size: usize) -> *mut c_void { #[test] #[serial] fn realloc_stress_across_alignments_preserves_data() { + warn_if_not_isolated(); // Mirrors the demo's menu, plus 2048 to bracket the 1024 cap on both // sides. Small alignments sample; those above the cap pass through. const ALIGNMENTS: &[usize] = &[1, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192]; @@ -336,3 +368,36 @@ fn realloc_stress_across_alignments_preserves_data() { #[cfg(not(feature = "live-heap"))] let _ = saw_sampled; } + +/// A failing allocation must surface the real allocator's error/errno through +/// our hooks unchanged. Avoids relying on `aligned_alloc` rejecting a bad +/// alignment, since older glibc (e.g. CentOS 7) doesn't validate that. +#[test] +#[serial] +fn invalid_alignment_passes_through_error() { + warn_if_not_isolated(); + let installed = libdd_profiling_heap_gotter::install_heap_overrides(); + assert!(installed); + + unsafe { + // posix_memalign with a non-power-of-two alignment must return EINVAL. + // POSIX-specified, so reliable across glibc versions; exercises our + // hook forwarding the real return code unchanged. + let mut out: *mut c_void = std::ptr::null_mut(); + let rc = libc::posix_memalign(&mut out, 3, 64); + assert_eq!(rc, libc::EINVAL, "posix_memalign should return EINVAL"); + + // A failing allocation must preserve the real allocator's errno through + // our post-call bookkeeping. A huge aligned_alloc reliably fails with + // ENOMEM on every libc (size is a multiple of the alignment). + *libc::__errno_location() = 0; + let huge = usize::MAX & !0xfff; + let p = libc::aligned_alloc(16, huge); + assert!(p.is_null(), "huge aligned_alloc should fail"); + assert_eq!( + std::io::Error::last_os_error().raw_os_error(), + Some(libc::ENOMEM), + "errno must survive our post-call bookkeeping" + ); + } +} diff --git a/libdd-profiling-heap-sampler/Cargo.toml b/libdd-profiling-heap-sampler/Cargo.toml index 11cbbaa02f..5a27aaeac8 100644 --- a/libdd-profiling-heap-sampler/Cargo.toml +++ b/libdd-profiling-heap-sampler/Cargo.toml @@ -22,6 +22,14 @@ default = [] # against allocs for retained/live-heap profiling, and sample # frees to emit ddheap:free. live-heap = [] +# Runtime ELF self-inspection of the USDT notes (see src/usdt_check.rs). Opt-in +# so the crate keeps zero runtime deps by default; only pulls in elf/anyhow. +sanity-check = ["dep:elf", "dep:anyhow"] + +# Only used by the `sanity-check` feature; the crate has no runtime deps otherwise. +[dependencies] +anyhow = { version = "1.0", optional = true } +elf = { version = "0.7", optional = true } [build-dependencies] cc = "1.1.31" diff --git a/libdd-profiling-heap-sampler/build.rs b/libdd-profiling-heap-sampler/build.rs index 7b4c124cd6..091a626db2 100644 --- a/libdd-profiling-heap-sampler/build.rs +++ b/libdd-profiling-heap-sampler/build.rs @@ -95,6 +95,14 @@ mod linux { // Translate the `live-heap` cargo feature into a C define. let live_heap = env::var_os("CARGO_FEATURE_LIVE_HEAP").is_some(); + // Define NDEBUG for optimized builds so C `assert()`s are stripped in + // release and live in dev/test (incl. `cargo test`). cc doesn't set + // this itself; `.debug()` only controls `-g`. Key off OPT_LEVEL, not + // DEBUG: this workspace's release profile keeps line-table debuginfo, + // so cargo reports DEBUG=true there too; OPT_LEVEL is "0" only for the + // unoptimized dev/test profile. + let optimized = env::var("OPT_LEVEL").as_deref() != Ok("0"); + let mut build = cc::Build::new(); build .files(SOURCES) @@ -112,6 +120,9 @@ mod linux { // loads it works on both glibc and musl without allocation // concerns (see tl_state.h for the full analysis). .flag_if_supported("-mtls-dialect=gnu2"); + if optimized { + build.define("NDEBUG", None); + } build.compile("dd_heap_sampler"); // `allocation_requested.c` calls `log()`; glibc keeps it in libm, diff --git a/libdd-profiling-heap-sampler/docs/tagging.md b/libdd-profiling-heap-sampler/docs/tagging.md index 653c2bd3f4..cbd884e46b 100644 --- a/libdd-profiling-heap-sampler/docs/tagging.md +++ b/libdd-profiling-heap-sampler/docs/tagging.md @@ -29,7 +29,7 @@ this case. Although x86-64 typically supports pointer tagging and it was (briefly) enabled in the kernel, it was pulled back out around Spectre due to security concerns. -Instead we steal, bytes from the allocation itself - when we decide to sample, we ask the +Instead we steal bytes from the allocation itself - when we decide to sample, we ask the underlying allocator for more memory than the caller requested, then place the user-visible pointer some way into that block, leaving room for a 16-byte header just before it. @@ -69,15 +69,16 @@ an unsampled allocation, and if that allocation's contents happen to look like our magic at the right offset, we'd wrongly treat it as sampled. Zeroing prevents that. -**Keeping the three sites in sync** +**Keeping the sizing in one place** -The formula for the bumped allocation size has to be computed identically in -three places: when we decide how much extra to ask for (`bumped_alloc_size` -in `allocation_requested.c`), when we place the header and user pointer -(`x86_apply` in `sample_flag.h`), and when we work out how big the original -allocation was so we can free the right amount (`dd_allocation_freed_slow` -in `allocation_freed.c`). If any of these disagree we corrupt memory. Touch -one, check the other two. +The bumped allocation size is computed by a single shared inline, +`x86_bumped_size` in `sample_flag.h`. Both the alloc side (`bumped_alloc_size` +in `allocation_requested.c`, deciding how much extra to ask for) and the free +side (`dd_allocation_freed_slow` in `allocation_freed.c`, recovering the size +for sized-free) call it, so the formula can't drift between them. The header +placement (`x86_apply`, also in `sample_flag.h`) shares the same +`x86_base_offset` helper for the reserve. All the inlines are +`always_inline`, so this centralisation adds no call overhead. **Alignment cap** diff --git a/libdd-profiling-heap-sampler/include/datadog/heap/allocation_created.h b/libdd-profiling-heap-sampler/include/datadog/heap/allocation_created.h index 715e8ce13c..120def3f25 100644 --- a/libdd-profiling-heap-sampler/include/datadog/heap/allocation_created.h +++ b/libdd-profiling-heap-sampler/include/datadog/heap/allocation_created.h @@ -44,7 +44,7 @@ void *dd_allocation_created_slow(void *raw, dd_alloc_req_t req) */ static inline __attribute__((always_inline, warn_unused_result)) void *dd_allocation_created(void *raw, dd_alloc_req_t req) { - if (__builtin_expect(req.weight == 0, 1)) return raw; + if (__builtin_expect(!dd_alloc_req_is_sampled(req), 1)) return raw; return dd_allocation_created_slow(raw, req); } diff --git a/libdd-profiling-heap-sampler/include/datadog/heap/allocation_freed.h b/libdd-profiling-heap-sampler/include/datadog/heap/allocation_freed.h index 84b64854fe..0d03a462cd 100644 --- a/libdd-profiling-heap-sampler/include/datadog/heap/allocation_freed.h +++ b/libdd-profiling-heap-sampler/include/datadog/heap/allocation_freed.h @@ -54,15 +54,21 @@ dd_alloc_freed_t dd_allocation_freed_slow(void *ptr, void *raw, size_t size, * verbatim. * * Args cover the superset of inputs any free-like call carries: - * ptr - allocation being freed (user pointer returned by alloc) + * ptr - allocation being freed (user pointer returned by alloc), + * or NULL (free(NULL) is a no-op; returned unchanged) * size - size the caller knows about, or 0 if unknown (plain free) - * alignment - alignment used at allocation time, or 0 + * alignment - alignment used at allocation time. Pass 0 if unknown (e.g. + * plain free): the returned size then falls back to a best- + * effort value, which is fine because plain free ignores it. + * Sized-free callers (Rust GlobalAlloc::dealloc, sdallocx, + * sized operator delete) must pass the real alignment so the + * returned size exactly matches the bumped allocation. */ static inline __attribute__((always_inline)) dd_alloc_freed_t dd_allocation_freed(void *ptr, size_t size, size_t alignment) { #if DD_HEAP_LIVE_TRACKING void *raw; - if (__builtin_expect(dd_sample_flag_check(ptr, &raw), 0)) { + if (__builtin_expect(dd_sample_flag_check_and_clear(ptr, &raw), 0)) { return dd_allocation_freed_slow(ptr, raw, size, alignment); } #else diff --git a/libdd-profiling-heap-sampler/include/datadog/heap/allocation_requested.h b/libdd-profiling-heap-sampler/include/datadog/heap/allocation_requested.h index 35421c5042..fb6a6e85a0 100644 --- a/libdd-profiling-heap-sampler/include/datadog/heap/allocation_requested.h +++ b/libdd-profiling-heap-sampler/include/datadog/heap/allocation_requested.h @@ -13,7 +13,7 @@ * The returned dd_alloc_req_t carries the values the caller must forward: * - size: the number of bytes to request from the real allocator * (may be larger than the original on architectures that - * store the sample flag in a header word before the user + * store the sample flag in a 16-byte header before the user * pointer). * - user_size: the original application-requested size, reported to * the profiler via the ddheap:alloc USDT. @@ -28,6 +28,7 @@ #ifndef DD_SAMPLERS_ALLOCATION_REQUESTED_H #define DD_SAMPLERS_ALLOCATION_REQUESTED_H +#include #include #include @@ -64,6 +65,18 @@ typedef struct { uint64_t weight; } dd_alloc_req_t; +/* + * True if this request was sampled (weight > 0). Both the C fast path + * (allocation_created.h) and callers that branch on sampled-ness (e.g. + * gotter's calloc hook) should use this instead of comparing `weight == 0` + * directly, so there is one named predicate rather than the same + * comparison repeated in C and Rust. + */ +static inline __attribute__((always_inline)) +bool dd_alloc_req_is_sampled(dd_alloc_req_t req) { + return req.weight != 0; +} + /* Slow path for an allocation request. This is only taken when we think we * need to sample, and is declared as a separate function to avoid bloating * the instruction cache of the fast path @@ -94,9 +107,10 @@ static inline __attribute__((always_inline, warn_unused_result)) dd_alloc_req_t dd_allocation_requested(size_t size, size_t alignment) { dd_alloc_req_t out = { size, size, alignment, 0 }; - // If we don't have TLS yet, or the reentry guard is set (meaning a sampled - // allocation is already in flight on this thread and something in its slow - // path triggered another allocation), pass through without sampling. + // If we don't have TLS yet (this is defensive and should never happen), or + // the reentry guard is set (meaning a sampled allocation is already in + // flight on this thread and something in its slow path triggered another + // allocation), pass through without sampling. // Either condition is rare on a hot path, so mark the branch unlikely. dd_tl_state_t *tl = dd_tl_state_get_or_init(); if (__builtin_expect(!tl || tl->reentry_guard, 0)) return out; diff --git a/libdd-profiling-heap-sampler/include/datadog/heap/sample_flag.h b/libdd-profiling-heap-sampler/include/datadog/heap/sample_flag.h index 9096530e1c..2b2e423ac7 100644 --- a/libdd-profiling-heap-sampler/include/datadog/heap/sample_flag.h +++ b/libdd-profiling-heap-sampler/include/datadog/heap/sample_flag.h @@ -54,7 +54,7 @@ bool dd_sample_flag_thread_init(void); void *dd_sample_flag_apply(void *raw, size_t alignment); /* - * Non-destructive variant of dd_sample_flag_check. Useful for realloc: + * Non-destructive variant of dd_sample_flag_check_and_clear. Useful for realloc: * callers can resolve the raw pointer before calling the underlying * realloc, while leaving the old allocation's flag intact in case * realloc fails and the old allocation remains live. @@ -111,6 +111,78 @@ bool dd_sample_flag_peek(void *user, void **raw_out, size_t *offset_out); #define DD_MAGIC 0xfab1eddec0dedca7ULL +/* + * Bytes reserved before the user pointer for the header + alignment slack: + * max(alignment, DD_HEADER_BYTES). x86_apply may add one more of these as a + * page-boundary bump (see x86_apply). + */ +static inline __attribute__((always_inline)) +size_t x86_base_offset(size_t alignment) { + return alignment > DD_HEADER_BYTES ? alignment : DD_HEADER_BYTES; +} + +/* + * Single source of truth for the bumped allocation size of a sampled + * `user_size`-byte request at `alignment`. Reserves 2 * base_offset (header + * plus room for the page-boundary bump) and rounds up to a multiple of + * alignment so aligned_alloc's size constraint holds. Returns false (leaving + * *out_size untouched) if alignment exceeds the cap or the arithmetic would + * overflow SIZE_MAX. bumped_alloc_size (allocation_requested.c) and + * dd_allocation_freed_slow (allocation_freed.c) both call this so the formula + * can never drift between the alloc and free sides. + */ +static inline __attribute__((always_inline)) +bool x86_bumped_size(size_t user_size, size_t alignment, size_t *out_size) { + if (alignment > DD_SAMPLE_ALIGNMENT_CAP) return false; + size_t base = x86_base_offset(alignment); + if (base > SIZE_MAX / 2) return false; + size_t reserve = base * 2; + if (reserve > SIZE_MAX - user_size) return false; + size_t bumped = user_size + reserve; + if (alignment > 1) { + size_t mask = alignment - 1; + if (bumped > SIZE_MAX - mask) return false; + bumped = (bumped + mask) & ~mask; + } + *out_size = bumped; + return true; +} + +/* + * The 16-byte header stamped immediately before a sampled user pointer: + * an 8-byte magic followed by the 8-byte offset from `user` back to `raw`. + * Read/written as one struct so the layout lives in a single place instead + * of being open-coded as two memcpys at every site. + */ +typedef struct { + uint64_t magic; + uint64_t offset; +} x86_header_t; + +_Static_assert(sizeof(x86_header_t) == DD_HEADER_BYTES, + "x86_header_t must exactly fill the reserved header"); + +/* Stamp the header in front of `user` (offset = user - raw). */ +static inline __attribute__((always_inline)) +void x86_header_stamp(void *user, uint64_t offset) { + x86_header_t hdr = { DD_MAGIC, offset }; + memcpy((char *)user - DD_HEADER_BYTES, &hdr, sizeof(hdr)); +} + +/* Read the header in front of `user`. */ +static inline __attribute__((always_inline)) +x86_header_t x86_header_read(const void *user) { + x86_header_t hdr; + memcpy(&hdr, (const char *)user - DD_HEADER_BYTES, sizeof(hdr)); + return hdr; +} + +/* Zero the header so a later reuse of this block isn't misdetected as sampled. */ +static inline __attribute__((always_inline)) +void x86_header_clear(void *user) { + memset((char *)user - DD_HEADER_BYTES, 0, sizeof(x86_header_t)); +} + /* * Layout helpers. x86_apply and x86_raw_from_user MUST be each other's * inverse: the offset stamped at apply time is what lets @@ -137,17 +209,13 @@ bool dd_sample_flag_peek(void *user, void **raw_out, size_t *offset_out); static inline __attribute__((always_inline)) void *x86_apply(void *raw, size_t alignment) { uintptr_t r = (uintptr_t)raw; - size_t n = alignment > DD_HEADER_BYTES ? alignment : DD_HEADER_BYTES; + size_t n = x86_base_offset(alignment); uintptr_t u = r + n; if ((u & (DD_PAGE_SIZE - 1)) < DD_HEADER_BYTES) { - n += (alignment > DD_HEADER_BYTES ? alignment : DD_HEADER_BYTES); + n += x86_base_offset(alignment); u = r + n; } - uint64_t magic = DD_MAGIC; - uint64_t offset = (uint64_t)n; - memcpy((void *)(u - DD_HEADER_BYTES), &magic, sizeof(magic)); - memcpy((void *)(u - DD_HEADER_BYTES + sizeof(magic)), &offset, - sizeof(offset)); + x86_header_stamp((void *)u, (uint64_t)n); return (void *)u; } @@ -166,21 +234,16 @@ void *x86_raw_from_user(void *user, uint64_t offset) { * true. Otherwise leave *raw_out untouched and return false. */ static inline __attribute__((always_inline)) -bool dd_sample_flag_check(void *user, void **raw_out) { +bool dd_sample_flag_check_and_clear(void *user, void **raw_out) { if (((uintptr_t)user & (DD_PAGE_SIZE - 1)) < DD_HEADER_BYTES) { return false; } - void *header = (char *)user - DD_HEADER_BYTES; - uint64_t magic; - memcpy(&magic, header, sizeof(magic)); - if (magic != DD_MAGIC) { + x86_header_t hdr = x86_header_read(user); + if (hdr.magic != DD_MAGIC) { return false; } - - uint64_t offset; - memcpy(&offset, (char *)header + sizeof(magic), sizeof(offset)); - if (offset < DD_HEADER_BYTES || offset > 2 * DD_SAMPLE_ALIGNMENT_CAP) { + if (hdr.offset < DD_HEADER_BYTES || hdr.offset > 2 * DD_SAMPLE_ALIGNMENT_CAP) { return false; } @@ -188,10 +251,9 @@ bool dd_sample_flag_check(void *user, void **raw_out) { * (e.g. allocator returns the same block to a later, unsampled * allocation whose user data happens to encode the magic) doesn't * masquerade as a stale sampled allocation. */ - const uint64_t zeros[2] = { 0, 0 }; - memcpy(header, zeros, sizeof(zeros)); + x86_header_clear(user); - *raw_out = x86_raw_from_user(user, offset); + *raw_out = x86_raw_from_user(user, hdr.offset); return true; } @@ -207,7 +269,7 @@ bool dd_sample_flag_check(void *user, void **raw_out) { * true. Otherwise leave *raw_out untouched and return false. */ static inline __attribute__((always_inline)) -bool dd_sample_flag_check(void *user, void **raw_out) { +bool dd_sample_flag_check_and_clear(void *user, void **raw_out) { uintptr_t addr = (uintptr_t)user; if ((addr & DD_TBI_TAG_MASK) != DD_TBI_TAGGED) { return false; diff --git a/libdd-profiling-heap-sampler/src/allocation_created.c b/libdd-profiling-heap-sampler/src/allocation_created.c index afebc8d67a..9b12d62157 100644 --- a/libdd-profiling-heap-sampler/src/allocation_created.c +++ b/libdd-profiling-heap-sampler/src/allocation_created.c @@ -6,6 +6,8 @@ #include #include +#include + /* * Slow path for dd_allocation_created. We only arrive here when the paired * dd_allocation_requested_slow decided to sample (req.weight > 0). @@ -47,9 +49,16 @@ void *dd_allocation_created_slow(void *raw, dd_alloc_req_t req) { } /* Always close the reentry guard, even on allocation failure (raw == NULL), - * so the thread isn't permanently locked out of sampling. */ + * so the thread isn't permanently locked out of sampling. + * + * In the real gotter/allocator flow, created is always paired with a + * preceding requested that initialised this thread's TLS and opened the + * guard, so if this thread has sampler state the guard must be open. A + * NULL tl means the caller never went through requested (e.g. an isolated + * unit-test call), which is allowed and simply has nothing to close. */ dd_tl_state_t *tl = dd_tl_state_get(); + assert(!tl || tl->reentry_guard); if (tl) tl->reentry_guard = false; return user; -} \ No newline at end of file +} diff --git a/libdd-profiling-heap-sampler/src/allocation_freed.c b/libdd-profiling-heap-sampler/src/allocation_freed.c index bf50e83bd3..b8f6b89f0f 100644 --- a/libdd-profiling-heap-sampler/src/allocation_freed.c +++ b/libdd-profiling-heap-sampler/src/allocation_freed.c @@ -9,7 +9,7 @@ /* * Slow path for dd_allocation_freed. We only arrive here when - * dd_sample_flag_check confirmed that ptr carries the sample flag, + * dd_sample_flag_check_and_clear confirmed that ptr carries the sample flag, * meaning this allocation was previously sampled. * * Fires the ddheap:free USDT with the user-visible pointer, then returns @@ -32,37 +32,27 @@ dd_alloc_freed_t dd_allocation_freed_slow(void *ptr, void *raw, size_t size, }; #if defined(__x86_64__) - /* Recover the bumped size the allocator actually holds. This must - * exactly mirror allocation_requested.c's bumped_alloc_size(): + /* Recover the bumped size the allocator actually holds via the shared + * x86_bumped_size (same helper allocation_requested.c uses to decide how + * much to ask for), so the alloc and free sides can't drift. * - * base = max(alignment, DD_HEADER_BYTES) - * reserve = 2 * base - * bumped = round_up(size + reserve, alignment) + * Do not use (user - raw) as the reserve. That offset is usually only one + * `base`, while allocation reserved two so x86_apply() has room for its + * optional page-boundary bump. Sized-free callers (Rust + * GlobalAlloc::dealloc, sdallocx, operator delete(sz)) rely on this exact. * - * Do not use (user - raw) as the reserve. That offset is usually - * only one `base`, while allocation reserved two so x86_apply() has - * room for its optional page-boundary bump. Sized-free callers - * (Rust GlobalAlloc::dealloc, sdallocx, operator delete(sz)) rely - * on this being exact. - * - * When the caller doesn't know the alignment (alignment == 0), - * fall back to size + offset. Plain free() ignores out.size so this - * only matters for sized-free variants that must supply an alignment. */ + * When the caller doesn't know the alignment (alignment == 0), or the + * formula rejects it, fall back to size + offset. Plain free() ignores + * out.size so this only matters for sized-free variants. */ size_t offset = (size_t)((uintptr_t)ptr - (uintptr_t)raw); - if (alignment == 0) { + size_t bumped; + if (alignment == 0 || !x86_bumped_size(size, alignment, &bumped)) { out.size = size + offset; } else { - size_t base = alignment > DD_HEADER_BYTES ? alignment : DD_HEADER_BYTES; - size_t reserve = base * 2; - size_t bumped = size + reserve; - if (alignment > 1) { - size_t mask = alignment - 1; - bumped = (bumped + mask) & ~mask; - } out.size = bumped; } #else (void)alignment; #endif return out; -} \ No newline at end of file +} diff --git a/libdd-profiling-heap-sampler/src/allocation_realloc.c b/libdd-profiling-heap-sampler/src/allocation_realloc.c index c7db8cc337..5ae2fdf740 100644 --- a/libdd-profiling-heap-sampler/src/allocation_realloc.c +++ b/libdd-profiling-heap-sampler/src/allocation_realloc.c @@ -13,7 +13,8 @@ #define DD_REALLOC_DEFAULT_ALIGNMENT (sizeof(void *) * 2) dd_realloc_prep_t dd_allocation_realloc_prepare(void *old_user, size_t new_size) { - dd_realloc_prep_t out = { + /* Default: hand the call to the real realloc unchanged. */ + const dd_realloc_prep_t passthrough = { .raw_ptr = old_user, .raw_size = new_size, .old_offset = 0, @@ -22,34 +23,41 @@ dd_realloc_prep_t dd_allocation_realloc_prepare(void *old_user, size_t new_size) }; if (old_user == NULL) { - dd_alloc_req_t req = dd_allocation_requested( - new_size, DD_REALLOC_DEFAULT_ALIGNMENT); - out.raw_ptr = NULL; - out.raw_size = req.size; - out.alloc_req = req; - out.kind = DD_REALLOC_KIND_ALLOC; - return out; + /* realloc(NULL, n) == malloc(n): run the allocation sampling path. */ + dd_alloc_req_t req = + dd_allocation_requested(new_size, DD_REALLOC_DEFAULT_ALIGNMENT); + return (dd_realloc_prep_t){ + .raw_ptr = NULL, + .raw_size = req.size, + .old_offset = 0, + .alloc_req = req, + .kind = DD_REALLOC_KIND_ALLOC, + }; } if (new_size == 0) { + /* realloc(ptr, 0) == free(ptr) for the allocators we hook. */ dd_alloc_freed_t freed = dd_allocation_freed(old_user, 0, 0); - out.raw_ptr = freed.ptr; - out.raw_size = 0; - out.kind = DD_REALLOC_KIND_FREE; - return out; + return (dd_realloc_prep_t){ + .raw_ptr = freed.ptr, + .raw_size = 0, + .old_offset = 0, + .alloc_req = { 0, 0, 0, 0 }, + .kind = DD_REALLOC_KIND_FREE, + }; } #if DD_HEAP_LIVE_TRACKING void *old_raw = NULL; size_t old_offset = 0; if (!dd_sample_flag_peek(old_user, &old_raw, &old_offset)) { - return out; /* passthrough: not sampled */ + return passthrough; /* not sampled */ } /* Clear the magic header NOW, while we still own the block. Once the * real realloc runs it may free the old block internally; if we left * the magic intact, a future allocation that reuses that memory could - * be falsely detected as sampled by dd_sample_flag_check, leading to + * be falsely detected as sampled by dd_sample_flag_check_and_clear, leading to * a bogus raw pointer and a heap corruption on free. * * This is safe even if the real realloc subsequently fails (returns @@ -60,11 +68,7 @@ dd_realloc_prep_t dd_allocation_realloc_prepare(void *old_user, size_t new_size) * incorrect (it should pass old_raw). To handle this, commit() * re-stamps the header when realloc fails. */ #if defined(__x86_64__) - { - void *old_header = (char *)old_user - DD_HEADER_BYTES; - const uint64_t zero = 0; - memcpy(old_header, &zero, sizeof(zero)); - } + x86_header_clear(old_user); #endif /* Reserve room for the old header+slack ([0, old_offset)) plus @@ -78,30 +82,33 @@ dd_realloc_prep_t dd_allocation_realloc_prepare(void *old_user, size_t new_size) * before falling through to passthrough, otherwise a later free * would not recover old_raw. */ if (new_size > SIZE_MAX - old_offset) { - /* Hand the real realloc old_raw, not the offset user pointer, so - * this doomed oversized request fails cleanly instead of corrupting - * the heap. */ - out.raw_ptr = old_raw; #if defined(__x86_64__) - void *hdr = (char *)old_user - DD_HEADER_BYTES; - uint64_t m = DD_MAGIC; - uint64_t o = (uint64_t)old_offset; - memcpy(hdr, &m, sizeof(m)); - memcpy((char *)hdr + sizeof(m), &o, sizeof(o)); + x86_header_stamp(old_user, (uint64_t)old_offset); #endif - return out; + /* Hand the real realloc old_raw, not the offset user pointer, so + * this doomed oversized request fails cleanly instead of corrupting + * the heap. Otherwise a plain passthrough with the caller's size. */ + return (dd_realloc_prep_t){ + .raw_ptr = old_raw, + .raw_size = new_size, + .old_offset = 0, + .alloc_req = { 0, 0, 0, 0 }, + .kind = DD_REALLOC_KIND_PASSTHROUGH, + }; } - out.raw_ptr = old_raw; - out.raw_size = new_size + old_offset; - out.old_offset = old_offset; - out.kind = DD_REALLOC_KIND_SAMPLED; - return out; + return (dd_realloc_prep_t){ + .raw_ptr = old_raw, + .raw_size = new_size + old_offset, + .old_offset = old_offset, + .alloc_req = { 0, 0, 0, 0 }, + .kind = DD_REALLOC_KIND_SAMPLED, + }; #else /* Live-heap tracking off: existing blocks are never flagged, so there * is no sampled realloc to recover. Pass straight through to the real * realloc. (realloc(NULL, n) already took the ALLOC path above.) */ - return out; + return passthrough; #endif } @@ -122,14 +129,10 @@ void *dd_allocation_realloc_commit(void *old_user, void *new_raw, dd_realloc_pre /* Underlying realloc failed: C says old_user is still live. * prepare() cleared the header optimistically, so re-stamp it * now so that a later free(old_user) correctly resolves the raw - * pointer via dd_sample_flag_check. */ + * pointer via dd_sample_flag_check_and_clear. */ if (new_raw == NULL) { #if defined(__x86_64__) - void *old_header = (char *)old_user - DD_HEADER_BYTES; - uint64_t magic = DD_MAGIC; - uint64_t offset = (uint64_t)prep.old_offset; - memcpy(old_header, &magic, sizeof(magic)); - memcpy((char *)old_header + sizeof(magic), &offset, sizeof(offset)); + x86_header_stamp(old_user, (uint64_t)prep.old_offset); #endif return NULL; } diff --git a/libdd-profiling-heap-sampler/src/allocation_requested.c b/libdd-profiling-heap-sampler/src/allocation_requested.c index 504b5231c3..758b33cd95 100644 --- a/libdd-profiling-heap-sampler/src/allocation_requested.c +++ b/libdd-profiling-heap-sampler/src/allocation_requested.c @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -111,31 +112,9 @@ static uint64_t sample(dd_tl_state_t *tl) { static bool bumped_alloc_size(size_t user_size, size_t alignment, size_t *out_size) { #if defined(__x86_64__) && DD_HEAP_LIVE_TRACKING - if (alignment > DD_SAMPLE_ALIGNMENT_CAP) return false; - - /* Reserve twice the base offset so x86_apply's page-boundary bump - * (which may push the user pointer another `base` bytes forward) - * always fits inside the allocation. Base is max(alignment, 16): - * the minimum offset needed to seat the 16-byte header before the - * user pointer while staying alignment-aligned. */ - size_t base = alignment > DD_HEADER_BYTES ? alignment : DD_HEADER_BYTES; - if (base > SIZE_MAX / 2) return false; - size_t reserve = base * 2; - - if (reserve > SIZE_MAX - user_size) return false; - size_t bumped = user_size + reserve; - - /* Round up to a multiple of alignment so aligned_alloc callers - * (which require size %% alignment == 0) are satisfied. For - * alignment <= DD_HEADER_BYTES this is already a multiple of - * alignment (reserve = 32, a multiple of 1/2/4/8/16). */ - if (alignment > 1) { - size_t mask = alignment - 1; - if (bumped > SIZE_MAX - mask) return false; - bumped = (bumped + mask) & ~mask; - } - *out_size = bumped; - return true; + /* Shared with dd_allocation_freed_slow via x86_bumped_size so the + * alloc and free sides can never disagree on the formula. */ + return x86_bumped_size(user_size, alignment, out_size); #else (void)alignment; *out_size = user_size; @@ -150,7 +129,10 @@ dd_alloc_req_t dd_allocation_requested_slow(dd_tl_state_t *tl, size_t size, * or the USDT machinery) will see the guard set and pass through unsampled. */ tl->reentry_guard = true; + /* Save / restore errno: sample() reaches log(), which may set it. */ + int saved_errno = errno; uint64_t weight = sample(tl); + errno = saved_errno; if (weight == 0) { /* First-interval miss: no sample this time. Close the guard now since * dd_allocation_created_slow won't be called on the sampled path. */ @@ -177,4 +159,4 @@ dd_alloc_req_t dd_allocation_requested_slow(dd_tl_state_t *tl, size_t size, .weight = weight, }; return out; -} \ No newline at end of file +} diff --git a/libdd-profiling-heap-sampler/src/generated/bindings.rs b/libdd-profiling-heap-sampler/src/generated/bindings.rs index 289f1c4f72..f3bbea0c86 100644 --- a/libdd-profiling-heap-sampler/src/generated/bindings.rs +++ b/libdd-profiling-heap-sampler/src/generated/bindings.rs @@ -64,6 +64,10 @@ const _: () = { ["Offset of field: dd_alloc_req_t::weight"] [::std::mem::offset_of!(dd_alloc_req_t, weight) - 24usize]; }; +unsafe extern "C" { + #[link_name = "dd_alloc_req_is_sampled__extern"] + pub fn dd_alloc_req_is_sampled(req: dd_alloc_req_t) -> bool; +} unsafe extern "C" { pub fn dd_allocation_requested_slow( tl: *mut dd_tl_state_t, @@ -105,8 +109,8 @@ unsafe extern "C" { ) -> bool; } unsafe extern "C" { - #[link_name = "dd_sample_flag_check__extern"] - pub fn dd_sample_flag_check( + #[link_name = "dd_sample_flag_check_and_clear__extern"] + pub fn dd_sample_flag_check_and_clear( user: *mut ::std::os::raw::c_void, raw_out: *mut *mut ::std::os::raw::c_void, ) -> bool; diff --git a/libdd-profiling-heap-sampler/src/generated/dd_heap_sampler_static_wrappers.c b/libdd-profiling-heap-sampler/src/generated/dd_heap_sampler_static_wrappers.c index 7cf466a24c..fbb8b9a7b5 100644 --- a/libdd-profiling-heap-sampler/src/generated/dd_heap_sampler_static_wrappers.c +++ b/libdd-profiling-heap-sampler/src/generated/dd_heap_sampler_static_wrappers.c @@ -15,7 +15,8 @@ dd_tl_state_t * dd_tl_state_get__extern(void) { return dd_tl_state_get(); } dd_tl_state_t * dd_tl_state_get_or_init__extern(void) { return dd_tl_state_get_or_init(); } +bool dd_alloc_req_is_sampled__extern(dd_alloc_req_t req) { return dd_alloc_req_is_sampled(req); } dd_alloc_req_t dd_allocation_requested__extern(size_t size, size_t alignment) { return dd_allocation_requested(size, alignment); } void * dd_allocation_created__extern(void *raw, dd_alloc_req_t req) { return dd_allocation_created(raw, req); } -bool dd_sample_flag_check__extern(void *user, void **raw_out) { return dd_sample_flag_check(user, raw_out); } +bool dd_sample_flag_check_and_clear__extern(void *user, void **raw_out) { return dd_sample_flag_check_and_clear(user, raw_out); } dd_alloc_freed_t dd_allocation_freed__extern(void *ptr, size_t size, size_t alignment) { return dd_allocation_freed(ptr, size, alignment); } diff --git a/libdd-profiling-heap-sampler/src/lib.rs b/libdd-profiling-heap-sampler/src/lib.rs index d5380de1ce..fd21af1447 100644 --- a/libdd-profiling-heap-sampler/src/lib.rs +++ b/libdd-profiling-heap-sampler/src/lib.rs @@ -32,6 +32,11 @@ mod sys { #[cfg(target_os = "linux")] pub use sys::*; +/// Runtime ELF self-inspection asserting one USDT note per probe. Opt-in via +/// the `sanity-check` feature (pulls in the `elf`/`anyhow` deps). +#[cfg(all(target_os = "linux", feature = "sanity-check"))] +pub mod usdt_check; + /// Name of the environment variable that toggles heap sampling. pub const DD_HEAP_SAMPLING_ENABLED: &str = "DD_HEAP_SAMPLING_ENABLED"; @@ -49,6 +54,7 @@ pub const DD_HEAP_SAMPLING_ENABLED: &str = "DD_HEAP_SAMPLING_ENABLED"; /// The lookup uses `getenv` rather than [`std::env::var`] specifically so it /// performs no heap allocation so that our consumers don't have to worry about /// re-entrancy. +#[inline] pub fn heap_sampling_enabled() -> bool { use std::sync::OnceLock; static ENABLED: OnceLock = OnceLock::new(); @@ -263,7 +269,7 @@ mod tests { buf[header_idx + 8..header_idx + 16].copy_from_slice(&8u64.to_ne_bytes()); let mut raw = core::ptr::null_mut(); - let sampled = unsafe { dd_sample_flag_check(user_addr as *mut c_void, &mut raw) }; + let sampled = unsafe { dd_sample_flag_check_and_clear(user_addr as *mut c_void, &mut raw) }; assert!(!sampled); assert!(raw.is_null()); diff --git a/libdd-profiling-heap-sampler/src/probes.c b/libdd-profiling-heap-sampler/src/probes.c index bdc6362013..b4bc5612a6 100644 --- a/libdd-profiling-heap-sampler/src/probes.c +++ b/libdd-profiling-heap-sampler/src/probes.c @@ -19,10 +19,17 @@ #include +#include + +/* Save / restore errno: an attached USDT consumer may perturb it. */ void dd_probe_alloc(void *user, uint64_t size, uint64_t weight) { + int saved_errno = errno; USDT(ddheap, alloc, user, size, weight); + errno = saved_errno; } void dd_probe_free(void *ptr) { + int saved_errno = errno; USDT(ddheap, free, ptr); -} \ No newline at end of file + errno = saved_errno; +} diff --git a/libdd-profiling-heap-sampler/src/sample_flag.c b/libdd-profiling-heap-sampler/src/sample_flag.c index def9b5c2ed..54c813c401 100644 --- a/libdd-profiling-heap-sampler/src/sample_flag.c +++ b/libdd-profiling-heap-sampler/src/sample_flag.c @@ -29,21 +29,16 @@ bool dd_sample_flag_peek(void *user, void **raw_out, size_t *offset_out) { return false; } - void *header = (char *)user - DD_HEADER_BYTES; - uint64_t magic; - memcpy(&magic, header, sizeof(magic)); - if (magic != DD_MAGIC) { + x86_header_t hdr = x86_header_read(user); + if (hdr.magic != DD_MAGIC) { return false; } - - uint64_t offset; - memcpy(&offset, (char *)header + sizeof(magic), sizeof(offset)); - if (offset < DD_HEADER_BYTES || offset > 2 * DD_SAMPLE_ALIGNMENT_CAP) { + if (hdr.offset < DD_HEADER_BYTES || hdr.offset > 2 * DD_SAMPLE_ALIGNMENT_CAP) { return false; } - *raw_out = x86_raw_from_user(user, offset); - *offset_out = (size_t)offset; + *raw_out = x86_raw_from_user(user, hdr.offset); + *offset_out = (size_t)hdr.offset; return true; } @@ -74,7 +69,7 @@ void *dd_sample_flag_apply(void *raw, size_t alignment) { } bool dd_sample_flag_peek(void *user, void **raw_out, size_t *offset_out) { - if (!dd_sample_flag_check(user, raw_out)) { + if (!dd_sample_flag_check_and_clear(user, raw_out)) { return false; } *offset_out = 0; @@ -122,4 +117,4 @@ bool dd_sample_flag_thread_init(void) { #else # error "dd_sample_flag: unsupported architecture (expected __x86_64__ or __aarch64__)" -#endif \ No newline at end of file +#endif diff --git a/libdd-profiling-heap-sampler/src/tl_state.c b/libdd-profiling-heap-sampler/src/tl_state.c index 3bdc306c84..91552fcc09 100644 --- a/libdd-profiling-heap-sampler/src/tl_state.c +++ b/libdd-profiling-heap-sampler/src/tl_state.c @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -68,5 +69,8 @@ static void tl_state_populate(dd_tl_state_t *st) { void dd_tl_state_init(void) { if (dd_tl_state_storage.initialized) return; + /* Save / restore errno: first-touch init calls prctl() / clock_gettime(). */ + int saved_errno = errno; tl_state_populate(&dd_tl_state_storage); + errno = saved_errno; } diff --git a/libdd-profiling-heap-sampler/src/usdt_check.rs b/libdd-profiling-heap-sampler/src/usdt_check.rs new file mode 100644 index 0000000000..fbf35ad9da --- /dev/null +++ b/libdd-profiling-heap-sampler/src/usdt_check.rs @@ -0,0 +1,132 @@ +// Copyright 2026-Present Datadog, Inc. https://www.datadoghq.com/ +// SPDX-License-Identifier: Apache-2.0 + +//! Runtime ELF self-inspection for the ddheap USDT probes. Verifies that each +//! probe has exactly one `.note.stapsdt` entry, so an attached consumer +//! (bpftrace / eBPF) sees one probe per site rather than attaching twice. +//! +//! The probes are emitted as non-inline functions in a dedicated translation +//! unit (`probes.c`) precisely so each `USDT()` expansion produces a single +//! note; a regression (e.g. `static inline` + bindgen's `wrap_static_fns`, or +//! LTO inlining across TUs) could duplicate the entry. This check catches that. +//! +//! Call [`sanity_check`] from within a shared object or statically linked +//! executable, or point [`check_usdt_probes_in`] at a built artifact. +//! +//! Only available on Linux (USDT/SystemTap notes are Linux-only) and only when +//! the `sanity-check` feature is enabled. + +use anyhow::{bail, Context}; +use elf::{endian::AnyEndian, ElfBytes}; +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +/// USDT provider name emitted by `probes.c` (`USDT(ddheap, ...)`). +const PROVIDER: &str = "ddheap"; +/// Probes we expect, each exactly once. +const EXPECTED_PROBES: &[&str] = &["alloc", "free"]; + +/// As [`sanity_check`], but takes the object file as an argument. Useful for a +/// test setting where the test code is separate from the artifact to validate. +pub fn check_usdt_probes_in(path: &Path) -> anyhow::Result<()> { + let data = std::fs::read(path).with_context(|| format!("failed to read {}", path.display()))?; + let elf = ElfBytes::::minimal_parse(&data) + .with_context(|| format!("failed to parse ELF at {}", path.display()))?; + check_one_note_per_probe(&elf)?; + Ok(()) +} + +/// Check that the current running module carries exactly one `.note.stapsdt` +/// entry per ddheap probe. +pub fn sanity_check() -> anyhow::Result<()> { + check_usdt_probes_in(&own_elf_path()?) +} + +/// Locate the current running module (shared or not) via `/proc/self/maps`. +fn own_elf_path() -> anyhow::Result { + // We use the address of an arbitrary function of this module. + let addr = sanity_check as *const () as usize; + let maps = + std::fs::read_to_string("/proc/self/maps").context("failed to read /proc/self/maps")?; + for line in maps.lines() { + // Format: address perms offset dev inode [pathname] + // Skip the first 5 whitespace-delimited tokens then take the rest + // verbatim as the path, so pathnames containing spaces are preserved. + let mut rest = line; + for _ in 0..5 { + rest = rest.trim_start_matches(|c: char| c.is_ascii_whitespace()); + rest = rest.trim_start_matches(|c: char| !c.is_ascii_whitespace()); + } + let path = rest.trim_start_matches(|c: char| c.is_ascii_whitespace()); + + if !path.starts_with('/') { + continue; + } + + if let Some((start_str, end_str)) = line + .split_whitespace() + .next() + .and_then(|f| f.split_once('-')) + { + let start = usize::from_str_radix(start_str, 16).unwrap_or(0); + let end = usize::from_str_radix(end_str, 16).unwrap_or(0); + if addr >= start && addr < end { + return Ok(PathBuf::from(path)); + } + } + } + bail!("could not find our own object file in /proc/self/maps") +} + +/// Parse `.note.stapsdt` and assert each expected probe appears exactly once. +fn check_one_note_per_probe(elf: &ElfBytes<'_, AnyEndian>) -> anyhow::Result<()> { + let shdr = elf + .section_header_by_name(".note.stapsdt") + .context("failed to read section headers")? + .context("no .note.stapsdt section: USDT probes are missing from this object")?; + + let notes = elf + .section_data_as_notes(&shdr) + .context("failed to parse .note.stapsdt")?; + + // Pointer width for the stapsdt descriptor's three leading addresses. + let word = if elf.ehdr.class == elf::file::Class::ELF64 { + 8 + } else { + 4 + }; + + let mut counts: BTreeMap = BTreeMap::new(); + for note in notes { + // stapsdt notes are emitted with name "stapsdt"; skip anything else. + let elf::note::Note::Unknown(any) = note else { + continue; + }; + if any.name != "stapsdt" { + continue; + } + // Descriptor: 3 target-word addresses (location, base, semaphore), + // then NUL-terminated provider, probe, and arg-format strings. + let strings = any.desc.get(3 * word..).unwrap_or(&[]); + let mut parts = strings.split(|&b| b == 0); + let provider = parts.next().unwrap_or(&[]); + let probe = parts.next().unwrap_or(&[]); + if provider == PROVIDER.as_bytes() { + if let Ok(name) = std::str::from_utf8(probe) { + *counts.entry(name.to_string()).or_default() += 1; + } + } + } + + for &probe in EXPECTED_PROBES { + match counts.get(probe).copied().unwrap_or(0) { + 1 => {} + 0 => bail!("USDT probe '{PROVIDER}:{probe}' has no .note.stapsdt entry"), + n => bail!( + "USDT probe '{PROVIDER}:{probe}' has {n} .note.stapsdt entries, expected 1 \ + (duplicate entries make an attached consumer fire twice)" + ), + } + } + Ok(()) +}