Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions symbolic-debuginfo/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,20 @@ use symbolic_common::{clean_path, join_path, Arch, CodeId, DebugId, Name};
use crate::sourcebundle::SourceFileDescriptor;
use crate::ParseObjectOptions;

pub(crate) trait Parse<'data>: Sized {
/// A generic parser trait, used by the various debuginfos.
pub trait Parse<'data>: Sized {
/// The type of error to emit during parsing.
type Error;

fn parse_with_opts(data: &'data [u8], _opts: ParseObjectOptions) -> Result<Self, Self::Error>;
/// Parse the supplied bytes, with the specified parsing options.
fn parse_with_opts(data: &'data [u8], opts: ParseObjectOptions) -> Result<Self, Self::Error>;

/// Parse the supplied bytes with default parsing options.
fn parse(data: &'data [u8]) -> Result<Self, Self::Error> {
Self::parse_with_opts(data, Default::default())
}

/// Returns true if the supplied bytes can be successfully parsed.
fn test(data: &'data [u8]) -> bool {
Self::parse(data).is_ok()
}
Expand Down
16 changes: 6 additions & 10 deletions symbolic-debuginfo/src/breakpad.rs
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,7 @@ pub struct BreakpadObject<'data> {
arch: Arch,
module: BreakpadModuleRecord<'data>,
data: &'data [u8],
max_inline_depth: Option<u32>,
max_inline_depth: u32,
}

impl<'data> BreakpadObject<'data> {
Expand Down Expand Up @@ -1270,7 +1270,7 @@ impl<'data> Iterator for BreakpadSymbolIterator<'data> {
pub struct BreakpadDebugSession<'data> {
file_map: BreakpadFileMap<'data>,
lines: Lines<'data>,
max_inline_depth: Option<u32>,
max_inline_depth: u32,
}

impl BreakpadDebugSession<'_> {
Expand Down Expand Up @@ -1336,15 +1336,11 @@ pub struct BreakpadFunctionIterator<'s> {
next_line: Option<&'s [u8]>,
inline_origin_map: BreakpadInlineOriginMap<'s>,
lines: Lines<'s>,
max_inline_depth: Option<u32>,
max_inline_depth: u32,
}

impl<'s> BreakpadFunctionIterator<'s> {
fn new(
file_map: &'s BreakpadFileMap<'s>,
mut lines: Lines<'s>,
max_inline_depth: Option<u32>,
) -> Self {
fn new(file_map: &'s BreakpadFileMap<'s>, mut lines: Lines<'s>, max_inline_depth: u32) -> Self {
let next_line = lines.next();
Self {
file_map,
Expand Down Expand Up @@ -1395,8 +1391,8 @@ impl<'s> Iterator for BreakpadFunctionIterator<'s> {
b"",
fun_record.address,
fun_record.size,
)
.max_inline_depth(self.max_inline_depth);
self.max_inline_depth,
);

for line in self.lines.by_ref() {
// Stop parsing LINE records once other expected records are encountered.
Expand Down
112 changes: 88 additions & 24 deletions symbolic-debuginfo/src/dwarf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ impl<'d> UnitRef<'d, '_> {
entry: &Die<'d>,
language: Language,
bcsymbolmap: Option<&'d BcSymbolMap<'d>>,
prior_offset: Option<UnitOffset>,
depth: u8,
) -> Result<Option<Name<'d>>, DwarfError> {
let mut fallback_name = None;
let mut reference_target = None;
Expand Down Expand Up @@ -527,21 +527,16 @@ impl<'d> UnitRef<'d, '_> {

if let Some(attr) = reference_target {
return self.resolve_reference(*attr, |ref_unit, ref_entry| {
// Self-references may have a layer of indirection. Avoid infinite recursion
// in this scenario.
if let Some(prior) = prior_offset {
if self.offset() == ref_unit.offset() && prior == ref_entry.offset() {
return Ok(None);
}
// Avoid infinite recursion.
if depth == 0 {
return Err(DwarfError::new(
DwarfErrorKind::CorruptedData,
"Exceeded maximum resolve_function_name depth",
));
}

if self.offset() != ref_unit.offset() || entry.offset() != ref_entry.offset() {
ref_unit.resolve_function_name(
ref_entry,
language,
bcsymbolmap,
Some(entry.offset()),
)
ref_unit.resolve_function_name(ref_entry, language, bcsymbolmap, depth - 1)
} else {
Ok(None)
}
Expand All @@ -563,6 +558,9 @@ struct DwarfUnit<'d, 'a> {
}

impl<'d, 'a> DwarfUnit<'d, 'a> {
/// The maximum depth to recurse to in order to resolve a function name.
const MAX_RESOLVE_FUNCTION_DEPTH: u8 = 32;

/// Creates a DWARF unit from the gimli `Unit` type.
fn from_unit(
unit: &'a Unit<'d>,
Expand Down Expand Up @@ -815,6 +813,7 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
fn parse_functions(
&self,
depth: isize,
remaining_inline_depth: u32,
entries: &mut EntriesRaw<'d, '_>,
output: &mut FunctionsOutput<'_, 'd>,
) -> Result<(), DwarfError> {
Expand All @@ -824,9 +823,17 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
if next_depth <= depth {
return Ok(());
}

if let Some(abbrev) = entries.read_abbreviation()? {
if abbrev.tag() == constants::DW_TAG_subprogram {
self.parse_function(dw_die_offset, next_depth, entries, abbrev, output)?;
self.parse_function(
dw_die_offset,
next_depth,
remaining_inline_depth,
entries,
abbrev,
output,
)?;
} else {
entries.skip_attributes(abbrev.attributes())?;
}
Expand All @@ -848,6 +855,7 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
&self,
dw_die_offset: gimli::UnitOffset<usize>,
depth: isize,
remaining_inline_depth: u32,
entries: &mut EntriesRaw<'d, '_>,
abbrev: &gimli::Abbreviation,
output: &mut FunctionsOutput<'_, 'd>,
Expand Down Expand Up @@ -883,7 +891,7 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
// However, non-inlined functions may be present in this subtree, so we must still descend
// into it.
if ranges.is_empty() {
return self.parse_functions(depth, entries, output);
return self.parse_functions(depth, remaining_inline_depth, entries, output);
}

// Resolve functions in the symbol table first. Only if there is no entry, fall back
Expand All @@ -910,7 +918,12 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
let name = symbol_name
.or_else(|| {
self.inner
.resolve_function_name(&entry, language, self.bcsymbolmap, None)
.resolve_function_name(
&entry,
language,
self.bcsymbolmap,
DwarfUnit::MAX_RESOLVE_FUNCTION_DEPTH,
)
Comment thread
cursor[bot] marked this conversation as resolved.
.ok()
.flatten()
})
Expand All @@ -925,12 +938,26 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
let size = range.end - range.begin;
(
*range,
FunctionBuilder::new(name.clone(), self.compilation_dir(), address, size),
FunctionBuilder::new(
name.clone(),
self.compilation_dir(),
address,
size,
remaining_inline_depth,
),
)
})
.collect();

self.parse_function_children(depth, 0, entries, &mut builders, output, language)?;
self.parse_function_children(
depth,
0,
remaining_inline_depth,
entries,
&mut builders,
output,
language,
)?;

if let Some(line_program) = &self.line_program {
for (range, builder) in &mut builders {
Expand All @@ -952,10 +979,12 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
}

/// Traverses a subtree during function parsing.
#[allow(clippy::too_many_arguments)]
fn parse_function_children(
&self,
depth: isize,
inline_depth: u32,
remaining_inline_depth: u32,
entries: &mut EntriesRaw<'d, '_>,
builders: &mut [(Range, FunctionBuilder<'d>)],
output: &mut FunctionsOutput<'_, 'd>,
Expand All @@ -974,13 +1003,21 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
match abbrev.tag() {
constants::DW_TAG_subprogram => {
// Nested subprograms resolve their own language independently.
self.parse_function(dw_die_offset, next_depth, entries, abbrev, output)?;
self.parse_function(
dw_die_offset,
next_depth,
remaining_inline_depth,
entries,
abbrev,
output,
)?;
}
constants::DW_TAG_inlined_subroutine => {
self.parse_inlinee(
dw_die_offset,
next_depth,
inline_depth,
remaining_inline_depth,
entries,
abbrev,
builders,
Expand Down Expand Up @@ -1010,12 +1047,20 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
dw_die_offset: gimli::UnitOffset<usize>,
depth: isize,
inline_depth: u32,
remaining_inline_depth: u32,
entries: &mut EntriesRaw<'d, '_>,
abbrev: &gimli::Abbreviation,
builders: &mut [(Range, FunctionBuilder<'d>)],
output: &mut FunctionsOutput<'_, 'd>,
language: Language,
) -> Result<(), DwarfError> {
if remaining_inline_depth == 0 {
return Err(DwarfError::new(
DwarfErrorKind::CorruptedData,
"Exceeded max parse inlinee depth",
));
}

let (ranges, call_location) = self.parse_ranges(entries, abbrev, &mut output.range_buf)?;

ranges.retain(|range| range.end > range.begin);
Expand All @@ -1028,7 +1073,7 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
// However, non-inlined functions may be present in this subtree, so we must still descend
// into it.
if ranges.is_empty() {
return self.parse_functions(depth, entries, output);
return self.parse_functions(depth, remaining_inline_depth, entries, output);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty inlinees bypass depth limit

High Severity

When a DW_TAG_inlined_subroutine or DW_TAG_subprogram has no code ranges, parsing continues via parse_functions without decrementing remaining_inline_depth. A long chain of empty surrogate DIEs can recurse indefinitely and still hit stack overflow, bypassing the new inline depth cap.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7e8612e. Configure here.

}

let entry = self.inner.unit.entry(dw_die_offset)?;
Expand All @@ -1039,7 +1084,12 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
// which carries the wrong language (e.g. a C++ LTO partial unit for C code).
let name = self
.inner
.resolve_function_name(&entry, language, self.bcsymbolmap, None)
.resolve_function_name(
&entry,
language,
self.bcsymbolmap,
DwarfUnit::MAX_RESOLVE_FUNCTION_DEPTH,
)
.ok()
.flatten()
.unwrap_or_else(|| Name::new("", NameMangling::Unmangled, language));
Expand Down Expand Up @@ -1073,17 +1123,26 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
);
}

self.parse_function_children(depth, inline_depth + 1, entries, builders, output, language)
self.parse_function_children(
depth,
inline_depth + 1,
remaining_inline_depth - 1,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dwarf breakpad inline limit mismatch

Medium Severity

The same max_inline_depth option is applied differently in DWARF versus Breakpad/FunctionBuilder. DWARF decrements a remaining budget per inlinee (so 128 allows 128 frames with depth indices 0..127), while FunctionBuilder keeps records while depth <= max_inline_depth (so 512 keeps depth indices 0..512, as in the symcache test).

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7e8612e. Configure here.

entries,
builders,
output,
language,
)
}

/// Collects all functions within this compilation unit.
fn functions(
&self,
seen_ranges: &mut BTreeSet<(u64, u64)>,
max_inline_depth: u32,
) -> Result<Vec<Function<'d>>, DwarfError> {
let mut entries = self.inner.unit.entries_raw(None)?;
let mut output = FunctionsOutput::with_seen_ranges(seen_ranges);
self.parse_functions(-1, &mut entries, &mut output)?;
self.parse_functions(-1, max_inline_depth, &mut entries, &mut output)?;
Ok(output.functions)
}
}
Expand Down Expand Up @@ -1407,6 +1466,7 @@ impl std::iter::FusedIterator for DwarfUnitIterator<'_> {}
pub struct DwarfDebugSession<'data> {
cell: SelfCell<Box<DwarfSections<'data>>, DwarfInfo<'data>>,
bcsymbolmap: Option<Arc<BcSymbolMap<'data>>>,
max_inline_depth: u32,
}

impl<'data> DwarfDebugSession<'data> {
Expand All @@ -1416,6 +1476,7 @@ impl<'data> DwarfDebugSession<'data> {
symbol_map: SymbolMap<'data>,
address_offset: i64,
kind: ObjectKind,
max_inline_depth: u32,
) -> Result<Self, DwarfError>
where
D: Dwarf<'data>,
Expand All @@ -1428,6 +1489,7 @@ impl<'data> DwarfDebugSession<'data> {
Ok(DwarfDebugSession {
cell,
bcsymbolmap: None,
max_inline_depth,
})
}

Expand Down Expand Up @@ -1456,6 +1518,7 @@ impl<'data> DwarfDebugSession<'data> {
functions: Vec::new().into_iter(),
seen_ranges: BTreeSet::new(),
finished: false,
max_inline_depth: self.max_inline_depth,
}
}

Expand Down Expand Up @@ -1566,6 +1629,7 @@ pub struct DwarfFunctionIterator<'s> {
functions: std::vec::IntoIter<Function<'s>>,
seen_ranges: BTreeSet<(u64, u64)>,
finished: bool,
max_inline_depth: u32,
}

impl<'s> Iterator for DwarfFunctionIterator<'s> {
Expand All @@ -1587,7 +1651,7 @@ impl<'s> Iterator for DwarfFunctionIterator<'s> {
None => break,
};

self.functions = match unit.functions(&mut self.seen_ranges) {
self.functions = match unit.functions(&mut self.seen_ranges, self.max_inline_depth) {
Ok(functions) => functions.into_iter(),
Err(error) => return Some(Err(error)),
};
Expand Down
Loading
Loading