Delete DWARF Reader trait, also use gimli::UnitRef (#11883)

* Delete DWARF Reader trait

Use the existing Reader type alias instead. Most places were already
using this.

* Use `gimli::UnitRef`

This reduces parameter counts and makes it harder to use the
wrong DWARF sections for split DWARF.

A drawback is that `UnitRef<'a, R>` is invariant over `R`,
so additional lifetime annotations are needed in some places.

This probably also fixes a bug with the parsing of
`AttributeValue::LocationListsRef`, but I didn't try to test this.

Author: philipc

crates/cranelift/src/debug/gc.rs

@@ -85,9 +85,10 @@ fn build_unit_dependencies(
     deps: &mut Dependencies,
 ) -> read::Result<()> {
     let unit = dwarf.unit(header)?;
+    let unit = unit.unit_ref(dwarf);
     let mut tree = unit.entries_tree(None)?;
     let root = tree.root()?;
-    build_die_dependencies(root, dwarf, &unit, at, deps)?;
+    build_die_dependencies(root, unit, at, deps)?;
     Ok(())
 }
 
@@ -144,20 +145,15 @@ fn has_die_back_edge(die: &read::DebuggingInformationEntry<Reader<'_>>) -> read:
 
 fn has_valid_code_range(
     die: &read::DebuggingInformationEntry<Reader<'_>>,
-    dwarf: &read::Dwarf<Reader<'_>>,
-    unit: &read::Unit<Reader<'_>>,
+    unit: read::UnitRef<Reader<'_>>,
     at: &AddressTransform,
 ) -> read::Result<bool> {
     match die.tag() {
         constants::DW_TAG_subprogram => {
             if let Some(ranges_attr) = die.attr_value(constants::DW_AT_ranges)? {
                 let offset = match ranges_attr {
-                    read::AttributeValue::RangeListsRef(val) => {
-                        dwarf.ranges_offset_from_raw(unit, val)
-                    }
-                    read::AttributeValue::DebugRngListsIndex(index) => {
-                        dwarf.ranges_offset(unit, index)?
-                    }
+                    read::AttributeValue::RangeListsRef(val) => unit.ranges_offset_from_raw(val),
+                    read::AttributeValue::DebugRngListsIndex(index) => unit.ranges_offset(index)?,
                     _ => return Ok(false),
                 };
                 let mut has_valid_base = if let Some(read::AttributeValue::Addr(low_pc)) =
@@ -167,7 +163,7 @@ fn has_valid_code_range(
                 } else {
                     None
                 };
-                let mut it = dwarf.ranges.raw_ranges(offset, unit.encoding())?;
+                let mut it = unit.raw_ranges(offset)?;
                 while let Some(range) = it.next()? {
                     // If at least one of the range addresses can be converted,
                     // declaring code range as valid.
@@ -188,7 +184,7 @@ fn has_valid_code_range(
                         }
                         read::RawRngListEntry::StartxEndx { begin, .. }
                         | read::RawRngListEntry::StartxLength { begin, .. } => {
-                            let addr = dwarf.address(unit, begin)?;
+                            let addr = unit.address(begin)?;
                             if at.can_translate_address(addr) {
                                 return Ok(true);
                             }
@@ -197,7 +193,7 @@ fn has_valid_code_range(
                             has_valid_base = Some(at.can_translate_address(addr));
                         }
                         read::RawRngListEntry::BaseAddressx { addr } => {
-                            let addr = dwarf.address(unit, addr)?;
+                            let addr = unit.address(addr)?;
                             has_valid_base = Some(at.can_translate_address(addr));
                         }
                         read::RawRngListEntry::OffsetPair { .. } => (),
@@ -208,7 +204,7 @@ fn has_valid_code_range(
                 if let read::AttributeValue::Addr(a) = low_pc {
                     return Ok(at.can_translate_address(a));
                 } else if let read::AttributeValue::DebugAddrIndex(i) = low_pc {
-                    let a = dwarf.debug_addr.get_address(4, unit.addr_base, i)?;
+                    let a = unit.address(i)?;
                     return Ok(at.can_translate_address(a));
                 }
             }
@@ -220,45 +216,43 @@ fn has_valid_code_range(
 
 fn build_die_dependencies(
     die: read::EntriesTreeNode<Reader<'_>>,
-    dwarf: &read::Dwarf<Reader<'_>>,
-    unit: &read::Unit<Reader<'_>>,
+    unit: read::UnitRef<Reader<'_>>,
     at: &AddressTransform,
     deps: &mut Dependencies,
 ) -> read::Result<()> {
     let entry = die.entry();
-    let offset = entry.offset().to_unit_section_offset(unit);
+    let offset = entry.offset().to_unit_section_offset(&unit);
     let mut attrs = entry.attrs();
     while let Some(attr) = attrs.next()? {
-        build_attr_dependencies(&attr, offset, dwarf, unit, at, deps)?;
+        build_attr_dependencies(&attr, offset, unit, at, deps)?;
     }
 
     let mut children = die.children();
     while let Some(child) = children.next()? {
         let child_entry = child.entry();
-        let child_offset = child_entry.offset().to_unit_section_offset(unit);
+        let child_offset = child_entry.offset().to_unit_section_offset(&unit);
         deps.add_edge(child_offset, offset);
         if has_die_back_edge(child_entry)? {
             deps.add_edge(offset, child_offset);
         }
-        if has_valid_code_range(child_entry, dwarf, unit, at)? {
+        if has_valid_code_range(child_entry, unit, at)? {
             deps.add_root(child_offset);
         }
-        build_die_dependencies(child, dwarf, unit, at, deps)?;
+        build_die_dependencies(child, unit, at, deps)?;
     }
     Ok(())
 }
 
 fn build_attr_dependencies(
     attr: &read::Attribute<Reader<'_>>,
     offset: UnitSectionOffset,
-    _dwarf: &read::Dwarf<Reader<'_>>,
-    unit: &read::Unit<Reader<'_>>,
+    unit: read::UnitRef<Reader<'_>>,
     _at: &AddressTransform,
     deps: &mut Dependencies,
 ) -> read::Result<()> {
     match attr.value() {
         read::AttributeValue::UnitRef(val) => {
-            let ref_offset = val.to_unit_section_offset(unit);
+            let ref_offset = val.to_unit_section_offset(&unit);
             deps.add_edge(offset, ref_offset);
         }
         read::AttributeValue::DebugInfoRef(val) => {

crates/cranelift/src/debug/transform/attr.rs

@@ -10,7 +10,7 @@ use super::{TransformError, dbi_log};
 use anyhow::{Error, bail};
 use cranelift_codegen::isa::TargetIsa;
 use gimli::{
-    AttributeValue, DebugLineOffset, DebuggingInformationEntry, Dwarf, Unit, UnitOffset, write,
+    AttributeValue, DebugLineOffset, DebuggingInformationEntry, UnitOffset, UnitRef, write,
 };
 
 #[derive(Debug)]
@@ -47,10 +47,9 @@ fn is_exprloc_to_loclist_allowed(attr_name: gimli::constants::DwAt) -> bool {
 }
 
 pub(crate) fn clone_die_attributes<'a>(
-    dwarf: &gimli::Dwarf<Reader<'a>>,
-    unit: &Unit<Reader<'a>>,
+    unit: UnitRef<Reader<'a>>,
     entry: &DebuggingInformationEntry<Reader<'a>>,
-    addr_tr: &'a AddressTransform,
+    addr_tr: &AddressTransform,
     frame_info: Option<&FunctionFrameInfo>,
     out_unit: &mut write::Unit,
     out_entry_id: write::UnitEntryId,
@@ -59,7 +58,7 @@ pub(crate) fn clone_die_attributes<'a>(
     out_strings: &mut write::StringTable,
     pending_die_refs: &mut PendingUnitRefs,
     pending_di_refs: &mut PendingDebugInfoRefs,
-    mut attr_context: EntryAttributesContext<'a>,
+    mut attr_context: EntryAttributesContext<'_>,
     isa: &dyn TargetIsa,
 ) -> Result<(), Error> {
     let unit_encoding = unit.encoding();
@@ -70,12 +69,12 @@ pub(crate) fn clone_die_attributes<'a>(
         // FIXME for CU: currently address_transform operate on a single
         // function range, and when CU spans multiple ranges the
         // transformation may be incomplete.
-        RangeInfoBuilder::from(dwarf, unit, entry)?
+        RangeInfoBuilder::from(unit, entry)?
     };
     range_info.build(addr_tr, out_unit, out_entry_id);
 
     let mut is_obj_ptr = false;
-    prepare_die_context(dwarf, unit, entry, &mut attr_context, &mut is_obj_ptr)?;
+    prepare_die_context(unit, entry, &mut attr_context, &mut is_obj_ptr)?;
 
     let mut attrs = entry.attrs();
     while let Some(attr) = attrs.next()? {
@@ -133,7 +132,7 @@ pub(crate) fn clone_die_attributes<'a>(
                     continue;
                 }
                 gimli::DW_AT_name => {
-                    let old_name: &str = &dwarf.attr_string(unit, attr.value())?.to_string_lossy();
+                    let old_name: &str = &unit.attr_string(attr.value())?.to_string_lossy();
                     let new_name = format!("__{old_name}");
                     dbi_log!(
                         "Object pointer: renamed '{}' -> '{}'",
@@ -158,7 +157,7 @@ pub(crate) fn clone_die_attributes<'a>(
                 write::AttributeValue::Address(addr)
             }
             AttributeValue::DebugAddrIndex(i) => {
-                let u = dwarf.address(unit, i)?;
+                let u = unit.address(i)?;
                 let addr = addr_tr.translate(u).unwrap_or(write::Address::Constant(0));
                 write::AttributeValue::Address(addr)
             }
@@ -202,28 +201,18 @@ pub(crate) fn clone_die_attributes<'a>(
             }
             AttributeValue::String(d) => write::AttributeValue::String(d.to_vec()),
             AttributeValue::DebugStrRef(_) | AttributeValue::DebugStrOffsetsIndex(_) => {
-                let s = dwarf
-                    .attr_string(unit, attr_value)?
-                    .to_string_lossy()
-                    .into_owned();
+                let s = unit.attr_string(attr_value)?.to_vec();
                 write::AttributeValue::StringRef(out_strings.add(s))
             }
             AttributeValue::RangeListsRef(_) | AttributeValue::DebugRngListsIndex(_) => {
-                let r = dwarf.attr_ranges_offset(unit, attr_value)?.unwrap();
-                let range_info = RangeInfoBuilder::from_ranges_ref(dwarf, unit, r)?;
+                let r = unit.attr_ranges_offset(attr_value)?.unwrap();
+                let range_info = RangeInfoBuilder::from_ranges_ref(unit, r)?;
                 let range_list_id = range_info.build_ranges(addr_tr, &mut out_unit.ranges);
                 write::AttributeValue::RangeListRef(range_list_id)
             }
             AttributeValue::LocationListsRef(_) | AttributeValue::DebugLocListsIndex(_) => {
-                let r = dwarf.attr_locations_offset(unit, attr_value)?.unwrap();
-                let low_pc = 0;
-                let mut locs = dwarf.locations.locations(
-                    r,
-                    unit_encoding,
-                    low_pc,
-                    &dwarf.debug_addr,
-                    unit.addr_base,
-                )?;
+                let r = unit.attr_locations_offset(attr_value)?.unwrap();
+                let mut locs = unit.locations(r)?;
                 let frame_base =
                     if let EntryAttributesContext::Children { frame_base, .. } = attr_context {
                         frame_base
@@ -373,10 +362,9 @@ pub(crate) fn clone_die_attributes<'a>(
     Ok(())
 }
 
-fn prepare_die_context(
-    dwarf: &Dwarf<Reader<'_>>,
-    unit: &Unit<Reader<'_>>,
-    entry: &DebuggingInformationEntry<Reader<'_>>,
+fn prepare_die_context<'a>(
+    unit: UnitRef<Reader<'a>>,
+    entry: &DebuggingInformationEntry<Reader<'a>>,
     attr_context: &mut EntryAttributesContext<'_>,
     is_obj_ptr: &mut bool,
 ) -> Result<(), Error> {
@@ -407,7 +395,7 @@ fn prepare_die_context(
                 subprogram.param_num += 1;
 
                 if subprogram.obj_ptr == entry.offset()
-                    || is_obj_ptr_param(dwarf, unit, entry, subprogram.param_num)?
+                    || is_obj_ptr_param(unit, entry, subprogram.param_num)?
                 {
                     *is_obj_ptr = true;
                 }
@@ -418,10 +406,9 @@ fn prepare_die_context(
     Ok(())
 }
 
-fn is_obj_ptr_param(
-    dwarf: &Dwarf<Reader<'_>>,
-    unit: &Unit<Reader<'_>>,
-    entry: &DebuggingInformationEntry<Reader<'_>>,
+fn is_obj_ptr_param<'a>(
+    unit: UnitRef<Reader<'a>>,
+    entry: &DebuggingInformationEntry<Reader<'a>>,
     param_num: isize,
 ) -> Result<bool, Error> {
     debug_assert!(entry.tag() == gimli::DW_TAG_formal_parameter);
@@ -437,7 +424,7 @@ fn is_obj_ptr_param(
     {
         // Either this has no name (declarations omit them), or its explicitly "this".
         let name = entry.attr_value(gimli::DW_AT_name)?;
-        if name.is_none() || dwarf.attr_string(unit, name.unwrap())?.slice().eq(b"this") {
+        if name.is_none() || unit.attr_string(name.unwrap())?.slice().eq(b"this") {
             // Finally, a type check. We expect a pointer.
             if let Some(type_attr) = entry.attr_value(gimli::DW_AT_type)? {
                 if let Some(type_die) = resolve_die_ref(unit, &type_attr)? {