From 4e2760442127ac7a6ca753d28e2451632e5b01f4 Mon Sep 17 00:00:00 2001 From: Chris Klochek Date: Sun, 30 Aug 2026 22:05:19 +0200 Subject: [PATCH 1/5] fix(debuginfo): Remove recursive function parsing --- symbolic-debuginfo/src/breakpad.rs | 26 +- symbolic-debuginfo/src/dwarf/mod.rs | 517 ++++++++++-------- symbolic-debuginfo/src/elf.rs | 8 +- symbolic-debuginfo/src/function_builder.rs | 11 +- symbolic-debuginfo/src/macho/mod.rs | 8 +- symbolic-debuginfo/src/object.rs | 12 +- symbolic-debuginfo/src/pe.rs | 8 +- symbolic-debuginfo/src/wasm.rs | 4 +- symbolic-debuginfo/src/wasm/parser.rs | 2 +- .../tests/test_recursion_limits.rs | 8 +- symbolic-symcache/src/writer.rs | 332 ++++++++++- symbolic-symcache/tests/breakpad.rs | 2 +- 12 files changed, 667 insertions(+), 271 deletions(-) diff --git a/symbolic-debuginfo/src/breakpad.rs b/symbolic-debuginfo/src/breakpad.rs index 0bc462623..0bd4212c0 100644 --- a/symbolic-debuginfo/src/breakpad.rs +++ b/symbolic-debuginfo/src/breakpad.rs @@ -957,7 +957,7 @@ pub struct BreakpadObject<'data> { arch: Arch, module: BreakpadModuleRecord<'data>, data: &'data [u8], - max_inline_depth: u32, + max_function_parse_depth: u32, } impl<'data> BreakpadObject<'data> { @@ -1000,7 +1000,7 @@ impl<'data> BreakpadObject<'data> { .map_err(|_| BreakpadErrorKind::InvalidArchitecture)?, module, data, - max_inline_depth: opts.max_inline_depth, + max_function_parse_depth: opts.max_function_parse_depth, }) } @@ -1089,7 +1089,7 @@ impl<'data> BreakpadObject<'data> { Ok(BreakpadDebugSession { file_map: self.file_map(), lines: Lines::new(self.data), - max_inline_depth: self.max_inline_depth, + max_function_parse_depth: self.max_function_parse_depth, }) } @@ -1282,13 +1282,17 @@ impl<'data> Iterator for BreakpadSymbolIterator<'data> { pub struct BreakpadDebugSession<'data> { file_map: BreakpadFileMap<'data>, lines: Lines<'data>, - max_inline_depth: u32, + max_function_parse_depth: u32, } impl BreakpadDebugSession<'_> { /// Returns an iterator over all functions in this debug file. pub fn functions(&self) -> BreakpadFunctionIterator<'_> { - BreakpadFunctionIterator::new(&self.file_map, self.lines.clone(), self.max_inline_depth) + BreakpadFunctionIterator::new( + &self.file_map, + self.lines.clone(), + self.max_function_parse_depth, + ) } /// Returns an iterator over all source files in this debug file. @@ -1359,18 +1363,22 @@ pub struct BreakpadFunctionIterator<'s> { next_line: Option<&'s [u8]>, inline_origin_map: BreakpadInlineOriginMap<'s>, lines: Lines<'s>, - max_inline_depth: u32, + max_function_parse_depth: u32, } impl<'s> BreakpadFunctionIterator<'s> { - fn new(file_map: &'s BreakpadFileMap<'s>, mut lines: Lines<'s>, max_inline_depth: u32) -> Self { + fn new( + file_map: &'s BreakpadFileMap<'s>, + mut lines: Lines<'s>, + max_function_parse_depth: u32, + ) -> Self { let next_line = lines.next(); Self { file_map, next_line, inline_origin_map: Default::default(), lines, - max_inline_depth, + max_function_parse_depth, } } } @@ -1414,7 +1422,7 @@ impl<'s> Iterator for BreakpadFunctionIterator<'s> { b"", fun_record.address, fun_record.size, - self.max_inline_depth, + self.max_function_parse_depth, ); for line in self.lines.by_ref() { diff --git a/symbolic-debuginfo/src/dwarf/mod.rs b/symbolic-debuginfo/src/dwarf/mod.rs index 4e6db6729..e9845d90d 100644 --- a/symbolic-debuginfo/src/dwarf/mod.rs +++ b/symbolic-debuginfo/src/dwarf/mod.rs @@ -637,6 +637,165 @@ struct DwarfUnit<'d, 'a> { prefer_dwarf_names: bool, } +type Builders<'a> = Vec<(Range, FunctionBuilder<'a>)>; + +struct RegularSubProgram<'a> { + index: usize, + depth: isize, + builders: Vec<(Range, FunctionBuilder<'a>)>, + variables: Vec>, + language: Language, +} + +struct InlinedSubProgram<'a> { + depth: isize, + relative_list_depth: isize, + ranges: Vec, + owning_function_idx: usize, + variables: Vec>, + name: Name<'a>, + call_file: FileInfo<'a>, + call_line: u64, + language: Language, +} + +struct DeadcodeSubProgram { + depth: isize, +} + +enum InProgressSubProgram<'a> { + Deadcode(DeadcodeSubProgram), + Regular(RegularSubProgram<'a>), + Inlined(InlinedSubProgram<'a>), +} + +impl<'a> InProgressSubProgram<'a> { + fn language(&self) -> Language { + match self { + InProgressSubProgram::Deadcode(_) => Language::Unknown, + InProgressSubProgram::Regular(p) => p.language, + InProgressSubProgram::Inlined(p) => p.language, + } + } + + fn own_builder_mut(&mut self) -> Option<&mut Builders<'a>> { + match self { + InProgressSubProgram::Regular(p) => Some(&mut p.builders), + InProgressSubProgram::Inlined(_) => None, + InProgressSubProgram::Deadcode(_) => None, + } + } + + fn owning_function_idx(&self) -> usize { + match self { + InProgressSubProgram::Deadcode(_) => 0, + InProgressSubProgram::Regular(p) => p.index, + InProgressSubProgram::Inlined(p) => p.owning_function_idx, + } + } + + fn finish( + self, + dwarf_unit: &DwarfUnit<'a, '_>, + output: &mut FunctionsOutput<'_, 'a>, + function_stack: &mut [InProgressSubProgram<'a>], + ) -> Result<(), FunctionBuilderError> { + match self { + InProgressSubProgram::Deadcode(_) => Ok(()), + InProgressSubProgram::Regular(mut p) => { + for (range, builder) in p.builders.iter_mut() { + for variable in &p.variables { + if let Some(variable) = dwarf_unit.variable_for_range(variable, *range) { + builder.add_variable(variable); + } + } + } + + if let Some(line_program) = &dwarf_unit.line_program { + for (range, builder) in p.builders.iter_mut() { + for row in line_program.get_rows(range) { + let address = offset(row.address, dwarf_unit.inner.info.address_offset); + let size = row.size; + let file = dwarf_unit.resolve_file(row.file_index).unwrap_or_default(); + let line = row.line.unwrap_or(0); + builder.add_leaf_line(address, size, file, line); + } + } + } + + for (_range, builder) in p.builders { + output.functions.push(builder.finish()?); + } + + Ok(()) + } + + // Inlinees don't output anything directly, they just contribute to the running list + // of function builders. + InProgressSubProgram::Inlined(p) => { + let Some(builders) = function_stack[p.owning_function_idx].own_builder_mut() else { + return Err(FunctionBuilderErrorKind::TooManyInlineeNestings.into()); + }; + // Create a separate inlinee for each range. + for range in p.ranges.iter() { + // Find the builder for the outer function that covers this range. Usually there's only + // one outer range, so only one builder. + // + // We can use `partition_point` here, because builders are sorted by range and + // non-overlapping, see `parse_ranges`. + + let builder_index = + builders.partition_point(|(outer_range, _)| outer_range.end <= range.begin); + + let Some((outer_range, builder)) = builders.get_mut(builder_index) else { + continue; + }; + // `partition_point` may return the next builder when `range.begin` falls into a gap between outer ranges. + if range.begin < outer_range.begin { + continue; + } + + let address = offset(range.begin, dwarf_unit.inner.info.address_offset); + let size = range.end - range.begin; + let variables = p + .variables + .iter() + .filter_map(|variable| dwarf_unit.variable_for_range(variable, *range)) + .collect(); + + builder.add_inlinee(FunctionBuilderInlinee { + depth: p.relative_list_depth as u32, + name: p.name.clone(), + address, + size, + call_file: p.call_file.clone(), + call_line: p.call_line, + variables, + }); + } + + Ok(()) + } + } + } + + fn push_variable(&mut self, variable: ParsedVariable<'a>) { + match self { + InProgressSubProgram::Deadcode(_) => {} + InProgressSubProgram::Regular(p) => p.variables.push(variable), + InProgressSubProgram::Inlined(p) => p.variables.push(variable), + } + } + + fn depth(&self) -> isize { + match self { + InProgressSubProgram::Deadcode(p) => p.depth, + InProgressSubProgram::Regular(p) => p.depth, + InProgressSubProgram::Inlined(p) => p.depth, + } + } +} + 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; @@ -914,60 +1073,19 @@ impl<'d, 'a> DwarfUnit<'d, 'a> { .unwrap_or(fallback_language) } - /// Parses any DW_TAG_subprogram DIEs in the DIE subtree. - fn parse_functions( + #[allow(clippy::too_many_arguments)] + fn consume_subprogram_tag( &self, + index: usize, depth: isize, - remaining_inline_depth: u32, - entries: &mut EntriesRaw<'d, '_>, - output: &mut FunctionsOutput<'_, 'd>, - ) -> Result<(), DwarfError> { - while !entries.is_empty() { - let dw_die_offset = entries.next_offset(); - let next_depth = entries.next_depth(); - 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, - remaining_inline_depth, - entries, - abbrev, - output, - )?; - } else { - entries.skip_attributes(abbrev.attributes())?; - } - } - } - Ok(()) - } - - /// Parse a single function from a DWARF DIE subtree. - /// - /// The `entries` iterator must be placed after the abbrev / before the attributes of the - /// function DIE. - /// - /// This method can call itself recursively if another DW_TAG_subprogram entry is encountered - /// in the subtree. - /// - /// On return, the `entries` iterator is placed after the attributes of the last-read DIE. - fn parse_function( - &self, dw_die_offset: gimli::UnitOffset, - depth: isize, - remaining_inline_depth: u32, entries: &mut EntriesRaw<'d, '_>, abbrev: &gimli::Abbreviation, - output: &mut FunctionsOutput<'_, 'd>, - ) -> Result<(), DwarfError> { - let (ranges, _) = self.parse_ranges(entries, abbrev, &mut output.range_buf)?; + seen_ranges: &mut BTreeSet<(u64, u64)>, + ranges: &mut Vec, + ) -> Result, DwarfError> { + self.parse_ranges(entries, abbrev, ranges)?; - let seen_ranges = &mut *output.seen_ranges; ranges.retain(|range| { // We have seen duplicate top-level function entries being yielded from the // [`DwarfFunctionIterator`], which combined with recursively walking its inlinees can @@ -991,7 +1109,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, remaining_inline_depth, entries, output); + return Ok(InProgressSubProgram::Deadcode(DeadcodeSubProgram { depth })); } // Resolve functions in the symbol table first. Only if there is no entry, fall back @@ -1031,153 +1149,41 @@ impl<'d, 'a> DwarfUnit<'d, 'a> { // Create one function per range. In the common case there is only one range, so // we usually only have one function builder here. - let mut builders: Vec<(Range, FunctionBuilder)> = ranges + let builders = ranges .iter() .map(|range| { let address = offset(range.begin, self.inner.info.address_offset); let size = range.end - range.begin; + // TODO: remove inline depth limit ( *range, - FunctionBuilder::new( - name.clone(), - self.compilation_dir(), - address, - size, - remaining_inline_depth, - ), + FunctionBuilder::new(name.clone(), self.compilation_dir(), address, size, 999), ) }) .collect(); - let mut variables = Vec::new(); - self.parse_function_children( + Ok(InProgressSubProgram::Regular(RegularSubProgram { + index, depth, - 0, - remaining_inline_depth, - entries, - &mut builders, - output, + builders, + variables: vec![], language, - &mut variables, - )?; - - for (range, builder) in &mut builders { - for variable in &variables { - if let Some(variable) = self.variable_for_range(variable, *range) { - builder.add_variable(variable); - } - } - } - - if let Some(line_program) = &self.line_program { - for (range, builder) in &mut builders { - for row in line_program.get_rows(range) { - let address = offset(row.address, self.inner.info.address_offset); - let size = row.size; - let file = self.resolve_file(row.file_index).unwrap_or_default(); - let line = row.line.unwrap_or(0); - builder.add_leaf_line(address, size, file, line); - } - } - } - - for (_range, builder) in builders { - output.functions.push(builder.finish()?); - } - - Ok(()) + })) } - /// Traverses a subtree during function parsing. #[allow(clippy::too_many_arguments)] - fn parse_function_children( + fn consume_inline_subprogram_tag( &self, depth: isize, - inline_depth: u32, - remaining_inline_depth: u32, - entries: &mut EntriesRaw<'d, '_>, - builders: &mut [(Range, FunctionBuilder<'d>)], - output: &mut FunctionsOutput<'_, 'd>, - language: Language, - variables: &mut Vec>, - ) -> Result<(), DwarfError> { - while !entries.is_empty() { - let dw_die_offset = entries.next_offset(); - let next_depth = entries.next_depth(); - if next_depth <= depth { - return Ok(()); - } - let abbrev = match entries.read_abbreviation()? { - Some(abbrev) => abbrev, - None => continue, - }; - match abbrev.tag() { - constants::DW_TAG_subprogram => { - // Nested subprograms resolve their own language independently. - 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, - output, - language, - )?; - } - constants::DW_TAG_variable | constants::DW_TAG_formal_parameter => { - if let Some(variable) = self.parse_variable(entries, abbrev)? { - variables.push(variable); - } - } - _ => { - entries.skip_attributes(abbrev.attributes())?; - } - } - } - Ok(()) - } - - /// Recursively parse the inlinees of a function from a DWARF DIE subtree. - /// - /// The `entries` iterator must be placed just before the attributes of the inline function DIE. - /// - /// This method calls itself recursively for other DW_TAG_inlined_subroutine entries in the - /// subtree. It can also call `parse_function` if a `DW_TAG_subprogram` entry is encountered. - /// - /// On return, the `entries` iterator is placed after the attributes of the last-read DIE. - #[allow(clippy::too_many_arguments)] - fn parse_inlinee( - &self, + relative_list_depth: isize, dw_die_offset: gimli::UnitOffset, - depth: isize, - inline_depth: u32, - remaining_inline_depth: u32, + owning_function_idx: usize, + language: Language, 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: &mut Vec, + ) -> Result, DwarfError> { + let (_, call_location) = self.parse_ranges(entries, abbrev, ranges)?; // Ranges can be empty for three reasons: (1) the function is a no-op and does not // contain any code, (2) the function did contain eliminated dead code, or (3) some @@ -1187,7 +1193,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, remaining_inline_depth, entries, output); + return Ok(InProgressSubProgram::Deadcode(DeadcodeSubProgram { depth })); } let ranges = ranges.clone(); @@ -1215,52 +1221,113 @@ impl<'d, 'a> DwarfUnit<'d, 'a> { .unwrap_or_default(); let call_line = call_location.call_line.unwrap_or(0); - let mut variables = Vec::new(); - self.parse_function_children( + Ok(InProgressSubProgram::Inlined(InlinedSubProgram { depth, - inline_depth + 1, - remaining_inline_depth - 1, - entries, - builders, - output, + relative_list_depth, + ranges, + owning_function_idx, + variables: vec![], + name, + call_file, + call_line, language, - &mut variables, - )?; - - // Create a separate inlinee for each range. - for range in ranges.iter() { - // Find the builder for the outer function that covers this range. Usually there's only - // one outer range, so only one builder. - // - // We can use `partition_point` here, because builders are sorted by range and - // non-overlapping, see `parse_ranges`. - let builder_index = - builders.partition_point(|(outer_range, _)| outer_range.end <= range.begin); - - let Some((outer_range, builder)) = builders.get_mut(builder_index) else { + })) + } + + /// Parses any DW_TAG_subprogram DIEs in the DIE subtree. + fn parse_functions( + &self, + entries: &mut EntriesRaw<'d, '_>, + output: &mut FunctionsOutput<'_, 'd>, + max_parse_depth: usize, + ) -> Result<(), DwarfError> { + let mut function_stack: Vec> = vec![]; + + while !entries.is_empty() { + let dw_die_offset = entries.next_offset(); + let next_depth = entries.next_depth(); + + // Use next_depth to finish any in-progress functions that are higher on the stack. + while let Some(last_func) = function_stack.last() { + if last_func.depth() < next_depth { + break; + } + + let last_func: InProgressSubProgram<'_> = + function_stack.pop().expect("already checked"); + last_func.finish(self, output, &mut function_stack)?; + } + + let Some(abbrev) = entries.read_abbreviation()? else { continue; }; - // `partition_point` may return the next builder when `range.begin` falls into a gap between outer ranges. - if range.begin < outer_range.begin { - continue; - } - let address = offset(range.begin, self.inner.info.address_offset); - let size = range.end - range.begin; - let variables = variables - .iter() - .filter_map(|variable| self.variable_for_range(variable, *range)) - .collect(); + // It's possible the top function is dead-code; if so, we want to ignore anything + // nested inside that is NOT a subprogram. + let deadcode_top = function_stack + .last() + .is_some_and(|p| matches!(p, InProgressSubProgram::Deadcode(_))); - builder.add_inlinee(FunctionBuilderInlinee { - depth: inline_depth, - name: name.clone(), - address, - size, - call_file: call_file.clone(), - call_line, - variables, - }); + match abbrev.tag() { + // Always process a subprogram, even if we have a deadcode frame at top of stack. + constants::DW_TAG_subprogram => { + let program = self.consume_subprogram_tag( + function_stack.len(), + next_depth, + dw_die_offset, + entries, + abbrev, + output.seen_ranges, + &mut output.range_buf, + )?; + + function_stack.push(program); + } + // If we have a deadcode frame at the top of stack, just skip the entry. + _ if deadcode_top => { + entries.skip_attributes(abbrev.attributes())?; + } + constants::DW_TAG_inlined_subroutine => { + let Some(prev_program) = function_stack.last() else { + entries.skip_attributes(abbrev.attributes())?; + continue; + }; + let owning_function_idx = prev_program.owning_function_idx(); + let language = prev_program.language(); + + let program = self.consume_inline_subprogram_tag( + next_depth, + function_stack.len() as isize - (owning_function_idx + 1) as isize, + dw_die_offset, + owning_function_idx, + language, + entries, + abbrev, + &mut output.range_buf, + )?; + function_stack.push(program); + } + constants::DW_TAG_variable | constants::DW_TAG_formal_parameter => { + if let Some(variable) = self.parse_variable(entries, abbrev)? { + if let Some(last_function) = function_stack.last_mut() { + last_function.push_variable(variable); + } + } + } + _ => { + entries.skip_attributes(abbrev.attributes())?; + } + } + if function_stack.len() > max_parse_depth { + return Err(DwarfError::new( + DwarfErrorKind::CorruptedData, + "Exceeded max parse depth", + )); + } + } + + while let Some(f) = function_stack.pop() { + f.finish(self, output, &mut function_stack)?; } Ok(()) @@ -1420,11 +1487,11 @@ impl<'d, 'a> DwarfUnit<'d, 'a> { fn functions( &self, seen_ranges: &mut BTreeSet<(u64, u64)>, - max_inline_depth: u32, + max_parse_depth: u32, ) -> Result>, DwarfError> { let mut entries = self.inner.unit.entries_raw(None)?; let mut output = FunctionsOutput::with_seen_ranges(seen_ranges); - self.parse_functions(-1, max_inline_depth, &mut entries, &mut output)?; + self.parse_functions(&mut entries, &mut output, max_parse_depth as usize)?; Ok(output.functions) } } @@ -1868,7 +1935,7 @@ impl std::iter::FusedIterator for DwarfUnitIterator<'_> {} pub struct DwarfDebugSession<'data> { cell: SelfCell>, DwarfInfo<'data>>, bcsymbolmap: Option>>, - max_inline_depth: u32, + max_parse_depth: u32, } impl<'data> DwarfDebugSession<'data> { @@ -1878,7 +1945,7 @@ impl<'data> DwarfDebugSession<'data> { symbol_map: SymbolMap<'data>, address_offset: i64, kind: ObjectKind, - max_inline_depth: u32, + max_parse_depth: u32, ) -> Result where D: Dwarf<'data>, @@ -1891,7 +1958,7 @@ impl<'data> DwarfDebugSession<'data> { Ok(DwarfDebugSession { cell, bcsymbolmap: None, - max_inline_depth, + max_parse_depth, }) } @@ -1920,7 +1987,7 @@ impl<'data> DwarfDebugSession<'data> { functions: Vec::new().into_iter(), seen_ranges: BTreeSet::new(), finished: false, - max_inline_depth: self.max_inline_depth, + max_parse_depth: self.max_parse_depth, } } @@ -2067,7 +2134,7 @@ pub struct DwarfFunctionIterator<'s> { functions: std::vec::IntoIter>, seen_ranges: BTreeSet<(u64, u64)>, finished: bool, - max_inline_depth: u32, + max_parse_depth: u32, } impl<'s> Iterator for DwarfFunctionIterator<'s> { @@ -2089,7 +2156,7 @@ impl<'s> Iterator for DwarfFunctionIterator<'s> { None => break, }; - self.functions = match unit.functions(&mut self.seen_ranges, self.max_inline_depth) { + self.functions = match unit.functions(&mut self.seen_ranges, self.max_parse_depth) { Ok(functions) => functions.into_iter(), Err(error) => return Some(Err(error)), }; diff --git a/symbolic-debuginfo/src/elf.rs b/symbolic-debuginfo/src/elf.rs index 99d33c894..1bbfc5360 100644 --- a/symbolic-debuginfo/src/elf.rs +++ b/symbolic-debuginfo/src/elf.rs @@ -72,7 +72,7 @@ pub struct ElfObject<'data> { data: &'data [u8], is_malformed: bool, max_decompressed_section_size: Option, - max_inline_depth: u32, + max_function_parse_depth: u32, } impl<'data> ElfObject<'data> { @@ -172,7 +172,7 @@ impl<'data> ElfObject<'data> { data, is_malformed: true, max_decompressed_section_size: opts.max_decompressed_section_size, - max_inline_depth: opts.max_inline_depth, + max_function_parse_depth: opts.max_function_parse_depth, }); } }; @@ -357,7 +357,7 @@ impl<'data> ElfObject<'data> { data, is_malformed: false, max_decompressed_section_size: opts.max_decompressed_section_size, - max_inline_depth: opts.max_inline_depth, + max_function_parse_depth: opts.max_function_parse_depth, }) } @@ -570,7 +570,7 @@ impl<'data> ElfObject<'data> { symbols, self.load_address() as i64, self.kind(), - self.max_inline_depth, + self.max_function_parse_depth, ) } diff --git a/symbolic-debuginfo/src/function_builder.rs b/symbolic-debuginfo/src/function_builder.rs index 1dc92f612..bf8269f91 100644 --- a/symbolic-debuginfo/src/function_builder.rs +++ b/symbolic-debuginfo/src/function_builder.rs @@ -63,7 +63,8 @@ pub struct FunctionBuilder<'s> { lines: Vec>, /// All variables found inside the function. variables: Vec>, - max_inline_depth: u32, + /// The maximum function parse depth we allow. + max_function_parse_depth: u32, } impl<'s> FunctionBuilder<'s> { @@ -73,7 +74,7 @@ impl<'s> FunctionBuilder<'s> { compilation_dir: &'s [u8], address: u64, size: u64, - max_inline_depth: u32, + max_function_parse_depth: u32, ) -> Self { Self { name, @@ -83,7 +84,7 @@ impl<'s> FunctionBuilder<'s> { inlinees: BinaryHeap::new(), lines: Vec::new(), variables: Vec::new(), - max_inline_depth, + max_function_parse_depth, } } @@ -93,7 +94,7 @@ impl<'s> FunctionBuilder<'s> { pub fn add_inlinee(&mut self, inlinee: FunctionBuilderInlinee<'s>) { // An inlinee that starts before the function is obviously bogus same for an inlinee that // has a depth deeper than the limit. - if inlinee.address < self.address || inlinee.depth > self.max_inline_depth { + if inlinee.address < self.address || inlinee.depth > self.max_function_parse_depth { return; } @@ -144,7 +145,7 @@ impl<'s> FunctionBuilder<'s> { inlinees, variables, mut lines, - max_inline_depth: _, + max_function_parse_depth: _, } = self; let inlinees = ensure_proper_nesting(inlinees)?; diff --git a/symbolic-debuginfo/src/macho/mod.rs b/symbolic-debuginfo/src/macho/mod.rs index a1aa10a40..fca7887ca 100644 --- a/symbolic-debuginfo/src/macho/mod.rs +++ b/symbolic-debuginfo/src/macho/mod.rs @@ -62,7 +62,7 @@ pub struct MachObject<'d> { macho: mach::MachO<'d>, data: &'d [u8], bcsymbolmap: Option>>, - max_inline_depth: u32, + max_function_parse_depth: u32, } impl<'d> MachObject<'d> { @@ -78,7 +78,7 @@ impl<'d> MachObject<'d> { macho, data, bcsymbolmap: None, - max_inline_depth: ParseObjectOptions::default().max_inline_depth, + max_function_parse_depth: ParseObjectOptions::default().max_function_parse_depth, }) .map_err(MachError::new) } @@ -326,7 +326,7 @@ impl<'d> MachObject<'d> { symbols, self.load_address() as i64, self.kind(), - self.max_inline_depth, + self.max_function_parse_depth, )?; session.load_symbolmap(self.bcsymbolmap.clone()); Ok(session) @@ -401,7 +401,7 @@ impl<'d> Parse<'d> for MachObject<'d> { macho, data, bcsymbolmap: None, - max_inline_depth: opts.max_inline_depth, + max_function_parse_depth: opts.max_function_parse_depth, }) .map_err(MachError::new) } diff --git a/symbolic-debuginfo/src/object.rs b/symbolic-debuginfo/src/object.rs index 50e433312..03eb8a4dd 100644 --- a/symbolic-debuginfo/src/object.rs +++ b/symbolic-debuginfo/src/object.rs @@ -126,13 +126,7 @@ impl Error for ObjectError { } } -// (Jul 2026): For reference, macOS Chromium has a max inlinee depth of around 60, so -// let's double it; 128 ought to be enough for anybody. -// (Aug 2026): For reference, Android Minecraft has a max inlinee depth of around 400. Set this -// to 512. -// (Aug 2026): For reference, FIFA (Android) has a max inlinee depth of around 670. Set this -// to 800. -const MAX_INLINE_DEPTH_DEFAULT: u32 = 800; +const MAX_FUNCTION_PARSE_DEPTH_DEFAULT: u32 = 2000; /// Options for parsing object files. #[non_exhaustive] @@ -149,7 +143,7 @@ pub struct ParseObjectOptions { pub max_decompressed_embedded_source_size: Option, /// The maximum inline nesting depth to process. - pub max_inline_depth: u32, + pub max_function_parse_depth: u32, } impl Default for ParseObjectOptions { @@ -157,7 +151,7 @@ impl Default for ParseObjectOptions { Self { max_decompressed_section_size: Default::default(), max_decompressed_embedded_source_size: Default::default(), - max_inline_depth: MAX_INLINE_DEPTH_DEFAULT, + max_function_parse_depth: MAX_FUNCTION_PARSE_DEPTH_DEFAULT, } } } diff --git a/symbolic-debuginfo/src/pe.rs b/symbolic-debuginfo/src/pe.rs index 00181cc7d..52c12235d 100644 --- a/symbolic-debuginfo/src/pe.rs +++ b/symbolic-debuginfo/src/pe.rs @@ -70,7 +70,7 @@ pub struct PeObject<'data> { pe: pe::PE<'data>, data: &'data [u8], is_stub: bool, - max_inline_depth: u32, + max_function_parse_depth: u32, } impl<'data> PeObject<'data> { @@ -99,7 +99,7 @@ impl<'data> PeObject<'data> { pe, data, is_stub, - max_inline_depth: ParseObjectOptions::default().max_inline_depth, + max_function_parse_depth: ParseObjectOptions::default().max_function_parse_depth, }) } @@ -262,7 +262,7 @@ impl<'data> PeObject<'data> { symbols, self.load_address() as i64, self.kind(), - self.max_inline_depth, + self.max_function_parse_depth, ) } @@ -419,7 +419,7 @@ impl<'data> Parse<'data> for PeObject<'data> { pe, data, is_stub, - max_inline_depth: popts.max_inline_depth, + max_function_parse_depth: popts.max_function_parse_depth, }) } } diff --git a/symbolic-debuginfo/src/wasm.rs b/symbolic-debuginfo/src/wasm.rs index c9775b5a5..8b48c78c0 100644 --- a/symbolic-debuginfo/src/wasm.rs +++ b/symbolic-debuginfo/src/wasm.rs @@ -34,7 +34,7 @@ pub struct WasmObject<'data> { data: &'data [u8], code_offset: u64, kind: ObjectKind, - max_inline_depth: u32, + max_function_parse_depth: u32, } impl<'data> WasmObject<'data> { @@ -124,7 +124,7 @@ impl<'data> WasmObject<'data> { symbols, -(self.code_offset() as i64), self.kind(), - self.max_inline_depth, + self.max_function_parse_depth, ) } diff --git a/symbolic-debuginfo/src/wasm/parser.rs b/symbolic-debuginfo/src/wasm/parser.rs index 94390f0cb..a32d6ae8b 100644 --- a/symbolic-debuginfo/src/wasm/parser.rs +++ b/symbolic-debuginfo/src/wasm/parser.rs @@ -240,7 +240,7 @@ impl<'d> Parse<'d> for WasmObject<'d> { data, code_offset, kind, - max_inline_depth: opts.max_inline_depth, + max_function_parse_depth: opts.max_function_parse_depth, }) } } diff --git a/symbolic-debuginfo/tests/test_recursion_limits.rs b/symbolic-debuginfo/tests/test_recursion_limits.rs index 662f250ac..88e41d572 100644 --- a/symbolic-debuginfo/tests/test_recursion_limits.rs +++ b/symbolic-debuginfo/tests/test_recursion_limits.rs @@ -1,5 +1,5 @@ use std::assert_matches; -use symbolic_debuginfo::{Object, ParseObjectOptions}; +use symbolic_debuginfo::{Object}; #[test] fn test_resolve_function() { @@ -19,11 +19,7 @@ fn test_resolve_function() { #[test] fn test_function_inlining() { let data = std::fs::read("tests/fixtures/deep_inline.elf").unwrap(); - let mut opts = ParseObjectOptions::default(); - // Use a lower bound that will still work in debug. The elf file itself has 10K (more?) - // nested inlinees. - opts.max_inline_depth = 400; - let object = Object::parse_with_opts(&data, opts).unwrap(); + let object = Object::parse(&data).unwrap(); let session = object.debug_session().unwrap(); diff --git a/symbolic-symcache/src/writer.rs b/symbolic-symcache/src/writer.rs index eb9029171..07dfd4b90 100644 --- a/symbolic-symcache/src/writer.rs +++ b/symbolic-symcache/src/writer.rs @@ -3,6 +3,7 @@ use std::collections::BTreeMap; use std::collections::btree_map; use std::io::Write; +use std::rc::Rc; use indexmap::IndexSet; use symbolic_common::{Arch, DebugId}; @@ -69,6 +70,13 @@ pub struct SymCacheConverter<'a> { last_addr: Option, } +struct InProgressFunction<'a> { + function: &'a Function<'a>, + base_index: u32, + depth: u16, + call_locations: Rc>, +} + impl<'a> SymCacheConverter<'a> { /// Creates a new Converter. pub fn new() -> Self { @@ -124,7 +132,14 @@ impl<'a> SymCacheConverter<'a> { for function in session.functions() { let function = function.map_err(|e| Error::new(ErrorKind::BadDebugFile, e))?; - self.process_symbolic_function_recursive(&session, &function, &[(0x0, u32::MAX)], 0, 0); + let function = InProgressFunction { + function: &function, + base_index: 0, + depth: 0, + call_locations: Rc::new(vec![(0x0, u32::MAX)]), + }; + let function_stack = vec![function]; + self.process_symbolic_functions(&session, function_stack); } for symbol in object.symbols() { @@ -456,6 +471,321 @@ impl<'a> SymCacheConverter<'a> { } } + /// Processes an individual [`Function`], adding its line information to the converter. + /// + /// `call_locations` is a non-empty sorted list of `(address, call_location index)` pairs. + fn process_symbolic_functions( + &mut self, + tr: &dyn TypeResolver, + mut function_stack: Vec>, + ) { + while let Some(in_progress_function) = function_stack.pop() { + let function = in_progress_function.function; + let base_idx = in_progress_function.base_index; + let fn_depth = in_progress_function.depth; + let call_locations = in_progress_function.call_locations; + + // skip over empty functions or functions whose address is too large to fit in a u32 + if function.size == 0 || function.address > u32::MAX as u64 { + return; + } + + let comp_dir = std::str::from_utf8(function.compilation_dir).ok(); + + let entry_pc = if function.inline { + u32::MAX + } else { + function.address as u32 + }; + + let function_idx = { + let language = function.name.language(); + let mut function = transform::Function { + name: function.name.as_str().into(), + comp_dir: comp_dir.map(Into::into), + }; + for transformer in &mut self.transformers.0 { + function = transformer.transform_function(function); + } + + let function_name = if self.is_windows_object { + undecorate_win_symbol(&function.name) + } else { + &function.name + }; + + let name_offset = self.string_table.insert(function_name) as u32; + + let lang = language as u32; + let (fun_idx, _) = self.functions.insert_full(v9::raw::Function { + name_offset, + _comp_dir_offset: u32::MAX, + entry_pc, + lang, + }); + fun_idx as u32 + }; + + let base_idx = match fn_depth { + // If the depth is zero, the current function is a 'base' function, + // it has not been inlined into another function. + 0 => function_idx, + // If the depth is non-zero, it means were are N levels deep in resolving + // inlinees. So keep the existing base. + _ => base_idx, + }; + self.process_symbolic_variables(tr, function, base_idx, fn_depth); + + // We can divide the instructions in a function into two buckets: + // (1) Instructions which are part of an inlined function call, and + // (2) instructions which are *not* part of an inlined function call. + // + // Our incoming line records cover both (1) and (2) types of instructions. + // + // Let's call the address ranges of these instructions (1) inlinee ranges and (2) self ranges. + // + // We use the following strategy: For each function, only insert that function's "self ranges" + // into `self.ranges`. Then recurse into the function's inlinees. Those will insert their + // own "self ranges". Once the entire tree has been traversed, `self.ranges` will contain + // entries from all levels. + // + // In order to compute this function's "self ranges", we first gather and sort its + // "inlinee ranges". Later, when we iterate over this function's lines, we will compute the + // "self ranges" from the gaps between the "inlinee ranges". + + let mut inlinee_ranges = Vec::new(); + for inlinee in &function.inlinees { + for line in &inlinee.lines { + let (start, end) = line_boundaries(line.address, line.size); + inlinee_ranges.push(start..end); + } + } + inlinee_ranges.sort_unstable_by_key(|range| range.start); + + // Walk three iterators. All of these are already sorted by address. + let mut line_iter = function.lines.iter(); + let mut call_location_iter = call_locations.iter(); + let mut inline_iter = inlinee_ranges.into_iter(); + + // call_locations is non-empty, so the first element always exists. + let mut current_call_location = call_location_iter.next().unwrap(); + + let mut next_call_location = call_location_iter.next(); + let mut next_line = line_iter.next(); + let mut next_inline = inline_iter.next(); + + // This will be the list we pass to our inlinees as the call_locations argument. + // This list is ordered by address by construction. + let mut callee_call_locations = Vec::new(); + + let string_table = &mut self.string_table; + + // Iterate over the line records. + while let Some(line) = next_line.take() { + let (line_range_start, line_range_end) = line_boundaries(line.address, line.size); + + // Find the call location for this line. + while next_call_location.is_some() + && next_call_location.unwrap().0 <= line_range_start + { + current_call_location = next_call_location.unwrap(); + next_call_location = call_location_iter.next(); + } + let inlined_into_idx = current_call_location.1; + + let mut location = transform::SourceLocation { + file: transform::File { + name: line.file.name_str(), + directory: Some(line.file.dir_str()), + comp_dir: comp_dir.map(Into::into), + srcsrv_name: line.file.srcsrv_name_str(), + srcsrv_dir: line.file.srcsrv_dir_str(), + srcsrv_revision: line.file.srcsrv_revision().map(|s| s.into()), + }, + line: line.line as u32, + }; + for transformer in &mut self.transformers.0 { + location = transformer.transform_source_location(location); + } + + let name_offset = string_table.insert(&location.file.name) as u32; + let directory_offset = location + .file + .directory + .map_or(u32::MAX, |d| string_table.insert(&d) as u32); + let comp_dir_offset = location + .file + .comp_dir + .map_or(u32::MAX, |cd| string_table.insert(&cd) as u32); + let srcsrv_name_offset = location + .file + .srcsrv_name + .map_or(u32::MAX, |r| string_table.insert(&r) as u32); + let srcsrv_dir_offset = location + .file + .srcsrv_dir + .map_or(u32::MAX, |r| string_table.insert(&r) as u32); + let srcsrv_revision_offset = location + .file + .srcsrv_revision + .map_or(u32::MAX, |r| string_table.insert(&r) as u32); + + let (file_idx, _) = self.files.insert_full(v9::raw::File { + name_offset, + directory_offset, + comp_dir_offset, + srcsrv_name_offset, + srcsrv_dir_offset, + srcsrv_revision_offset, + }); + + let source_location = v9::raw::SourceLocation { + file_idx: file_idx as u32, + line: location.line, + function_idx, + inlined_into_idx, + }; + + // The current line can be a "self line", or a "call line", or even a mixture. + // + // Examples: + // + // a) Just self line: + // Line: |==============| + // Inlinee ranges: (none) + // + // Effect: insert_range + // + // b) Just call line: + // Line: |==============| + // Inlinee ranges: |--------------| + // + // Effect: make_call_location + // + // c) Just call line, for multiple inlined calls: + // Line: |==========================| + // Inlinee ranges: |----------||--------------| + // + // Effect: make_call_location, make_call_location + // + // d) Call line and trailing self line: + // Line: |==================| + // Inlinee ranges: |-----------| + // + // Effect: make_call_location, insert_range + // + // e) Leading self line and also call line: + // Line: |==================| + // Inlinee ranges: |-----------| + // + // Effect: insert_range, make_call_location + // + // f) Interleaving + // Line: |======================================| + // Inlinee ranges: |-----------| |-------| + // + // Effect: insert_range, make_call_location, insert_range, make_call_location, insert_range + // + // g) Bad debug info + // Line: |=======| + // Inlinee ranges: |-------------| + // + // Effect: make_call_location + + let mut current_address = line_range_start; + while current_address < line_range_end { + // Emit our source location at current_address if current_address is not covered by an inlinee. + if next_inline + .as_ref() + .is_none_or(|next| next.start > current_address) + { + // "insert_range" + self.ranges.insert(current_address, source_location.clone()); + } + + // If there is an inlinee range covered by this line record, turn this line into that + // call's "call line". Make a `call_location_idx` for it and store it in `callee_call_locations`. + if let Some(inline_range) = + take_if(&mut next_inline, |next| next.start < line_range_end) + { + // "make_call_location" + let (call_location_idx, _) = + self.call_locations.insert_full(source_location.clone()); + callee_call_locations.push((inline_range.start, call_location_idx as u32)); + + // Advance current_address to the end of this inlinee range. + current_address = inline_range.end; + next_inline = inline_iter.next(); + } else { + // No further inlinee ranges are overlapping with this line record. Advance to the + // end of the line record. + current_address = line_range_end; + } + } + + // Advance the line iterator. + next_line = line_iter.next(); + + // Skip any lines that start before current_address. + // Such lines can exist if the debug information is faulty, or if the compiler created + // multiple identical small "call line" records instead of one combined record + // covering the entire inlinee range. We can't have different "call lines" for a single + // inlinee range anyway, so it's fine to skip these. + while next_line + .as_ref() + .is_some_and(|next| (next.address as u32) < current_address) + { + next_line = line_iter.next(); + } + } + + if !function.inline { + // add the bare minimum of information for the function if there isn't any. + insert_source_location(&mut self.ranges, entry_pc, || v9::raw::SourceLocation { + file_idx: u32::MAX, + line: 0, + function_idx, + inlined_into_idx: u32::MAX, + }); + } + + // We've processed all address ranges which are *not* covered by inlinees. + // Now it's time to recurse. + // Process our inlinees. + if !callee_call_locations.is_empty() { + let callee_call_locations = Rc::new(callee_call_locations); + for inlinee in function.inlinees.iter().rev() { + let function = InProgressFunction { + function: inlinee, + base_index: base_idx, + depth: fn_depth + 1, + call_locations: callee_call_locations.clone(), + }; + function_stack.push(function); + } + } + + let function_end = function.end_address() as u32; + let last_addr = self.last_addr.get_or_insert(0); + if function_end > *last_addr { + *last_addr = function_end; + } + + // Insert an explicit "empty" mapping for the end of the function. + // This is to ensure that addresses that fall "between" functions don't get + // erroneously mapped to the previous function. + // + // We only do this if there is no previous mapping for the end address—we don't + // want to overwrite valid mappings. + // + // If the next function starts right at this function's end, that's no trouble, + // it will just overwrite this mapping with one of its ranges. + if let btree_map::Entry::Vacant(vacant_entry) = self.ranges.entry(function_end) { + vacant_entry.insert(v9::raw::NO_SOURCE_LOCATION); + } + } + } + /// Collects all variables from a [`Function`]. /// /// This takes the current `function`, which may have been inlined into an `outer` function. diff --git a/symbolic-symcache/tests/breakpad.rs b/symbolic-symcache/tests/breakpad.rs index d19a2a66e..1f6b55ed5 100644 --- a/symbolic-symcache/tests/breakpad.rs +++ b/symbolic-symcache/tests/breakpad.rs @@ -167,7 +167,7 @@ FUNC 1000 2000 0 outer let limit = 512; let mut opts = ParseObjectOptions::default(); - opts.max_inline_depth = limit; + opts.max_function_parse_depth = limit; let breakpad = Object::parse_with_opts(sym.as_bytes(), opts).unwrap(); let mut buffer = Vec::new(); From 7599615de1fc9fd4ae5f1d30f0ab8ba48191a852 Mon Sep 17 00:00:00 2001 From: Chris Klochek Date: Mon, 31 Aug 2026 13:23:31 +0200 Subject: [PATCH 2/5] fixes --- symbolic-debuginfo/src/dwarf/mod.rs | 11 +++++++++-- symbolic-symcache/src/writer.rs | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/symbolic-debuginfo/src/dwarf/mod.rs b/symbolic-debuginfo/src/dwarf/mod.rs index e9845d90d..347fb3568 100644 --- a/symbolic-debuginfo/src/dwarf/mod.rs +++ b/symbolic-debuginfo/src/dwarf/mod.rs @@ -1078,6 +1078,7 @@ impl<'d, 'a> DwarfUnit<'d, 'a> { &self, index: usize, depth: isize, + max_parse_depth: usize, dw_die_offset: gimli::UnitOffset, entries: &mut EntriesRaw<'d, '_>, abbrev: &gimli::Abbreviation, @@ -1154,10 +1155,15 @@ impl<'d, 'a> DwarfUnit<'d, 'a> { .map(|range| { let address = offset(range.begin, self.inner.info.address_offset); let size = range.end - range.begin; - // TODO: remove inline depth limit ( *range, - FunctionBuilder::new(name.clone(), self.compilation_dir(), address, size, 999), + FunctionBuilder::new( + name.clone(), + self.compilation_dir(), + address, + size, + max_parse_depth as u32, + ), ) }) .collect(); @@ -1274,6 +1280,7 @@ impl<'d, 'a> DwarfUnit<'d, 'a> { let program = self.consume_subprogram_tag( function_stack.len(), next_depth, + max_parse_depth, dw_die_offset, entries, abbrev, diff --git a/symbolic-symcache/src/writer.rs b/symbolic-symcache/src/writer.rs index 07dfd4b90..d0313da39 100644 --- a/symbolic-symcache/src/writer.rs +++ b/symbolic-symcache/src/writer.rs @@ -487,7 +487,7 @@ impl<'a> SymCacheConverter<'a> { // skip over empty functions or functions whose address is too large to fit in a u32 if function.size == 0 || function.address > u32::MAX as u64 { - return; + continue; } let comp_dir = std::str::from_utf8(function.compilation_dir).ok(); From 3cce2fb000a5ec0a1d68cd08b0bffdb1ae907857 Mon Sep 17 00:00:00 2001 From: Chris Klochek Date: Mon, 31 Aug 2026 14:57:30 +0200 Subject: [PATCH 3/5] feedback --- symbolic-debuginfo/src/object.rs | 2 +- .../tests/test_recursion_limits.rs | 2 +- symbolic-symcache/src/writer.rs | 335 +----------------- 3 files changed, 15 insertions(+), 324 deletions(-) diff --git a/symbolic-debuginfo/src/object.rs b/symbolic-debuginfo/src/object.rs index 03eb8a4dd..722a70155 100644 --- a/symbolic-debuginfo/src/object.rs +++ b/symbolic-debuginfo/src/object.rs @@ -126,7 +126,7 @@ impl Error for ObjectError { } } -const MAX_FUNCTION_PARSE_DEPTH_DEFAULT: u32 = 2000; +const MAX_FUNCTION_PARSE_DEPTH_DEFAULT: u32 = 800; /// Options for parsing object files. #[non_exhaustive] diff --git a/symbolic-debuginfo/tests/test_recursion_limits.rs b/symbolic-debuginfo/tests/test_recursion_limits.rs index 88e41d572..76fdd4af8 100644 --- a/symbolic-debuginfo/tests/test_recursion_limits.rs +++ b/symbolic-debuginfo/tests/test_recursion_limits.rs @@ -1,5 +1,5 @@ use std::assert_matches; -use symbolic_debuginfo::{Object}; +use symbolic_debuginfo::Object; #[test] fn test_resolve_function() { diff --git a/symbolic-symcache/src/writer.rs b/symbolic-symcache/src/writer.rs index d0313da39..be3761d9d 100644 --- a/symbolic-symcache/src/writer.rs +++ b/symbolic-symcache/src/writer.rs @@ -153,337 +153,28 @@ impl<'a> SymCacheConverter<'a> { /// Processes an individual [`Function`], adding its line information to the converter. pub fn process_symbolic_function(&mut self, function: &Function<'_>) { - self.process_symbolic_function_recursive( - &NoopTypeResolver, - function, - &[(0x0, u32::MAX)], - 0, - 0, - ); - } - - /// Processes an individual [`Function`], adding its line information to the converter. - /// - /// `call_locations` is a non-empty sorted list of `(address, call_location index)` pairs. - fn process_symbolic_function_recursive( - &mut self, - tr: &dyn TypeResolver, - function: &Function<'_>, - call_locations: &[(u32, u32)], - base_idx: u32, - fn_depth: u16, - ) { - // skip over empty functions or functions whose address is too large to fit in a u32 - if function.size == 0 || function.address > u32::MAX as u64 { - return; - } - - let comp_dir = std::str::from_utf8(function.compilation_dir).ok(); - - let entry_pc = if function.inline { - u32::MAX - } else { - function.address as u32 + let function = InProgressFunction { + function: &function, + base_index: 0, + depth: 0, + call_locations: Rc::new(vec![(0x0, u32::MAX)]), }; - - let function_idx = { - let language = function.name.language(); - let mut function = transform::Function { - name: function.name.as_str().into(), - comp_dir: comp_dir.map(Into::into), - }; - for transformer in &mut self.transformers.0 { - function = transformer.transform_function(function); - } - - let function_name = if self.is_windows_object { - undecorate_win_symbol(&function.name) - } else { - &function.name - }; - - let name_offset = self.string_table.insert(function_name) as u32; - - let lang = language as u32; - let (fun_idx, _) = self.functions.insert_full(v9::raw::Function { - name_offset, - _comp_dir_offset: u32::MAX, - entry_pc, - lang, - }); - fun_idx as u32 - }; - - let base_idx = match fn_depth { - // If the depth is zero, the current function is a 'base' function, - // it has not been inlined into another function. - 0 => function_idx, - // If the depth is non-zero, it means were are N levels deep in resolving - // inlinees. So keep the existing base. - _ => base_idx, - }; - self.process_symbolic_variables(tr, function, base_idx, fn_depth); - - // We can divide the instructions in a function into two buckets: - // (1) Instructions which are part of an inlined function call, and - // (2) instructions which are *not* part of an inlined function call. - // - // Our incoming line records cover both (1) and (2) types of instructions. - // - // Let's call the address ranges of these instructions (1) inlinee ranges and (2) self ranges. - // - // We use the following strategy: For each function, only insert that function's "self ranges" - // into `self.ranges`. Then recurse into the function's inlinees. Those will insert their - // own "self ranges". Once the entire tree has been traversed, `self.ranges` will contain - // entries from all levels. - // - // In order to compute this function's "self ranges", we first gather and sort its - // "inlinee ranges". Later, when we iterate over this function's lines, we will compute the - // "self ranges" from the gaps between the "inlinee ranges". - - let mut inlinee_ranges = Vec::new(); - for inlinee in &function.inlinees { - for line in &inlinee.lines { - let (start, end) = line_boundaries(line.address, line.size); - inlinee_ranges.push(start..end); - } - } - inlinee_ranges.sort_unstable_by_key(|range| range.start); - - // Walk three iterators. All of these are already sorted by address. - let mut line_iter = function.lines.iter(); - let mut call_location_iter = call_locations.iter(); - let mut inline_iter = inlinee_ranges.into_iter(); - - // call_locations is non-empty, so the first element always exists. - let mut current_call_location = call_location_iter.next().unwrap(); - - let mut next_call_location = call_location_iter.next(); - let mut next_line = line_iter.next(); - let mut next_inline = inline_iter.next(); - - // This will be the list we pass to our inlinees as the call_locations argument. - // This list is ordered by address by construction. - let mut callee_call_locations = Vec::new(); - - let string_table = &mut self.string_table; - - // Iterate over the line records. - while let Some(line) = next_line.take() { - let (line_range_start, line_range_end) = line_boundaries(line.address, line.size); - - // Find the call location for this line. - while next_call_location.is_some() && next_call_location.unwrap().0 <= line_range_start - { - current_call_location = next_call_location.unwrap(); - next_call_location = call_location_iter.next(); - } - let inlined_into_idx = current_call_location.1; - - let mut location = transform::SourceLocation { - file: transform::File { - name: line.file.name_str(), - directory: Some(line.file.dir_str()), - comp_dir: comp_dir.map(Into::into), - srcsrv_name: line.file.srcsrv_name_str(), - srcsrv_dir: line.file.srcsrv_dir_str(), - srcsrv_revision: line.file.srcsrv_revision().map(|s| s.into()), - }, - line: line.line as u32, - }; - for transformer in &mut self.transformers.0 { - location = transformer.transform_source_location(location); - } - - let name_offset = string_table.insert(&location.file.name) as u32; - let directory_offset = location - .file - .directory - .map_or(u32::MAX, |d| string_table.insert(&d) as u32); - let comp_dir_offset = location - .file - .comp_dir - .map_or(u32::MAX, |cd| string_table.insert(&cd) as u32); - let srcsrv_name_offset = location - .file - .srcsrv_name - .map_or(u32::MAX, |r| string_table.insert(&r) as u32); - let srcsrv_dir_offset = location - .file - .srcsrv_dir - .map_or(u32::MAX, |r| string_table.insert(&r) as u32); - let srcsrv_revision_offset = location - .file - .srcsrv_revision - .map_or(u32::MAX, |r| string_table.insert(&r) as u32); - - let (file_idx, _) = self.files.insert_full(v9::raw::File { - name_offset, - directory_offset, - comp_dir_offset, - srcsrv_name_offset, - srcsrv_dir_offset, - srcsrv_revision_offset, - }); - - let source_location = v9::raw::SourceLocation { - file_idx: file_idx as u32, - line: location.line, - function_idx, - inlined_into_idx, - }; - - // The current line can be a "self line", or a "call line", or even a mixture. - // - // Examples: - // - // a) Just self line: - // Line: |==============| - // Inlinee ranges: (none) - // - // Effect: insert_range - // - // b) Just call line: - // Line: |==============| - // Inlinee ranges: |--------------| - // - // Effect: make_call_location - // - // c) Just call line, for multiple inlined calls: - // Line: |==========================| - // Inlinee ranges: |----------||--------------| - // - // Effect: make_call_location, make_call_location - // - // d) Call line and trailing self line: - // Line: |==================| - // Inlinee ranges: |-----------| - // - // Effect: make_call_location, insert_range - // - // e) Leading self line and also call line: - // Line: |==================| - // Inlinee ranges: |-----------| - // - // Effect: insert_range, make_call_location - // - // f) Interleaving - // Line: |======================================| - // Inlinee ranges: |-----------| |-------| - // - // Effect: insert_range, make_call_location, insert_range, make_call_location, insert_range - // - // g) Bad debug info - // Line: |=======| - // Inlinee ranges: |-------------| - // - // Effect: make_call_location - - let mut current_address = line_range_start; - while current_address < line_range_end { - // Emit our source location at current_address if current_address is not covered by an inlinee. - if next_inline - .as_ref() - .is_none_or(|next| next.start > current_address) - { - // "insert_range" - self.ranges.insert(current_address, source_location.clone()); - } - - // If there is an inlinee range covered by this line record, turn this line into that - // call's "call line". Make a `call_location_idx` for it and store it in `callee_call_locations`. - if let Some(inline_range) = - take_if(&mut next_inline, |next| next.start < line_range_end) - { - // "make_call_location" - let (call_location_idx, _) = - self.call_locations.insert_full(source_location.clone()); - callee_call_locations.push((inline_range.start, call_location_idx as u32)); - - // Advance current_address to the end of this inlinee range. - current_address = inline_range.end; - next_inline = inline_iter.next(); - } else { - // No further inlinee ranges are overlapping with this line record. Advance to the - // end of the line record. - current_address = line_range_end; - } - } - - // Advance the line iterator. - next_line = line_iter.next(); - - // Skip any lines that start before current_address. - // Such lines can exist if the debug information is faulty, or if the compiler created - // multiple identical small "call line" records instead of one combined record - // covering the entire inlinee range. We can't have different "call lines" for a single - // inlinee range anyway, so it's fine to skip these. - while next_line - .as_ref() - .is_some_and(|next| (next.address as u32) < current_address) - { - next_line = line_iter.next(); - } - } - - if !function.inline { - // add the bare minimum of information for the function if there isn't any. - insert_source_location(&mut self.ranges, entry_pc, || v9::raw::SourceLocation { - file_idx: u32::MAX, - line: 0, - function_idx, - inlined_into_idx: u32::MAX, - }); - } - - // We've processed all address ranges which are *not* covered by inlinees. - // Now it's time to recurse. - // Process our inlinees. - if !callee_call_locations.is_empty() { - for inlinee in &function.inlinees { - self.process_symbolic_function_recursive( - tr, - inlinee, - &callee_call_locations, - base_idx, - fn_depth + 1, - ); - } - } - - let function_end = function.end_address() as u32; - let last_addr = self.last_addr.get_or_insert(0); - if function_end > *last_addr { - *last_addr = function_end; - } - - // Insert an explicit "empty" mapping for the end of the function. - // This is to ensure that addresses that fall "between" functions don't get - // erroneously mapped to the previous function. - // - // We only do this if there is no previous mapping for the end address—we don't - // want to overwrite valid mappings. - // - // If the next function starts right at this function's end, that's no trouble, - // it will just overwrite this mapping with one of its ranges. - if let btree_map::Entry::Vacant(vacant_entry) = self.ranges.entry(function_end) { - vacant_entry.insert(v9::raw::NO_SOURCE_LOCATION); - } + self.process_symbolic_functions(&NoopTypeResolver, vec![function]); } - /// Processes an individual [`Function`], adding its line information to the converter. - /// - /// `call_locations` is a non-empty sorted list of `(address, call_location index)` pairs. + /// Processes a stack of [`Function`]s, adding their line information to the converter. fn process_symbolic_functions( &mut self, tr: &dyn TypeResolver, mut function_stack: Vec>, ) { while let Some(in_progress_function) = function_stack.pop() { - let function = in_progress_function.function; - let base_idx = in_progress_function.base_index; - let fn_depth = in_progress_function.depth; - let call_locations = in_progress_function.call_locations; + let InProgressFunction { + function, + base_index: base_idx, + depth: fn_depth, + call_locations, + } = in_progress_function; // skip over empty functions or functions whose address is too large to fit in a u32 if function.size == 0 || function.address > u32::MAX as u64 { From 0ce9104a54b284173869e5aee785a2c7989be3a7 Mon Sep 17 00:00:00 2001 From: Chris Klochek Date: Mon, 31 Aug 2026 14:59:13 +0200 Subject: [PATCH 4/5] unstage symcache writer work --- symbolic-symcache/src/writer.rs | 581 +++++++++++++++----------------- 1 file changed, 280 insertions(+), 301 deletions(-) diff --git a/symbolic-symcache/src/writer.rs b/symbolic-symcache/src/writer.rs index be3761d9d..eb9029171 100644 --- a/symbolic-symcache/src/writer.rs +++ b/symbolic-symcache/src/writer.rs @@ -3,7 +3,6 @@ use std::collections::BTreeMap; use std::collections::btree_map; use std::io::Write; -use std::rc::Rc; use indexmap::IndexSet; use symbolic_common::{Arch, DebugId}; @@ -70,13 +69,6 @@ pub struct SymCacheConverter<'a> { last_addr: Option, } -struct InProgressFunction<'a> { - function: &'a Function<'a>, - base_index: u32, - depth: u16, - call_locations: Rc>, -} - impl<'a> SymCacheConverter<'a> { /// Creates a new Converter. pub fn new() -> Self { @@ -132,14 +124,7 @@ impl<'a> SymCacheConverter<'a> { for function in session.functions() { let function = function.map_err(|e| Error::new(ErrorKind::BadDebugFile, e))?; - let function = InProgressFunction { - function: &function, - base_index: 0, - depth: 0, - call_locations: Rc::new(vec![(0x0, u32::MAX)]), - }; - let function_stack = vec![function]; - self.process_symbolic_functions(&session, function_stack); + self.process_symbolic_function_recursive(&session, &function, &[(0x0, u32::MAX)], 0, 0); } for symbol in object.symbols() { @@ -153,328 +138,322 @@ impl<'a> SymCacheConverter<'a> { /// Processes an individual [`Function`], adding its line information to the converter. pub fn process_symbolic_function(&mut self, function: &Function<'_>) { - let function = InProgressFunction { - function: &function, - base_index: 0, - depth: 0, - call_locations: Rc::new(vec![(0x0, u32::MAX)]), - }; - self.process_symbolic_functions(&NoopTypeResolver, vec![function]); + self.process_symbolic_function_recursive( + &NoopTypeResolver, + function, + &[(0x0, u32::MAX)], + 0, + 0, + ); } - /// Processes a stack of [`Function`]s, adding their line information to the converter. - fn process_symbolic_functions( + /// Processes an individual [`Function`], adding its line information to the converter. + /// + /// `call_locations` is a non-empty sorted list of `(address, call_location index)` pairs. + fn process_symbolic_function_recursive( &mut self, tr: &dyn TypeResolver, - mut function_stack: Vec>, + function: &Function<'_>, + call_locations: &[(u32, u32)], + base_idx: u32, + fn_depth: u16, ) { - while let Some(in_progress_function) = function_stack.pop() { - let InProgressFunction { - function, - base_index: base_idx, - depth: fn_depth, - call_locations, - } = in_progress_function; - - // skip over empty functions or functions whose address is too large to fit in a u32 - if function.size == 0 || function.address > u32::MAX as u64 { - continue; - } - - let comp_dir = std::str::from_utf8(function.compilation_dir).ok(); - - let entry_pc = if function.inline { - u32::MAX - } else { - function.address as u32 - }; - - let function_idx = { - let language = function.name.language(); - let mut function = transform::Function { - name: function.name.as_str().into(), - comp_dir: comp_dir.map(Into::into), - }; - for transformer in &mut self.transformers.0 { - function = transformer.transform_function(function); - } + // skip over empty functions or functions whose address is too large to fit in a u32 + if function.size == 0 || function.address > u32::MAX as u64 { + return; + } - let function_name = if self.is_windows_object { - undecorate_win_symbol(&function.name) - } else { - &function.name - }; + let comp_dir = std::str::from_utf8(function.compilation_dir).ok(); - let name_offset = self.string_table.insert(function_name) as u32; + let entry_pc = if function.inline { + u32::MAX + } else { + function.address as u32 + }; - let lang = language as u32; - let (fun_idx, _) = self.functions.insert_full(v9::raw::Function { - name_offset, - _comp_dir_offset: u32::MAX, - entry_pc, - lang, - }); - fun_idx as u32 + let function_idx = { + let language = function.name.language(); + let mut function = transform::Function { + name: function.name.as_str().into(), + comp_dir: comp_dir.map(Into::into), }; + for transformer in &mut self.transformers.0 { + function = transformer.transform_function(function); + } - let base_idx = match fn_depth { - // If the depth is zero, the current function is a 'base' function, - // it has not been inlined into another function. - 0 => function_idx, - // If the depth is non-zero, it means were are N levels deep in resolving - // inlinees. So keep the existing base. - _ => base_idx, + let function_name = if self.is_windows_object { + undecorate_win_symbol(&function.name) + } else { + &function.name }; - self.process_symbolic_variables(tr, function, base_idx, fn_depth); - // We can divide the instructions in a function into two buckets: - // (1) Instructions which are part of an inlined function call, and - // (2) instructions which are *not* part of an inlined function call. - // - // Our incoming line records cover both (1) and (2) types of instructions. - // - // Let's call the address ranges of these instructions (1) inlinee ranges and (2) self ranges. - // - // We use the following strategy: For each function, only insert that function's "self ranges" - // into `self.ranges`. Then recurse into the function's inlinees. Those will insert their - // own "self ranges". Once the entire tree has been traversed, `self.ranges` will contain - // entries from all levels. - // - // In order to compute this function's "self ranges", we first gather and sort its - // "inlinee ranges". Later, when we iterate over this function's lines, we will compute the - // "self ranges" from the gaps between the "inlinee ranges". + let name_offset = self.string_table.insert(function_name) as u32; - let mut inlinee_ranges = Vec::new(); - for inlinee in &function.inlinees { - for line in &inlinee.lines { - let (start, end) = line_boundaries(line.address, line.size); - inlinee_ranges.push(start..end); - } - } - inlinee_ranges.sort_unstable_by_key(|range| range.start); + let lang = language as u32; + let (fun_idx, _) = self.functions.insert_full(v9::raw::Function { + name_offset, + _comp_dir_offset: u32::MAX, + entry_pc, + lang, + }); + fun_idx as u32 + }; - // Walk three iterators. All of these are already sorted by address. - let mut line_iter = function.lines.iter(); - let mut call_location_iter = call_locations.iter(); - let mut inline_iter = inlinee_ranges.into_iter(); + let base_idx = match fn_depth { + // If the depth is zero, the current function is a 'base' function, + // it has not been inlined into another function. + 0 => function_idx, + // If the depth is non-zero, it means were are N levels deep in resolving + // inlinees. So keep the existing base. + _ => base_idx, + }; + self.process_symbolic_variables(tr, function, base_idx, fn_depth); - // call_locations is non-empty, so the first element always exists. - let mut current_call_location = call_location_iter.next().unwrap(); + // We can divide the instructions in a function into two buckets: + // (1) Instructions which are part of an inlined function call, and + // (2) instructions which are *not* part of an inlined function call. + // + // Our incoming line records cover both (1) and (2) types of instructions. + // + // Let's call the address ranges of these instructions (1) inlinee ranges and (2) self ranges. + // + // We use the following strategy: For each function, only insert that function's "self ranges" + // into `self.ranges`. Then recurse into the function's inlinees. Those will insert their + // own "self ranges". Once the entire tree has been traversed, `self.ranges` will contain + // entries from all levels. + // + // In order to compute this function's "self ranges", we first gather and sort its + // "inlinee ranges". Later, when we iterate over this function's lines, we will compute the + // "self ranges" from the gaps between the "inlinee ranges". + + let mut inlinee_ranges = Vec::new(); + for inlinee in &function.inlinees { + for line in &inlinee.lines { + let (start, end) = line_boundaries(line.address, line.size); + inlinee_ranges.push(start..end); + } + } + inlinee_ranges.sort_unstable_by_key(|range| range.start); - let mut next_call_location = call_location_iter.next(); - let mut next_line = line_iter.next(); - let mut next_inline = inline_iter.next(); + // Walk three iterators. All of these are already sorted by address. + let mut line_iter = function.lines.iter(); + let mut call_location_iter = call_locations.iter(); + let mut inline_iter = inlinee_ranges.into_iter(); - // This will be the list we pass to our inlinees as the call_locations argument. - // This list is ordered by address by construction. - let mut callee_call_locations = Vec::new(); + // call_locations is non-empty, so the first element always exists. + let mut current_call_location = call_location_iter.next().unwrap(); - let string_table = &mut self.string_table; + let mut next_call_location = call_location_iter.next(); + let mut next_line = line_iter.next(); + let mut next_inline = inline_iter.next(); - // Iterate over the line records. - while let Some(line) = next_line.take() { - let (line_range_start, line_range_end) = line_boundaries(line.address, line.size); + // This will be the list we pass to our inlinees as the call_locations argument. + // This list is ordered by address by construction. + let mut callee_call_locations = Vec::new(); - // Find the call location for this line. - while next_call_location.is_some() - && next_call_location.unwrap().0 <= line_range_start - { - current_call_location = next_call_location.unwrap(); - next_call_location = call_location_iter.next(); - } - let inlined_into_idx = current_call_location.1; - - let mut location = transform::SourceLocation { - file: transform::File { - name: line.file.name_str(), - directory: Some(line.file.dir_str()), - comp_dir: comp_dir.map(Into::into), - srcsrv_name: line.file.srcsrv_name_str(), - srcsrv_dir: line.file.srcsrv_dir_str(), - srcsrv_revision: line.file.srcsrv_revision().map(|s| s.into()), - }, - line: line.line as u32, - }; - for transformer in &mut self.transformers.0 { - location = transformer.transform_source_location(location); - } + let string_table = &mut self.string_table; - let name_offset = string_table.insert(&location.file.name) as u32; - let directory_offset = location - .file - .directory - .map_or(u32::MAX, |d| string_table.insert(&d) as u32); - let comp_dir_offset = location - .file - .comp_dir - .map_or(u32::MAX, |cd| string_table.insert(&cd) as u32); - let srcsrv_name_offset = location - .file - .srcsrv_name - .map_or(u32::MAX, |r| string_table.insert(&r) as u32); - let srcsrv_dir_offset = location - .file - .srcsrv_dir - .map_or(u32::MAX, |r| string_table.insert(&r) as u32); - let srcsrv_revision_offset = location - .file - .srcsrv_revision - .map_or(u32::MAX, |r| string_table.insert(&r) as u32); - - let (file_idx, _) = self.files.insert_full(v9::raw::File { - name_offset, - directory_offset, - comp_dir_offset, - srcsrv_name_offset, - srcsrv_dir_offset, - srcsrv_revision_offset, - }); + // Iterate over the line records. + while let Some(line) = next_line.take() { + let (line_range_start, line_range_end) = line_boundaries(line.address, line.size); - let source_location = v9::raw::SourceLocation { - file_idx: file_idx as u32, - line: location.line, - function_idx, - inlined_into_idx, - }; + // Find the call location for this line. + while next_call_location.is_some() && next_call_location.unwrap().0 <= line_range_start + { + current_call_location = next_call_location.unwrap(); + next_call_location = call_location_iter.next(); + } + let inlined_into_idx = current_call_location.1; - // The current line can be a "self line", or a "call line", or even a mixture. - // - // Examples: - // - // a) Just self line: - // Line: |==============| - // Inlinee ranges: (none) - // - // Effect: insert_range - // - // b) Just call line: - // Line: |==============| - // Inlinee ranges: |--------------| - // - // Effect: make_call_location - // - // c) Just call line, for multiple inlined calls: - // Line: |==========================| - // Inlinee ranges: |----------||--------------| - // - // Effect: make_call_location, make_call_location - // - // d) Call line and trailing self line: - // Line: |==================| - // Inlinee ranges: |-----------| - // - // Effect: make_call_location, insert_range - // - // e) Leading self line and also call line: - // Line: |==================| - // Inlinee ranges: |-----------| - // - // Effect: insert_range, make_call_location - // - // f) Interleaving - // Line: |======================================| - // Inlinee ranges: |-----------| |-------| - // - // Effect: insert_range, make_call_location, insert_range, make_call_location, insert_range - // - // g) Bad debug info - // Line: |=======| - // Inlinee ranges: |-------------| - // - // Effect: make_call_location - - let mut current_address = line_range_start; - while current_address < line_range_end { - // Emit our source location at current_address if current_address is not covered by an inlinee. - if next_inline - .as_ref() - .is_none_or(|next| next.start > current_address) - { - // "insert_range" - self.ranges.insert(current_address, source_location.clone()); - } + let mut location = transform::SourceLocation { + file: transform::File { + name: line.file.name_str(), + directory: Some(line.file.dir_str()), + comp_dir: comp_dir.map(Into::into), + srcsrv_name: line.file.srcsrv_name_str(), + srcsrv_dir: line.file.srcsrv_dir_str(), + srcsrv_revision: line.file.srcsrv_revision().map(|s| s.into()), + }, + line: line.line as u32, + }; + for transformer in &mut self.transformers.0 { + location = transformer.transform_source_location(location); + } - // If there is an inlinee range covered by this line record, turn this line into that - // call's "call line". Make a `call_location_idx` for it and store it in `callee_call_locations`. - if let Some(inline_range) = - take_if(&mut next_inline, |next| next.start < line_range_end) - { - // "make_call_location" - let (call_location_idx, _) = - self.call_locations.insert_full(source_location.clone()); - callee_call_locations.push((inline_range.start, call_location_idx as u32)); - - // Advance current_address to the end of this inlinee range. - current_address = inline_range.end; - next_inline = inline_iter.next(); - } else { - // No further inlinee ranges are overlapping with this line record. Advance to the - // end of the line record. - current_address = line_range_end; - } - } + let name_offset = string_table.insert(&location.file.name) as u32; + let directory_offset = location + .file + .directory + .map_or(u32::MAX, |d| string_table.insert(&d) as u32); + let comp_dir_offset = location + .file + .comp_dir + .map_or(u32::MAX, |cd| string_table.insert(&cd) as u32); + let srcsrv_name_offset = location + .file + .srcsrv_name + .map_or(u32::MAX, |r| string_table.insert(&r) as u32); + let srcsrv_dir_offset = location + .file + .srcsrv_dir + .map_or(u32::MAX, |r| string_table.insert(&r) as u32); + let srcsrv_revision_offset = location + .file + .srcsrv_revision + .map_or(u32::MAX, |r| string_table.insert(&r) as u32); + + let (file_idx, _) = self.files.insert_full(v9::raw::File { + name_offset, + directory_offset, + comp_dir_offset, + srcsrv_name_offset, + srcsrv_dir_offset, + srcsrv_revision_offset, + }); + + let source_location = v9::raw::SourceLocation { + file_idx: file_idx as u32, + line: location.line, + function_idx, + inlined_into_idx, + }; - // Advance the line iterator. - next_line = line_iter.next(); + // The current line can be a "self line", or a "call line", or even a mixture. + // + // Examples: + // + // a) Just self line: + // Line: |==============| + // Inlinee ranges: (none) + // + // Effect: insert_range + // + // b) Just call line: + // Line: |==============| + // Inlinee ranges: |--------------| + // + // Effect: make_call_location + // + // c) Just call line, for multiple inlined calls: + // Line: |==========================| + // Inlinee ranges: |----------||--------------| + // + // Effect: make_call_location, make_call_location + // + // d) Call line and trailing self line: + // Line: |==================| + // Inlinee ranges: |-----------| + // + // Effect: make_call_location, insert_range + // + // e) Leading self line and also call line: + // Line: |==================| + // Inlinee ranges: |-----------| + // + // Effect: insert_range, make_call_location + // + // f) Interleaving + // Line: |======================================| + // Inlinee ranges: |-----------| |-------| + // + // Effect: insert_range, make_call_location, insert_range, make_call_location, insert_range + // + // g) Bad debug info + // Line: |=======| + // Inlinee ranges: |-------------| + // + // Effect: make_call_location - // Skip any lines that start before current_address. - // Such lines can exist if the debug information is faulty, or if the compiler created - // multiple identical small "call line" records instead of one combined record - // covering the entire inlinee range. We can't have different "call lines" for a single - // inlinee range anyway, so it's fine to skip these. - while next_line + let mut current_address = line_range_start; + while current_address < line_range_end { + // Emit our source location at current_address if current_address is not covered by an inlinee. + if next_inline .as_ref() - .is_some_and(|next| (next.address as u32) < current_address) + .is_none_or(|next| next.start > current_address) { - next_line = line_iter.next(); + // "insert_range" + self.ranges.insert(current_address, source_location.clone()); } - } - - if !function.inline { - // add the bare minimum of information for the function if there isn't any. - insert_source_location(&mut self.ranges, entry_pc, || v9::raw::SourceLocation { - file_idx: u32::MAX, - line: 0, - function_idx, - inlined_into_idx: u32::MAX, - }); - } - // We've processed all address ranges which are *not* covered by inlinees. - // Now it's time to recurse. - // Process our inlinees. - if !callee_call_locations.is_empty() { - let callee_call_locations = Rc::new(callee_call_locations); - for inlinee in function.inlinees.iter().rev() { - let function = InProgressFunction { - function: inlinee, - base_index: base_idx, - depth: fn_depth + 1, - call_locations: callee_call_locations.clone(), - }; - function_stack.push(function); + // If there is an inlinee range covered by this line record, turn this line into that + // call's "call line". Make a `call_location_idx` for it and store it in `callee_call_locations`. + if let Some(inline_range) = + take_if(&mut next_inline, |next| next.start < line_range_end) + { + // "make_call_location" + let (call_location_idx, _) = + self.call_locations.insert_full(source_location.clone()); + callee_call_locations.push((inline_range.start, call_location_idx as u32)); + + // Advance current_address to the end of this inlinee range. + current_address = inline_range.end; + next_inline = inline_iter.next(); + } else { + // No further inlinee ranges are overlapping with this line record. Advance to the + // end of the line record. + current_address = line_range_end; } } - let function_end = function.end_address() as u32; - let last_addr = self.last_addr.get_or_insert(0); - if function_end > *last_addr { - *last_addr = function_end; + // Advance the line iterator. + next_line = line_iter.next(); + + // Skip any lines that start before current_address. + // Such lines can exist if the debug information is faulty, or if the compiler created + // multiple identical small "call line" records instead of one combined record + // covering the entire inlinee range. We can't have different "call lines" for a single + // inlinee range anyway, so it's fine to skip these. + while next_line + .as_ref() + .is_some_and(|next| (next.address as u32) < current_address) + { + next_line = line_iter.next(); } + } - // Insert an explicit "empty" mapping for the end of the function. - // This is to ensure that addresses that fall "between" functions don't get - // erroneously mapped to the previous function. - // - // We only do this if there is no previous mapping for the end address—we don't - // want to overwrite valid mappings. - // - // If the next function starts right at this function's end, that's no trouble, - // it will just overwrite this mapping with one of its ranges. - if let btree_map::Entry::Vacant(vacant_entry) = self.ranges.entry(function_end) { - vacant_entry.insert(v9::raw::NO_SOURCE_LOCATION); + if !function.inline { + // add the bare minimum of information for the function if there isn't any. + insert_source_location(&mut self.ranges, entry_pc, || v9::raw::SourceLocation { + file_idx: u32::MAX, + line: 0, + function_idx, + inlined_into_idx: u32::MAX, + }); + } + + // We've processed all address ranges which are *not* covered by inlinees. + // Now it's time to recurse. + // Process our inlinees. + if !callee_call_locations.is_empty() { + for inlinee in &function.inlinees { + self.process_symbolic_function_recursive( + tr, + inlinee, + &callee_call_locations, + base_idx, + fn_depth + 1, + ); } } + + let function_end = function.end_address() as u32; + let last_addr = self.last_addr.get_or_insert(0); + if function_end > *last_addr { + *last_addr = function_end; + } + + // Insert an explicit "empty" mapping for the end of the function. + // This is to ensure that addresses that fall "between" functions don't get + // erroneously mapped to the previous function. + // + // We only do this if there is no previous mapping for the end address—we don't + // want to overwrite valid mappings. + // + // If the next function starts right at this function's end, that's no trouble, + // it will just overwrite this mapping with one of its ranges. + if let btree_map::Entry::Vacant(vacant_entry) = self.ranges.entry(function_end) { + vacant_entry.insert(v9::raw::NO_SOURCE_LOCATION); + } } /// Collects all variables from a [`Function`]. From b3002d6ded8c1434cca8279f81a7047eb3cf0bf0 Mon Sep 17 00:00:00 2001 From: Chris Klochek Date: Tue, 1 Sep 2026 14:38:22 +0200 Subject: [PATCH 5/5] feedback --- symbolic-debuginfo/src/dwarf/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/symbolic-debuginfo/src/dwarf/mod.rs b/symbolic-debuginfo/src/dwarf/mod.rs index 347fb3568..dd930e39f 100644 --- a/symbolic-debuginfo/src/dwarf/mod.rs +++ b/symbolic-debuginfo/src/dwarf/mod.rs @@ -1259,8 +1259,7 @@ impl<'d, 'a> DwarfUnit<'d, 'a> { break; } - let last_func: InProgressSubProgram<'_> = - function_stack.pop().expect("already checked"); + let last_func = function_stack.pop().expect("already checked"); last_func.finish(self, output, &mut function_stack)?; } @@ -1270,9 +1269,10 @@ impl<'d, 'a> DwarfUnit<'d, 'a> { // It's possible the top function is dead-code; if so, we want to ignore anything // nested inside that is NOT a subprogram. - let deadcode_top = function_stack - .last() - .is_some_and(|p| matches!(p, InProgressSubProgram::Deadcode(_))); + let deadcode_top = matches!( + function_stack.last(), + Some(InProgressSubProgram::Deadcode(_)) + ); match abbrev.tag() { // Always process a subprogram, even if we have a deadcode frame at top of stack.