Split symbolize::Sym::path member into dir and file

All our supported formats internally store source code information as a
directory path and a file name. Many clients may reasonably expect this
split and while we could always do it retroactively, it makes sense to
stay as close to the supported formats as possible. Down the line, that
could allow us to stop allocating new buffers altogether and just hand
out references, in a more zero-copy (and zero-alloc) fashion.
With this change we split the symbolize::Sym::path member into dir and
file attributes to represent this split in the library. Note that
currently either both the directory and file are present or neither is.
We could encode this constraint in the type system and it would simplify
client code somewhat, but we opted not to for symmetry with other
members and because it is unclear whether this constraint is truly
guaranteed to true unconditionally in the future.

Signed-off-by: Daniel Müller <deso@posteo.net>

CHANGELOG.md

@@ -3,12 +3,13 @@ Unreleased
 - Added support for automatic demangling of symbols, controlled by
   `demangle` feature (at compile time) and corresponding flag in
   `symbolize::Builder` (at runtime)
-- Renamed `symbolize::SymbolizedResult` to `Sym` and made it
-  non-exhaustive
-  - Renamed `Sym::symbol` to `name`
-  - Added `Sym::offset` member
-  - Changed `Sym::line` to be of type `u32` and `Sym::column` to `u16`
+- Renamed `symbolize::SymbolizedResult` to `Sym` and reworked it
+  - Made it non-exhaustive
+  - Renamed `symbol` member to `name`
+  - Added `offset` member
+  - Changed `line` member to be of type `u32` and `column` to `u16`
   - Made all source code location information optional
+  - Split `path` member into `dir` and `file`
 - Added additional end-to-end benchmarks
   - Added benchmark result summary to CI runs
 - Fixed spurious maps file path creation for low addresses as part of

cli/src/main.rs

@@ -2,6 +2,8 @@
 
 mod args;
 
+use std::path::PathBuf;
+
 use anyhow::Context;
 use anyhow::Result;
 
@@ -52,7 +54,12 @@ fn symbolize(symbolize: args::Symbolize) -> Result<()> {
                     name, addr, offset, ..
                 } = sym;
 
