From dd0747a03fe1072e004a3ee63c0e0782f9a3c4be Mon Sep 17 00:00:00 2001 From: not-matthias Date: Tue, 3 Nov 2026 15:45:20 +0100 Subject: [PATCH 1/2] fix: resolve existing workspace Clippy warnings Remove redundant formatting and unused imports, provide Default implementations, and name specialized iterator conversions explicitly. Use Infallible for the Linux profiler, which never returns Err. --- samply-api/src/asm/mod.rs | 2 +- samply/src/lib.rs | 16 ++++++---------- samply/src/linux/profiler.rs | 3 ++- samply/src/linux_shared/converter.rs | 2 +- samply/src/mac/thread_profiler.rs | 1 - samply/src/shared/jit_category_manager.rs | 6 ++++++ samply/src/shared/lib_mappings.rs | 9 ++++++--- samply/src/shared/recycling.rs | 6 ++++++ samply/src/shared/symbol_precog.rs | 2 +- tools/dump_table/src/lib.rs | 4 ++-- tools/query_api/src/lib.rs | 4 ++-- 11 files changed, 33 insertions(+), 22 deletions(-) diff --git a/samply-api/src/asm/mod.rs b/samply-api/src/asm/mod.rs index b5b487212..ec757f39b 100644 --- a/samply-api/src/asm/mod.rs +++ b/samply-api/src/asm/mod.rs @@ -526,7 +526,7 @@ where let s = remaining_bytes .iter() .take(A::ADJUST_BY_AFTER_ERROR) - .map(|b| format!("{b:#02x}")) + .map(|b| format!("{b:#x}")) .collect::>() .join(", "); let s2 = remaining_bytes diff --git a/samply/src/lib.rs b/samply/src/lib.rs index 22748403b..0f716c022 100644 --- a/samply/src/lib.rs +++ b/samply/src/lib.rs @@ -159,17 +159,13 @@ pub fn do_record_action(record_args: cli::RecordArgs) { // A process killed by a signal has no exit code; report it as 128 + signal, // following the shell convention. + #[cfg(unix)] let exit_code = exit_status.code().unwrap_or_else(|| { - #[cfg(unix)] - { - use std::os::unix::process::ExitStatusExt; - exit_status.signal().map_or(1, |signal| 128 + signal) - } - #[cfg(not(unix))] - { - 1 - } + use std::os::unix::process::ExitStatusExt; + exit_status.signal().map_or(1, |signal| 128 + signal) }); + #[cfg(not(unix))] + let exit_code = exit_status.code().unwrap_or(1); std::process::exit(exit_code); } @@ -256,7 +252,7 @@ pub fn run_server_serving_profile( let precog_path = profile_path.with_extension("syms.json"); if let Some(precog_info) = shared::symbol_precog::PrecogSymbolInfo::try_load(&precog_path) { - for symbol_map in precog_info.into_iter() { + for symbol_map in precog_info.into_symbol_maps() { let lib_info = symbol_map.library_info(); symbol_manager.add_known_library_symbols(lib_info, Arc::new(symbol_map)); } diff --git a/samply/src/linux/profiler.rs b/samply/src/linux/profiler.rs index bf5cdc662..a2b7bedb8 100644 --- a/samply/src/linux/profiler.rs +++ b/samply/src/linux/profiler.rs @@ -1,5 +1,6 @@ use std::cell::RefCell; use std::collections::{HashMap, HashSet}; +use std::convert::Infallible; use std::ops::Deref; use std::os::unix::process::ExitStatusExt; use std::path::Path; @@ -39,7 +40,7 @@ pub fn run( recording_mode: RecordingMode, recording_props: RecordingProps, profile_creation_props: ProfileCreationProps, -) -> Result<(Profile, ExitStatus), ()> { +) -> Result<(Profile, ExitStatus), Infallible> { let process_launch_props = match recording_mode { RecordingMode::All => { // TODO: Implement, by sudo launching a helper process which opens cpu-wide perf events diff --git a/samply/src/linux_shared/converter.rs b/samply/src/linux_shared/converter.rs index cd142ce6c..bd46c2d13 100644 --- a/samply/src/linux_shared/converter.rs +++ b/samply/src/linux_shared/converter.rs @@ -698,7 +698,7 @@ where fold_recursive_prefix: bool, call_chain_return_addresses_are_preadjusted: bool, ) { - stack.truncate(0); + stack.clear(); // Parse e.callchain into kernel frames and user FP frames. let mut callchain_buf = Vec::new(); diff --git a/samply/src/mac/thread_profiler.rs b/samply/src/mac/thread_profiler.rs index 5e32f7aee..4cb1755a9 100644 --- a/samply/src/mac/thread_profiler.rs +++ b/samply/src/mac/thread_profiler.rs @@ -14,7 +14,6 @@ use super::thread_info::{ thread_info_t, time_value, THREAD_BASIC_INFO, THREAD_BASIC_INFO_COUNT, THREAD_EXTENDED_INFO, THREAD_EXTENDED_INFO_COUNT, THREAD_IDENTIFIER_INFO, THREAD_IDENTIFIER_INFO_COUNT, }; -use crate::mac::time; use crate::shared::recycling::ThreadRecycler; use crate::shared::types::{StackFrame, StackMode}; use crate::shared::unresolved_samples::{UnresolvedSamples, UnresolvedStacks}; diff --git a/samply/src/shared/jit_category_manager.rs b/samply/src/shared/jit_category_manager.rs index 560e4e149..ab14d9485 100644 --- a/samply/src/shared/jit_category_manager.rs +++ b/samply/src/shared/jit_category_manager.rs @@ -27,6 +27,12 @@ pub struct JitCategoryManager { generic_jit_category: LazilyCreatedCategory, } +impl Default for JitCategoryManager { + fn default() -> Self { + Self::new() + } +} + impl JitCategoryManager { /// (prefix, name, color, is_js) const CATEGORIES: &'static [(&'static str, Category<'static>, bool)] = &[ diff --git a/samply/src/shared/lib_mappings.rs b/samply/src/shared/lib_mappings.rs index be98c970f..0c4dfdac7 100644 --- a/samply/src/shared/lib_mappings.rs +++ b/samply/src/shared/lib_mappings.rs @@ -87,7 +87,10 @@ pub struct LibMappingsHierarchy { impl LibMappingsHierarchy { pub fn new(regular_lib_mappings_ops: LibMappingOpQueue) -> Self { Self { - regular_libs: (LibMappings::default(), regular_lib_mappings_ops.into_iter()), + regular_libs: ( + LibMappings::default(), + regular_lib_mappings_ops.into_queue_iter(), + ), jitdumps: Vec::new(), perf_map: None, } @@ -95,7 +98,7 @@ impl LibMappingsHierarchy { pub fn add_jitdump_lib_mappings_ops(&mut self, lib_mappings_ops: LibMappingOpQueue) { self.jitdumps - .push((LibMappings::default(), lib_mappings_ops.into_iter())); + .push((LibMappings::default(), lib_mappings_ops.into_queue_iter())); } pub fn add_perf_map_mappings(&mut self, mappings: LibMappings) { @@ -143,7 +146,7 @@ impl LibMappingOpQueue { self.0.is_empty() } - pub fn into_iter(self) -> LibMappingOpQueueIter { + pub fn into_queue_iter(self) -> LibMappingOpQueueIter { LibMappingOpQueueIter(self.0.into_iter().peekable()) } } diff --git a/samply/src/shared/recycling.rs b/samply/src/shared/recycling.rs index fc88c1b0d..32e7672df 100644 --- a/samply/src/shared/recycling.rs +++ b/samply/src/shared/recycling.rs @@ -38,6 +38,12 @@ pub type ThreadRecycler = RecyclerByName<(ThreadHandle, StringHandle)>; pub struct RecyclerByName(FastHashMap>>); +impl Default for RecyclerByName { + fn default() -> Self { + Self::new() + } +} + impl RecyclerByName { pub fn new() -> Self { Self(FastHashMap::default()) diff --git a/samply/src/shared/symbol_precog.rs b/samply/src/shared/symbol_precog.rs index 0e98e0a09..81103b68f 100644 --- a/samply/src/shared/symbol_precog.rs +++ b/samply/src/shared/symbol_precog.rs @@ -237,7 +237,7 @@ impl PrecogSymbolInfo { serde_json::from_reader(reader).expect("failed to parse sidecar syms.json") } - pub fn into_iter(self) -> impl Iterator { + pub fn into_symbol_maps(self) -> impl Iterator { let Self { data, string_table } = self; let string_table = Arc::new(string_table); data.into_iter() diff --git a/tools/dump_table/src/lib.rs b/tools/dump_table/src/lib.rs index f54f8abce..9258f067b 100644 --- a/tools/dump_table/src/lib.rs +++ b/tools/dump_table/src/lib.rs @@ -219,13 +219,13 @@ impl FileAndPathHelper for Helper { let redirected_path = self.symbol_directory.join(filename); if std::fs::metadata(&redirected_path).is_ok() { // redirected_path exists! - eprintln!("Redirecting {:?} to {:?}", &path, &redirected_path); + eprintln!("Redirecting {:?} to {:?}", path, redirected_path); path = redirected_path; } } } - eprintln!("Reading file {:?}", &path); + eprintln!("Reading file {:?}", path); let file = File::open(&path)?; let mmap = unsafe { memmap2::MmapOptions::new().map(&file)? }; Ok(mmap_to_file_contents(mmap)) diff --git a/tools/query_api/src/lib.rs b/tools/query_api/src/lib.rs index d9b9c3e9c..2eb08dc2c 100644 --- a/tools/query_api/src/lib.rs +++ b/tools/query_api/src/lib.rs @@ -121,13 +121,13 @@ impl FileAndPathHelper for Helper { let redirected_path = self.symbol_directory.join(filename); if std::fs::metadata(&redirected_path).is_ok() { // redirected_path exists! - eprintln!("Redirecting {:?} to {:?}", &path, &redirected_path); + eprintln!("Redirecting {:?} to {:?}", path, redirected_path); path = redirected_path; } } } - eprintln!("Reading file {:?}", &path); + eprintln!("Reading file {:?}", path); let file = File::open(&path)?; Ok(unsafe { memmap2::MmapOptions::new().map(&file)? }) }) From 37b48c5d5dfdb24d8db1bdf46ffec57dbedbf154 Mon Sep 17 00:00:00 2001 From: not-matthias Date: Tue, 3 Nov 2026 15:45:35 +0100 Subject: [PATCH 2/2] feat(linux): support frame-pointer and configurable DWARF unwinding Add --unwinding-mode fp and dwarf, while preserving the default DWARF stack size. Configure perf callchains, user registers, and stack copying for the selected mode. Document frame-pointer requirements and cover CLI validation. --- README.md | 10 ++++ samply/src/cli.rs | 97 ++++++++++++++++++++++++++++++++- samply/src/linux/perf_event.rs | 10 +--- samply/src/linux/profiler.rs | 33 +++++++++-- samply/src/shared/prop_types.rs | 12 ++++ 5 files changed, 148 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 50c856a33..ea0f26931 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,16 @@ On Linux, samply uses perf events. You can grant temporary access by running: echo '-1' | sudo tee /proc/sys/kernel/perf_event_paranoid ``` +On Linux, use `--unwinding-mode fp` to record kernel frame-pointer callchains without copying user stack bytes or registers: + +```sh +samply record --unwinding-mode fp ./my-application my-arguments +``` + +The default, `--unwinding-mode dwarf`, keeps stack/register capture and framehop unwinding with the kernel frame-pointer callchain as a fallback. + +Frame-pointer recording requires the application and its libraries to preserve frame pointers. For Rust, build with `RUSTFLAGS="-C force-frame-pointers=yes"`; for C/C++, use `-fno-omit-frame-pointer -mno-omit-leaf-frame-pointer`. Missing frame pointers can truncate the stack. Kernel callchain support depends on the target architecture and kernel, and callchain depth is bounded by `/proc/sys/kernel/perf_event_max_stack`. + On Windows, you can use `samply record -a` to record all processes. You'll usually also want to use some symbol servers, most importantly the Microsoft Symbol Server so that you can see symbols for Windows libraries. Here's a command which supports symbols for Windows, Firefox and Chrome: ``` diff --git a/samply/src/cli.rs b/samply/src/cli.rs index 456e30394..e6a9dc999 100644 --- a/samply/src/cli.rs +++ b/samply/src/cli.rs @@ -10,7 +10,7 @@ use super::server::{PortSelection, ServerProps}; use super::shared::included_processes::IncludedProcesses; use super::shared::prop_types::{ CoreClrProfileProps, ImportProps, ProcessLaunchProps, ProfileCreationProps, RecordingMode, - RecordingProps, SymbolProps, + RecordingProps, SymbolProps, UnwindingMode, }; #[derive(Debug, Parser)] @@ -133,6 +133,14 @@ pub struct RecordArgs { #[arg(short, long, default_value = "1000")] pub rate: f64, + /// Stack unwinding on Linux: `fp`, `dwarf`, or `dwarf,`. + /// `fp` requires binaries compiled with frame pointers. + /// `dwarf` copies 32000 bytes of user stack per sample by default. + /// An explicit size must be a non-zero multiple of 8 below 65536. + #[cfg(target_os = "linux")] + #[arg(long, default_value = "dwarf", value_parser = parse_unwinding_mode)] + pub unwinding_mode: UnwindingMode, + /// Limit the recorded time to the specified number of seconds #[arg(short, long)] pub duration: Option, @@ -456,6 +464,10 @@ impl RecordArgs { output_file: self.output.clone(), time_limit, interval, + #[cfg(target_os = "linux")] + unwinding_mode: self.unwinding_mode, + #[cfg(not(target_os = "linux"))] + unwinding_mode: UnwindingMode::Dwarf(None), perf_events, gfx: self.gfx, browsers: self.browsers, @@ -614,6 +626,34 @@ impl SymbolArgs { } } +#[cfg(target_os = "linux")] +fn parse_unwinding_mode(value: &str) -> Result { + const FRAME_POINTER_UNWINDING: &str = "fp"; + const DWARF_UNWINDING: &str = "dwarf"; + + let (mode, stack_size) = match value.split_once(',') { + Some((mode, size)) => { + let size: u32 = size + .parse() + .map_err(|e| format!("Invalid stack size `{size}`: {e}"))?; + // The kernel requires sample_stack_user to be 8-byte aligned and fit in a u16. + if size == 0 || size % 8 != 0 || size > u32::from(u16::MAX) { + return Err("must be a non-zero multiple of 8 below 65536".into()); + } + (mode, Some(size)) + } + None => (value, None), + }; + match (mode, stack_size) { + (DWARF_UNWINDING, stack_size) => Ok(UnwindingMode::Dwarf(stack_size)), + (FRAME_POINTER_UNWINDING, None) => Ok(UnwindingMode::Fp), + (FRAME_POINTER_UNWINDING, Some(_)) => { + Err("A stack size only applies to `dwarf` unwinding".into()) + } + _ => Err("Expected `fp`, `dwarf`, or `dwarf,`".into()), + } +} + #[cfg(test)] mod test { use super::*; @@ -648,4 +688,59 @@ mod test { let opt_res = Opt::try_parse_from(["samply", "record", "-p", "1234", "rustup"]); assert!(opt_res.is_err()); } + + #[cfg(target_os = "linux")] + #[test] + fn record_fp_unwinding_with_launched_command() { + let opt = Opt::try_parse_from([ + "samply", + "record", + "--unwinding-mode", + "fp", + "rustup", + "--version", + ]) + .unwrap(); + let Action::Record(args) = opt.action else { + panic!("expected record action"); + }; + assert_eq!(args.recording_props().unwinding_mode, UnwindingMode::Fp); + } + + #[cfg(target_os = "linux")] + #[test] + fn record_unwinding_stack_size() { + let parse = |mode: &str| { + Opt::try_parse_from([ + "samply", + "record", + "--unwinding-mode", + mode, + "--pid", + "1234", + ]) + }; + for (mode, expected) in [ + ("dwarf", UnwindingMode::Dwarf(None)), + ("dwarf,8", UnwindingMode::Dwarf(Some(8))), + ("dwarf,65528", UnwindingMode::Dwarf(Some(65528))), + ] { + let Action::Record(args) = parse(mode).unwrap().action else { + panic!("expected record action"); + }; + assert_eq!(args.recording_props().unwinding_mode, expected); + } + for invalid in [ + "dwarf,0", + "dwarf,1001", + "dwarf,65536", + "dwarf,", + "dwarf,8,16", + "fp,8", + "unsupported", + ] { + let error = parse(invalid).unwrap_err(); + assert_eq!(error.kind(), clap::error::ErrorKind::ValueValidation); + } + } } diff --git a/samply/src/linux/perf_event.rs b/samply/src/linux/perf_event.rs index a34cf4bad..4fa7e4366 100644 --- a/samply/src/linux/perf_event.rs +++ b/samply/src/linux/perf_event.rs @@ -505,13 +505,9 @@ impl PerfBuilder { attr.read_format = PERF_FORMAT_GROUP | PERF_FORMAT_ID; } - // Always request callchain so the kernel walks frame pointers at sample - // time. The user portion serves as a fallback when DWARF unwinding - // truncates (e.g. due to the 32 KB captured stack window). + // FP-only recording uses the callchain directly; DWARF recording uses + // its user portion as a fallback when the captured stack is too short. attr.sample_type |= PERF_SAMPLE_CALLCHAIN; - if exclude_kernel { - attr.flags |= PERF_ATTR_FLAG_EXCLUDE_CALLCHAIN_KERNEL; - } attr.sample_regs_user = reg_mask; attr.sample_stack_user = stack_size; @@ -533,7 +529,7 @@ impl PerfBuilder { } if exclude_kernel { - attr.flags |= PERF_ATTR_FLAG_EXCLUDE_KERNEL; + attr.flags |= PERF_ATTR_FLAG_EXCLUDE_KERNEL | PERF_ATTR_FLAG_EXCLUDE_CALLCHAIN_KERNEL; } if inherit { diff --git a/samply/src/linux/profiler.rs b/samply/src/linux/profiler.rs index a2b7bedb8..e4381030c 100644 --- a/samply/src/linux/profiler.rs +++ b/samply/src/linux/profiler.rs @@ -27,7 +27,8 @@ use crate::linux_shared::{ }; use crate::shared::ctrl_c::CtrlC; use crate::shared::prop_types::{ - ProcessLaunchProps, ProfileCreationProps, RecordingMode, RecordingProps, + ProcessLaunchProps, ProfileCreationProps, RecordingMode, RecordingProps, UnwindingMode, + DEFAULT_UNWINDING_STACK_SIZE, }; #[cfg(target_arch = "x86_64")] @@ -94,6 +95,7 @@ pub fn run( // Launch the observer thread. This thread will manage the perf events. let interval = recording_props.interval; + let unwinding_mode = recording_props.unwinding_mode; let time_limit = recording_props.time_limit; // Resolve the extra event names before launching anything, so that a // configuration error aborts immediately. @@ -118,7 +120,14 @@ pub fn run( }; // Create the perf events, setting ENABLE_ON_EXEC. - let perf_group = init_profiler(interval, pid, attach_mode, extra_events, &mut converter); + let perf_group = init_profiler( + interval, + unwinding_mode, + pid, + attach_mode, + extra_events, + &mut converter, + ); // Tell the main thread to tell the child process to begin executing. profile_another_pid_reply_sender.send(true).unwrap(); @@ -282,8 +291,14 @@ fn start_profiling_pid( else { panic!("The first message should be a StartProfilingAnotherProcess") }; - let perf_group = - init_profiler(interval, pid, attach_mode, extra_events, &mut converter); + let perf_group = init_profiler( + interval, + recording_props.unwinding_mode, + pid, + attach_mode, + extra_events, + &mut converter, + ); // Tell the main thread that we are now executing. profile_another_pid_reply_sender.send(true).unwrap(); @@ -404,6 +419,7 @@ fn resolve_extra_events(specs: &[String]) -> Vec { fn init_profiler( interval: Duration, + unwinding_mode: UnwindingMode, pid: u32, attach_mode: AttachMode, extra_events: Vec, @@ -418,8 +434,13 @@ fn init_profiler( }; let frequency = (1_000_000_000 / interval_nanos) as u32; - let stack_size = 32000; - let regs_mask = ConvertRegsNative::regs_mask(); + let (stack_size, regs_mask) = match unwinding_mode { + UnwindingMode::Dwarf(stack_size) => ( + stack_size.unwrap_or(DEFAULT_UNWINDING_STACK_SIZE), + ConvertRegsNative::regs_mask(), + ), + UnwindingMode::Fp => (0, 0), + }; let mut perf = PerfGroup::open( pid, diff --git a/samply/src/shared/prop_types.rs b/samply/src/shared/prop_types.rs index 4b901841e..50ea3d896 100644 --- a/samply/src/shared/prop_types.rs +++ b/samply/src/shared/prop_types.rs @@ -25,6 +25,16 @@ impl CoreClrProfileProps { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum UnwindingMode { + Fp, + Dwarf(Option), +} + +/// Default bytes of user stack copied per sample for DWARF unwinding. +#[cfg(any(target_os = "linux", target_os = "android"))] +pub const DEFAULT_UNWINDING_STACK_SIZE: u32 = 32_000; + /// Properties which are meaningful both for recording a fresh process /// as well as for recording an existing process. #[derive(Debug, Clone)] @@ -33,6 +43,8 @@ pub struct RecordingProps { pub output_file: PathBuf, pub time_limit: Option, pub interval: Duration, + #[allow(dead_code)] // Only used on Linux and Android + pub unwinding_mode: UnwindingMode, /// Names of extra perf events to record alongside the main sampling /// event. Only used on Linux. #[allow(dead_code)]