From f90174efab99e7eba504831ead579f179b760dbf Mon Sep 17 00:00:00 2001 From: Philip Craig Date: Mon, 15 Apr 2024 18:27:46 +1000 Subject: [PATCH] Fix clippy warnings --- benches/bench.rs | 8 ++-- src/frame.rs | 1 + src/function.rs | 110 ++++++++++++++++++------------------------- src/lazy.rs | 11 +++-- src/lib.rs | 1 + src/unit.rs | 38 +++++++++------ tests/correctness.rs | 3 +- 7 files changed, 84 insertions(+), 88 deletions(-) diff --git a/benches/bench.rs b/benches/bench.rs index d6ff4a6..24ff271 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -26,23 +26,23 @@ fn with_file)>(target: &path::Path, f: F) { f(&file) } -fn dwarf_load<'a>(object: &object::File<'a>) -> gimli::Dwarf> { +fn dwarf_load<'a>(object: &object::File<'a>) -> gimli::DwarfSections> { let load_section = |id: gimli::SectionId| -> Result, gimli::Error> { Ok(object .section_by_name(id.name()) .map(|section| section.uncompressed_data().unwrap()) .unwrap_or(Cow::Borrowed(&[][..]))) }; - gimli::Dwarf::load(&load_section).unwrap() + gimli::DwarfSections::load(&load_section).unwrap() } fn dwarf_borrow<'a>( - dwarf: &'a gimli::Dwarf>, + dwarf: &'a gimli::DwarfSections>, ) -> gimli::Dwarf> { let borrow_section: &dyn for<'b> Fn( &'b Cow<'_, [u8]>, ) -> gimli::EndianSlice<'b, gimli::LittleEndian> = - &|section| gimli::EndianSlice::new(&*section, gimli::LittleEndian); + &|section| gimli::EndianSlice::new(section, gimli::LittleEndian); dwarf.borrow(&borrow_section) } diff --git a/src/frame.rs b/src/frame.rs index 475d49c..f665eeb 100644 --- a/src/frame.rs +++ b/src/frame.rs @@ -80,6 +80,7 @@ where } /// Advances the iterator and returns the next frame. + #[allow(clippy::should_implement_trait)] pub fn next(&mut self) -> Result>, Error> { let frames = match &mut self.0 { FrameIterState::Empty => return Ok(None), diff --git a/src/function.rs b/src/function.rs index 83458aa..f87bd3d 100644 --- a/src/function.rs +++ b/src/function.rs @@ -2,18 +2,14 @@ use alloc::boxed::Box; use alloc::vec::Vec; use core::cmp::Ordering; -use crate::lazy::LazyCell; +use crate::lazy::LazyResult; use crate::maybe_small; use crate::{Context, DebugFile, Error, RangeAttributes}; pub(crate) struct Functions { /// List of all `DW_TAG_subprogram` details in the unit. - pub(crate) functions: Box< - [( - gimli::UnitOffset, - LazyCell, Error>>, - )], - >, + #[allow(clippy::type_complexity)] + pub(crate) functions: Box<[(gimli::UnitOffset, LazyResult>)]>, /// List of `DW_TAG_subprogram` address ranges in the unit. pub(crate) addresses: Box<[FunctionAddress]>, } @@ -103,13 +99,14 @@ impl Functions { } let function_index = functions.len(); - if ranges.for_each_range(sections, unit, |range| { + let has_address = ranges.for_each_range(sections, unit, |range| { addresses.push(FunctionAddress { range, function: function_index, }); - })? { - functions.push((dw_die_offset, LazyCell::new())); + })?; + if has_address { + functions.push((dw_die_offset, LazyResult::new())); } } else { entries.skip_attributes(abbrev.attributes())?; @@ -205,19 +202,16 @@ impl Function { } } - let mut inlined_functions = Vec::new(); - let mut inlined_addresses = Vec::new(); - Function::parse_children( - &mut entries, - depth, + let mut state = InlinedState { + entries, + inlined_functions: Vec::new(), + inlined_addresses: Vec::new(), file, unit, ctx, sections, - &mut inlined_functions, - &mut inlined_addresses, - 0, - )?; + }; + Function::parse_children(&mut state, depth, 0)?; // Sort ranges in "breadth-first traversal order", i.e. first by call_depth // and then by range.begin. This allows finding the range containing an @@ -229,7 +223,7 @@ impl Function { // In this example, if you want to look up address 7 at depth 0, and you // encounter [0..2 at depth 1], are you before or after the target range? // You don't know. - inlined_addresses.sort_by(|r1, r2| { + state.inlined_addresses.sort_by(|r1, r2| { if r1.call_depth < r2.call_depth { Ordering::Less } else if r1.call_depth > r2.call_depth { @@ -246,50 +240,38 @@ impl Function { Ok(Function { dw_die_offset, name, - inlined_functions: inlined_functions.into_boxed_slice(), - inlined_addresses: inlined_addresses.into_boxed_slice(), + inlined_functions: state.inlined_functions.into_boxed_slice(), + inlined_addresses: state.inlined_addresses.into_boxed_slice(), }) } fn parse_children( - entries: &mut gimli::EntriesRaw<'_, '_, R>, + state: &mut InlinedState, depth: isize, - file: DebugFile, - unit: &gimli::Unit, - ctx: &Context, - sections: &gimli::Dwarf, - inlined_functions: &mut Vec>, - inlined_addresses: &mut Vec, inlined_depth: usize, ) -> Result<(), Error> { loop { - let dw_die_offset = entries.next_offset(); - let next_depth = entries.next_depth(); + let dw_die_offset = state.entries.next_offset(); + let next_depth = state.entries.next_depth(); if next_depth <= depth { return Ok(()); } - if let Some(abbrev) = entries.read_abbreviation()? { + if let Some(abbrev) = state.entries.read_abbreviation()? { match abbrev.tag() { gimli::DW_TAG_subprogram => { - Function::skip(entries, abbrev, next_depth)?; + Function::skip(&mut state.entries, abbrev, next_depth)?; } gimli::DW_TAG_inlined_subroutine => { InlinedFunction::parse( + state, dw_die_offset, - entries, abbrev, next_depth, - file, - unit, - ctx, - sections, - inlined_functions, - inlined_addresses, inlined_depth, )?; } _ => { - entries.skip_attributes(abbrev.attributes())?; + state.entries.skip_attributes(abbrev.attributes())?; } } } @@ -352,25 +334,21 @@ impl Function { impl InlinedFunction { fn parse( + state: &mut InlinedState, dw_die_offset: gimli::UnitOffset, - entries: &mut gimli::EntriesRaw<'_, '_, R>, abbrev: &gimli::Abbreviation, depth: isize, - file: DebugFile, - unit: &gimli::Unit, - ctx: &Context, - sections: &gimli::Dwarf, - inlined_functions: &mut Vec>, - inlined_addresses: &mut Vec, inlined_depth: usize, ) -> Result<(), Error> { + let unit = state.unit; + let sections = state.sections; let mut ranges = RangeAttributes::default(); let mut name = None; let mut call_file = None; let mut call_line = 0; let mut call_column = 0; for spec in abbrev.attributes() { - match entries.read_attribute(*spec) { + match state.entries.read_attribute(*spec) { Ok(ref attr) => match attr.name() { gimli::DW_AT_low_pc => match attr.value() { gimli::AttributeValue::Addr(val) => ranges.low_pc = Some(val), @@ -402,7 +380,8 @@ impl InlinedFunction { } gimli::DW_AT_abstract_origin | gimli::DW_AT_specification => { if name.is_none() { - name = name_attr(attr.value(), file, unit, ctx, sections, 16)?; + name = + name_attr(attr.value(), state.file, unit, state.ctx, sections, 16)?; } } gimli::DW_AT_call_file => { @@ -433,8 +412,8 @@ impl InlinedFunction { } } - let function_index = inlined_functions.len(); - inlined_functions.push(InlinedFunction { + let function_index = state.inlined_functions.len(); + state.inlined_functions.push(InlinedFunction { dw_die_offset, name, call_file, @@ -443,27 +422,30 @@ impl InlinedFunction { }); ranges.for_each_range(sections, unit, |range| { - inlined_addresses.push(InlinedFunctionAddress { + state.inlined_addresses.push(InlinedFunctionAddress { range, call_depth: inlined_depth, function: function_index, }); })?; - Function::parse_children( - entries, - depth, - file, - unit, - ctx, - sections, - inlined_functions, - inlined_addresses, - inlined_depth + 1, - ) + Function::parse_children(state, depth, inlined_depth + 1) } } +struct InlinedState<'a, R: gimli::Reader> { + // Mutable fields. + entries: gimli::EntriesRaw<'a, 'a, R>, + inlined_functions: Vec>, + inlined_addresses: Vec, + + // Constant fields. + file: DebugFile, + unit: &'a gimli::Unit, + ctx: &'a Context, + sections: &'a gimli::Dwarf, +} + fn name_attr( attr: gimli::AttributeValue, mut file: DebugFile, diff --git a/src/lazy.rs b/src/lazy.rs index 2df2ed6..4460b8a 100644 --- a/src/lazy.rs +++ b/src/lazy.rs @@ -1,20 +1,23 @@ use core::cell::UnsafeCell; -pub struct LazyCell { +pub(crate) type LazyResult = LazyCell>; + +pub(crate) struct LazyCell { contents: UnsafeCell>, } + impl LazyCell { - pub fn new() -> LazyCell { + pub(crate) fn new() -> LazyCell { LazyCell { contents: UnsafeCell::new(None), } } - pub fn borrow(&self) -> Option<&T> { + pub(crate) fn borrow(&self) -> Option<&T> { unsafe { &*self.contents.get() }.as_ref() } - pub fn borrow_with(&self, closure: impl FnOnce() -> T) -> &T { + pub(crate) fn borrow_with(&self, closure: impl FnOnce() -> T) -> &T { // First check if we're already initialized... let ptr = self.contents.get(); if let Some(val) = unsafe { &*ptr } { diff --git a/src/lib.rs b/src/lib.rs index 7826121..96274e7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -167,6 +167,7 @@ impl Context { /// Construct a new `Context` from DWARF sections. /// /// This method does not support using a supplementary object file. + #[allow(clippy::too_many_arguments)] pub fn from_sections( debug_abbrev: gimli::DebugAbbrev, debug_addr: gimli::DebugAddr, diff --git a/src/unit.rs b/src/unit.rs index 75006d4..af24e2e 100644 --- a/src/unit.rs +++ b/src/unit.rs @@ -3,7 +3,7 @@ use alloc::sync::Arc; use alloc::vec::Vec; use core::cmp; -use crate::lazy::LazyCell; +use crate::lazy::LazyResult; use crate::{ Context, DebugFile, Error, Function, Functions, LineLocationRangeIter, Lines, Location, LookupContinuation, LookupResult, RangeAttributes, SimpleLookup, SplitDwarfLoad, @@ -19,28 +19,27 @@ pub(crate) struct ResUnit { offset: gimli::DebugInfoOffset, dw_unit: gimli::Unit, pub(crate) lang: Option, - lines: LazyCell>, - funcs: LazyCell, Error>>, - dwo: LazyCell>, gimli::Unit)>>, Error>>, + lines: LazyResult, + funcs: LazyResult>, + dwo: LazyResult>>>, } +type UnitRef<'unit, R> = (DebugFile, &'unit gimli::Dwarf, &'unit gimli::Unit); + impl ResUnit { pub(crate) fn dwarf_and_unit_dwo<'unit, 'ctx: 'unit>( &'unit self, ctx: &'ctx Context, ) -> LookupResult< SimpleLookup< - Result<(DebugFile, &'unit gimli::Dwarf, &'unit gimli::Unit), Error>, + Result, Error>, R, - impl FnOnce( - Option>>, - ) - -> Result<(DebugFile, &'unit gimli::Dwarf, &'unit gimli::Unit), Error>, + impl FnOnce(Option>>) -> Result, Error>, >, > { loop { break SimpleLookup::new_complete(match self.dwo.borrow() { - Some(Ok(Some(v))) => Ok((DebugFile::Dwo, &*v.0, &v.1)), + Some(Ok(Some(dwo))) => Ok((DebugFile::Dwo, &*dwo.sections, &dwo.dw_unit)), Some(Ok(None)) => Ok((DebugFile::Primary, &*ctx.sections, &self.dw_unit)), Some(Err(e)) => Err(*e), None => { @@ -83,7 +82,10 @@ impl ResUnit { let mut dwo_unit = dwo_dwarf.unit(dwo_header)?; dwo_unit.copy_relocated_attributes(&self.dw_unit); - Ok(Some(Box::new((dwo_dwarf, dwo_unit)))) + Ok(Some(Box::new(DwoUnit { + sections: dwo_dwarf, + dw_unit: dwo_unit, + }))) }; return SimpleLookup::new_needs_load( @@ -94,7 +96,7 @@ impl ResUnit { parent: ctx.sections.clone(), }, move |dwo_dwarf| match self.dwo.borrow_with(|| process_dwo(dwo_dwarf)) { - Ok(Some(v)) => Ok((DebugFile::Dwo, &*v.0, &v.1)), + Ok(Some(dwo)) => Ok((DebugFile::Dwo, &*dwo.sections, &dwo.dw_unit)), Ok(None) => Ok((DebugFile::Primary, &*ctx.sections, &self.dw_unit)), Err(e) => Err(*e), }, @@ -341,7 +343,7 @@ impl ResUnits { } } - let lines = LazyCell::new(); + let lines = LazyResult::new(); if !have_unit_range { // The unit did not declare any ranges. // Try to get some ranges from the line program sequences. @@ -366,8 +368,8 @@ impl ResUnits { dw_unit, lang, lines, - funcs: LazyCell::new(), - dwo: LazyCell::new(), + funcs: LazyResult::new(), + dwo: LazyResult::new(), }); } @@ -489,6 +491,12 @@ impl ResUnits { } } +/// A DWO unit has its own DWARF sections. +struct DwoUnit { + sections: Arc>, + dw_unit: gimli::Unit, +} + pub(crate) struct SupUnit { offset: gimli::DebugInfoOffset, dw_unit: gimli::Unit, diff --git a/tests/correctness.rs b/tests/correctness.rs index 9c78175..7cb6c87 100644 --- a/tests/correctness.rs +++ b/tests/correctness.rs @@ -29,10 +29,11 @@ fn find_debuginfo() -> memmap2::Mmap { } } - return map; + map } #[test] +#[allow(clippy::fn_to_numeric_cast)] fn correctness() { let map = find_debuginfo(); let file = &object::File::parse(&*map).unwrap();