-                let src_loc = if let (Some(path), Some(line)) = (sym.path, sym.line) {
+                let path = match (sym.dir, sym.file) {
+                    (Some(dir), Some(file)) => Some(dir.join(file)),
+                    (dir, file) => dir.or_else(|| file.map(PathBuf::from)),
+                };
+
+                let src_loc = if let (Some(path), Some(line)) = (path, sym.line) {
                     if let Some(col) = sym.column {
                         format!(" {}:{line}:{col}", path.display())
                     } else {

examples/addr2ln.rs

@@ -1,4 +1,5 @@
 use std::env;
+use std::path::PathBuf;
 
 use anyhow::bail;
 use anyhow::Context as _;
@@ -46,7 +47,12 @@ fn main() -> Result<()> {
                     name, addr, offset, ..
                 } = sym;
 
-                let src_loc = if let (Some(path), Some(line)) = (sym.path, sym.line) {
+                let path = match (sym.dir, sym.file) {
+                    (Some(dir), Some(file)) => Some(dir.join(file)),
+                    (dir, file) => dir.or_else(|| file.map(PathBuf::from)),
+                };
+
+                let src_loc = if let (Some(path), Some(line)) = (path, sym.line) {
                     if let Some(col) = sym.column {
                         format!(" {}:{line}:{col}", path.display())
                     } else {

examples/addr2ln_pid.rs

@@ -1,4 +1,5 @@
 use std::env;
+use std::path::PathBuf;
 
 use anyhow::bail;
 use anyhow::Context as _;
@@ -49,7 +50,12 @@ print its symbol, the file name of the source, and the line number.",
                     name, addr, offset, ..
                 } = sym;
 
-                let src_loc = if let (Some(path), Some(line)) = (sym.path, sym.line) {
+                let path = match (sym.dir, sym.file) {
+                    (Some(dir), Some(file)) => Some(dir.join(file)),
+                    (dir, file) => dir.or_else(|| file.map(PathBuf::from)),
+                };
+
+                let src_loc = if let (Some(path), Some(line)) = (path, sym.line) {
                     if let Some(col) = sym.column {
                         format!(" {}:{line}:{col}", path.display())
                     } else {

examples/backtrace.rs

@@ -1,6 +1,7 @@
 use std::cmp::min;
 use std::mem::size_of;
 use std::mem::transmute;
+use std::path::PathBuf;
 use std::ptr;
 
 use blazesym::symbolize::Process;
@@ -41,7 +42,12 @@ fn symbolize_current_bt() {
                     name, addr, offset, ..
                 } = sym;
 
-                let src_loc = if let (Some(path), Some(line)) = (sym.path, sym.line) {
+                let path = match (sym.dir, sym.file) {
+                    (Some(dir), Some(file)) => Some(dir.join(file)),
+                    (dir, file) => dir.or_else(|| file.map(PathBuf::from)),
+                };
+
+                let src_loc = if let (Some(path), Some(line)) = (path, sym.line) {
                     if let Some(col) = sym.column {
                         format!(" {}:{line}:{col}", path.display())
                     } else {

include/blazesym.h

@@ -283,11 +283,17 @@ typedef struct blaze_sym {
    */
   size_t offset;
   /**
-   * The path of the source file defining the symbol.
+   * The directory in which the source file resides.
    *
    * This attribute is optional and may be NULL.
    */
-  const char *path;
+  const char *dir;
+  /**
+   * The file that defines the symbol.
+   *
+   * This attribute is optional and may be NULL.
+   */
+  const char *file;
   /**
    * The line number on which the symbol is located in the source
    * code.

src/c_api/symbolize.rs

@@ -187,10 +187,14 @@ pub struct blaze_sym {
     /// context (which may have been relocated and/or have layout randomizations
     /// applied).
     pub offset: usize,
-    /// The path of the source file defining the symbol.
+    /// The directory in which the source file resides.
     ///
     /// This attribute is optional and may be NULL.
-    pub path: *const c_char,
+    pub dir: *const c_char,
+    /// The file that defines the symbol.
+    ///
+    /// This attribute is optional and may be NULL.
+    pub file: *const c_char,
     /// The line number on which the symbol is located in the source
     /// code.
     pub line: u32,
@@ -316,12 +320,13 @@ unsafe fn convert_symbolizedresults_to_c(results: Vec<Vec<Sym>>) -> *const blaze
     // blaze_sym, and C strings of symbol and path.
     let strtab_size = results.iter().flatten().fold(0, |acc, result| {
         acc + result.name.len()
+            + 1
             + result
-                .path
+                .dir
                 .as_ref()
-                .map(|p| p.as_os_str().len())
+                .map(|p| p.as_os_str().len() + 1)
                 .unwrap_or(0)
-            + 2
+            + result.file.as_ref().map(|p| p.len() + 1).unwrap_or(0)
     });
     let all_csym_size = results.iter().flatten().count();
     let buf_size = strtab_size
@@ -371,17 +376,23 @@ unsafe fn convert_symbolizedresults_to_c(results: Vec<Vec<Sym>>) -> *const blaze
 
         for r in entry {
             let name_ptr = make_cstr(OsStr::new(&r.name));
-            let path_ptr = r
-                .path
+            let dir_ptr = r
+                .dir
+                .as_ref()
+                .map(|d| make_cstr(d.as_os_str()))
+                .unwrap_or_else(ptr::null_mut);
+            let file_ptr = r
+                .file
                 .as_ref()
-                .map(|p| make_cstr(p.as_os_str()))
+                .map(|f| make_cstr(f))
                 .unwrap_or_else(ptr::null_mut);
 
             let csym_ref = unsafe { &mut *csym_last };
             csym_ref.name = name_ptr;
             csym_ref.addr = r.addr;
             csym_ref.offset = r.offset;
-            csym_ref.path = path_ptr;
+            csym_ref.dir = dir_ptr;
+            csym_ref.file = file_ptr;
             csym_ref.line = r.line.unwrap_or(0);
             csym_ref.column = r.column.unwrap_or(0);
 
@@ -604,13 +615,14 @@ mod tests {
             name: ptr::null(),
             addr: 0x1337,
             offset: 24,
-            path: ptr::null(),
+            dir: ptr::null(),
+            file: ptr::null(),
             line: 42,
             column: 1,
         };
         assert_eq!(
             format!("{sym:?}"),
-            "blaze_sym { name: 0x0, addr: 4919, offset: 24, path: 0x0, line: 42, column: 1 }"
+            "blaze_sym { name: 0x0, addr: 4919, offset: 24, dir: 0x0, file: 0x0, line: 42, column: 1 }"
         );
 
         let entry = blaze_entry {

src/symbolize/mod.rs

@@ -9,6 +9,7 @@
 //! # use std::cmp::min;
 //! # use std::mem::size_of;
 //! # use std::mem::transmute;
+//! # use std::path::PathBuf;
 //! # use std::ptr;
 //! use blazesym::symbolize::Source;
 //! use blazesym::symbolize::Process;
@@ -46,7 +47,12 @@
 //!                 name, addr, offset, ..
 //!             } = sym;
 //!
-//!             let src_loc = if let (Some(path), Some(line)) = (sym.path, sym.line) {
+//!             let path = match (sym.dir, sym.file) {
+//!                 (Some(dir), Some(file)) => Some(dir.join(file)),
+//!                 (dir, file) => dir.or_else(|| file.map(PathBuf::from)),
+//!             };
+//!
+//!             let src_loc = if let (Some(path), Some(line)) = (path, sym.line) {
 //!                 if let Some(col) = sym.column {
 //!                     format!(" {}:{line}:{col}", path.display())
 //!                 } else {

src/symbolize/symbolizer.rs

@@ -1,4 +1,5 @@
 use std::ffi::OsStr;
+use std::ffi::OsString;
 use std::fmt::Debug;
 use std::path::Path;
 use std::path::PathBuf;
@@ -89,8 +90,10 @@ pub struct Sym {
     /// context (which may have been relocated and/or have layout randomizations
     /// applied).
     pub offset: usize,
-    /// The source path that defines the symbol.
-    pub path: Option<PathBuf>,
+    /// The directory in which the source file resides.
+    pub dir: Option<PathBuf>,
+    /// The file that defines the symbol.
+    pub file: Option<OsString>,
     /// The line number of the symbolized instruction in the source
     /// code.
     ///
@@ -235,7 +238,8 @@ impl Symbolizer {
                     name: self.maybe_demangle(name, lang),
                     addr: sym_addr,
                     offset: addr - sym_addr,
-                    path: Some(linfo.dir.join(linfo.file)),
+                    dir: Some(linfo.dir.to_path_buf()),
+                    file: Some(linfo.file.to_os_string()),
                     line: linfo.line,
                     column: linfo.column,
                     _non_exhaustive: (),
@@ -250,7 +254,8 @@ impl Symbolizer {
                     name: self.maybe_demangle(name, lang),
                     addr: sym_addr,
                     offset: addr - sym_addr,
-                    path: None,
+                    dir: None,
+                    file: None,
                     line: None,
                     column: None,
                     _non_exhaustive: (),

tests/blazesym.rs

@@ -2,6 +2,7 @@
 
 use std::env::current_exe;
 use std::ffi::CString;
+use std::ffi::OsStr;
 use std::fs::read as read_file;
 use std::io::Error;
 use std::os::unix::ffi::OsStringExt as _;
@@ -124,14 +125,15 @@ fn symbolize_elf_dwarf() {
         assert_eq!(result.offset, 0);
 
         if dwarf {
-            assert!(result
-                .path
-                .as_ref()
-                .unwrap()
-                .ends_with("test-stable-addresses.c"));
+            assert_ne!(result.dir, None);
+            assert_eq!(
+                result.file.as_deref(),
+                Some(OsStr::new("test-stable-addresses.c"))
+            );
             assert_eq!(result.line, Some(8));
         } else {
-            assert_eq!(result.path, None);
+            assert_eq!(result.dir, None);
+            assert_eq!(result.file, None);
             assert_eq!(result.line, None);
         }
 
@@ -156,14 +158,15 @@ fn symbolize_elf_dwarf() {
             assert_eq!(result.offset, offset);
 
             if dwarf {
-                assert!(result
-                    .path
-                    .as_ref()
-                    .unwrap()
-                    .ends_with("test-stable-addresses.c"));
+                assert_ne!(result.dir, None);
+                assert_eq!(
+                    result.file.as_deref(),
+                    Some(OsStr::new("test-stable-addresses.c"))
+                );
                 assert!(result.line.is_some());
             } else {
-                assert_eq!(result.path, None);
+                assert_eq!(result.dir, None);
+                assert_eq!(result.file, None);
                 assert_eq!(result.line, None);
             }
         }

tests/c_api.rs

@@ -88,14 +88,16 @@ fn symbolize_elf_dwarf() {
         assert_eq!(sym.offset, 0);
 
         if dwarf {
-            assert!(!sym.path.is_null());
-            assert!(unsafe { CStr::from_ptr(sym.path) }
-                .to_str()
-                .unwrap()
-                .ends_with("test-stable-addresses.c"));
+            assert!(!sym.dir.is_null());
+            assert!(!sym.file.is_null());
+            assert_eq!(
+                unsafe { CStr::from_ptr(sym.file) },
+                CStr::from_bytes_with_nul(b"test-stable-addresses.c\0").unwrap()
+            );
             assert_eq!(sym.line, 8);
         } else {
-            assert!(sym.path.is_null());
+            assert!(sym.dir.is_null());
+            assert!(sym.file.is_null());
             assert_eq!(sym.line, 0);
         }