Skip to content

Commit

Permalink
Make debug_syms a per-request attribute
Browse files Browse the repository at this point in the history
The Symbolizer currently treats the debug_syms setting as immutable: it
is set once for the instance and cannot be changed. The Inspector, on
the other hand, has it be a per-request attribute.
The latter is obviously more flexible, as the format can trivially be
expressed with it. Conceptually, it does not make sense to have this
setting apply to the Symbolizer instance and, hence, to all sources.
That is because there simply is no concept of debug symbols for anything
but ELF (which happens to be able to contain DWARF).
To make this explicit, move this setting to the individual sources that
it does affect. That is quite a bit more verbose, but it is the better
and more flexible design. If it becomes a problem moving forward, we
could consider introducing a `DwarfOpts` setting or similar.
Note that the "code info" setting is somewhat similar, but different
enough to have a different reasoning applied: it is a mere property of
the format whether it contains line and source code information, but
conceptually each symbolization source could support it. As such, having
it be a property of the Symbolizer makes a bit more sense. It's still
limiting in the sense that it can't be flipped between requests, and so
ultimately we may move it into the source representations as well, but
for now we don't go down that route.

Closes: #349

Signed-off-by: Daniel Müller <[email protected]>
  • Loading branch information
d-e-s-o committed Dec 4, 2023
1 parent 9c37ce6 commit 7c53f91
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 70 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Unreleased
- Adjusted various symbolization code paths to stop heap-allocating
- Adjusted normalization logic to honor executable and readable proc maps
entries
- Changed `debug_syms` to be a symbolization source property instead of a
`symbolize::Symbolizer` attribute
- Renamed `inspect::Elf::debug_info` to `debug_syms`
- Handled potential numeric overflow in Gsym inlined function parser more
gracefully
Expand Down
9 changes: 4 additions & 5 deletions benches/symbolize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,11 @@ fn symbolize_elf() {
let elf_vmlinux = Path::new(&env!("CARGO_MANIFEST_DIR"))
.join("data")
.join("vmlinux-5.17.12-100.fc34.x86_64.elf");
let src = Source::Elf(Elf::new(elf_vmlinux));
let symbolizer = Symbolizer::builder()
.enable_debug_syms(false)
.enable_code_info(false)
.build();
let mut elf = Elf::new(elf_vmlinux);
elf.debug_syms = false;
let src = Source::Elf(elf);

let symbolizer = Symbolizer::builder().enable_code_info(false).build();
let result = symbolizer
.symbolize_single(
black_box(&src),
Expand Down
19 changes: 15 additions & 4 deletions capi/include/blazesym.h
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,6 @@ typedef struct blaze_symbolizer blaze_symbolizer;
* Options for configuring `blaze_symbolizer` objects.
*/
typedef struct blaze_symbolizer_opts {
/**
* Whether to enable usage of debug symbols.
*/
bool debug_syms;
/**
* Whether to attempt to gather source code location information.
*
Expand Down Expand Up @@ -371,6 +367,11 @@ typedef struct blaze_symbolize_src_process {
* files.
*/
uint32_t pid;
/**
* Whether or not to consult debug symbols to satisfy the request
* (if present).
*/
bool debug_syms;
} blaze_symbolize_src_process;

/**
Expand Down Expand Up @@ -398,6 +399,11 @@ typedef struct blaze_symbolize_src_kernel {
* `"/usr/lib/debug/boot/"`.
*/
const char *kernel_image;
/**
* Whether or not to consult debug symbols from `kernel_image`
* to satisfy the request (if present).
*/
bool debug_syms;
} blaze_symbolize_src_kernel;

/**
Expand All @@ -416,6 +422,11 @@ typedef struct blaze_symbolize_src_elf {
* libc.
*/
const char *path;
/**
* Whether or not to consult debug symbols to satisfy the request
* (if present).
*/
bool debug_syms;
} blaze_symbolize_src_elf;

/**
Expand Down
57 changes: 40 additions & 17 deletions capi/src/symbolize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,17 @@ pub struct blaze_symbolize_src_elf {
/// passing "/lib/libc.so.xxx" will load symbols and debug information from
/// libc.
pub path: *const c_char,
/// Whether or not to consult debug symbols to satisfy the request
/// (if present).
pub debug_syms: bool,
}

impl From<&blaze_symbolize_src_elf> for Elf {
fn from(elf: &blaze_symbolize_src_elf) -> Self {
let blaze_symbolize_src_elf { path } = elf;
let blaze_symbolize_src_elf { path, debug_syms } = elf;
Self {
path: unsafe { from_cstr(*path) },
debug_syms: *debug_syms,
_non_exhaustive: (),
}
}
Expand Down Expand Up @@ -76,17 +80,22 @@ pub struct blaze_symbolize_src_kernel {
/// kernel image of the running kernel in `"/boot/"` or
/// `"/usr/lib/debug/boot/"`.
pub kernel_image: *const c_char,
/// Whether or not to consult debug symbols from `kernel_image`
/// to satisfy the request (if present).
pub debug_syms: bool,
}

impl From<&blaze_symbolize_src_kernel> for Kernel {
fn from(kernel: &blaze_symbolize_src_kernel) -> Self {
let blaze_symbolize_src_kernel {
kallsyms,
kernel_image,
debug_syms,
} = kernel;
Self {
kallsyms: (!kallsyms.is_null()).then(|| unsafe { from_cstr(*kallsyms) }),
kernel_image: (!kernel_image.is_null()).then(|| unsafe { from_cstr(*kernel_image) }),
debug_syms: *debug_syms,
_non_exhaustive: (),
}
}
Expand All @@ -105,13 +114,17 @@ pub struct blaze_symbolize_src_process {
/// blazesym will parse `/proc/<pid>/maps` and load all the object
/// files.
pub pid: u32,
/// Whether or not to consult debug symbols to satisfy the request
/// (if present).
pub debug_syms: bool,
}

impl From<&blaze_symbolize_src_process> for Process {
fn from(process: &blaze_symbolize_src_process) -> Self {
let blaze_symbolize_src_process { pid } = process;
let blaze_symbolize_src_process { pid, debug_syms } = process;
Self {
pid: (*pid).into(),
debug_syms: *debug_syms,
_non_exhaustive: (),
}
}
Expand Down Expand Up @@ -264,8 +277,6 @@ unsafe fn from_cstr(cstr: *const c_char) -> PathBuf {
#[repr(C)]
#[derive(Debug)]
pub struct blaze_symbolizer_opts {
/// Whether to enable usage of debug symbols.
pub debug_syms: bool,
/// Whether to attempt to gather source code location information.
///
/// This setting implies `debug_syms` (and forces it to `true`).
Expand Down Expand Up @@ -300,14 +311,12 @@ pub unsafe extern "C" fn blaze_symbolizer_new_opts(
// SAFETY: The caller ensures that the pointer is valid.
let opts = unsafe { &*opts };
let blaze_symbolizer_opts {
debug_syms,
code_info,
inlined_fns,
demangle,
} = opts;

let symbolizer = Symbolizer::builder()
.enable_debug_syms(*debug_syms)
.enable_code_info(*code_info)
.enable_inlined_fns(*inlined_fns)
.enable_demangling(*demangle)
Expand Down Expand Up @@ -666,22 +675,32 @@ mod tests {
/// Exercise the `Debug` representation of various types.
#[test]
fn debug_repr() {
let elf = blaze_symbolize_src_elf { path: ptr::null() };
assert_eq!(format!("{elf:?}"), "blaze_symbolize_src_elf { path: 0x0 }");
let elf = blaze_symbolize_src_elf {
path: ptr::null(),
debug_syms: false,
};
assert_eq!(
format!("{elf:?}"),
"blaze_symbolize_src_elf { path: 0x0, debug_syms: false }"
);

let kernel = blaze_symbolize_src_kernel {
kallsyms: ptr::null(),
kernel_image: ptr::null(),
debug_syms: true,
};
assert_eq!(
format!("{kernel:?}"),
"blaze_symbolize_src_kernel { kallsyms: 0x0, kernel_image: 0x0 }"
"blaze_symbolize_src_kernel { kallsyms: 0x0, kernel_image: 0x0, debug_syms: true }"
);

let process = blaze_symbolize_src_process { pid: 1337 };
let process = blaze_symbolize_src_process {
pid: 1337,
debug_syms: true,
};
assert_eq!(
format!("{process:?}"),
"blaze_symbolize_src_process { pid: 1337 }"
"blaze_symbolize_src_process { pid: 1337, debug_syms: true }"
);

let gsym_data = blaze_symbolize_src_gsym_data {
Expand Down Expand Up @@ -735,14 +754,13 @@ mod tests {
assert_eq!(format!("{result:?}"), "blaze_result { cnt: 0, syms: [] }");

let opts = blaze_symbolizer_opts {
debug_syms: true,
code_info: false,
inlined_fns: false,
demangle: true,
};
assert_eq!(
format!("{opts:?}"),
"blaze_symbolizer_opts { debug_syms: true, code_info: false, inlined_fns: false, demangle: true }"
"blaze_symbolizer_opts { code_info: false, inlined_fns: false, demangle: true }"
);
}

Expand All @@ -753,6 +771,7 @@ mod tests {
let kernel = blaze_symbolize_src_kernel {
kallsyms: ptr::null(),
kernel_image: ptr::null(),
debug_syms: true,
};
let kernel = Kernel::from(&kernel);
assert_eq!(kernel.kallsyms, None);
Expand All @@ -761,6 +780,7 @@ mod tests {
let kernel = blaze_symbolize_src_kernel {
kallsyms: b"/proc/kallsyms\0" as *const _ as *const c_char,
kernel_image: b"/boot/image\0" as *const _ as *const c_char,
debug_syms: false,
};
let kernel = Kernel::from(&kernel);
assert_eq!(kernel.kallsyms, Some(PathBuf::from("/proc/kallsyms")));
Expand Down Expand Up @@ -897,7 +917,6 @@ mod tests {
#[test]
fn symbolizer_creation_with_opts() {
let opts = blaze_symbolizer_opts {
debug_syms: true,
code_info: false,
inlined_fns: false,
demangle: true,
Expand Down Expand Up @@ -956,6 +975,7 @@ mod tests {
let path_c = CString::new(path.to_str().unwrap()).unwrap();
let elf_src = blaze_symbolize_src_elf {
path: path_c.as_ptr(),
debug_syms: true,
};
let symbolize = |symbolizer, addrs, addr_cnt| unsafe {
blaze_symbolize_elf_file_addrs(symbolizer, &elf_src, addrs, addr_cnt)
Expand All @@ -969,6 +989,7 @@ mod tests {
let path_c = CString::new(path.to_str().unwrap()).unwrap();
let elf_src = blaze_symbolize_src_elf {
path: path_c.as_ptr(),
debug_syms: true,
};
let symbolize = |symbolizer, addrs, addr_cnt| unsafe {
blaze_symbolize_elf_file_addrs(symbolizer, &elf_src, addrs, addr_cnt)
Expand Down Expand Up @@ -1009,7 +1030,6 @@ mod tests {
fn symbolize_dwarf_demangle() {
fn test(path: &Path, addr: Addr) -> Result<(), ()> {
let opts = blaze_symbolizer_opts {
debug_syms: true,
code_info: true,
inlined_fns: true,
demangle: false,
Expand All @@ -1018,6 +1038,7 @@ mod tests {
let path_c = CString::new(path.to_str().unwrap()).unwrap();
let elf_src = blaze_symbolize_src_elf {
path: path_c.as_ptr(),
debug_syms: true,
};
let symbolizer = unsafe { blaze_symbolizer_new_opts(&opts) };
let addrs = [addr];
Expand Down Expand Up @@ -1056,7 +1077,6 @@ mod tests {

// Do it again, this time with demangling enabled.
let opts = blaze_symbolizer_opts {
debug_syms: true,
code_info: true,
inlined_fns: true,
demangle: true,
Expand Down Expand Up @@ -1129,7 +1149,10 @@ mod tests {
/// Make sure that we can symbolize an address in a process.
#[test]
fn symbolize_in_process() {
let process_src = blaze_symbolize_src_process { pid: 0 };
let process_src = blaze_symbolize_src_process {
pid: 0,
debug_syms: true,
};

let symbolizer = blaze_symbolizer_new();
let addrs = [blaze_symbolizer_new as Addr];
Expand Down
Loading

0 comments on commit 7c53f91

Please sign in to comment.