Improve documentation for converting relocatable object files (#837)

This includes a restructure of the logic in the range list and
location list conversion, but the result should be the same.

Also add subprogram range list handling to convert example.

Author: philipc

crates/examples/src/bin/convert.rs

@@ -264,6 +264,53 @@ fn need_entry<R: gimli::Reader<Offset = usize>>(
                 if let Some(address) = entry.unit.attr_address(attr)? {
                     return Ok(!is_tombstone_address(entry, address));
                 }
+            } else if let Some(attr) = entry.attr_value(gimli::DW_AT_ranges) {
+                if let Some(offset) = entry.unit.attr_ranges_offset(attr)? {
+                    let mut base = if entry.unit.low_pc != 0 {
+                        // Have a base and it is valid.
+                        Some(true)
+                    } else {
+                        // Don't have a base.
+                        None
+                    };
+                    let mut ranges = entry.unit.raw_ranges(offset)?;
+                    while let Some(range) = ranges.next()? {
+                        match range {
+                            gimli::RawRngListEntry::AddressOrOffsetPair { begin, .. } => {
+                                if base == Some(true)
+                                    || (base.is_none() && !is_tombstone_address(entry, begin))
+                                {
+                                    return Ok(true);
+                                }
+                            }
+                            gimli::RawRngListEntry::OffsetPair { .. } => {
+                                if base == Some(true) {
+                                    return Ok(true);
+                                }
+                            }
+                            gimli::RawRngListEntry::BaseAddress { addr } => {
+                                base = Some(!is_tombstone_address(entry, addr));
+                            }
+                            gimli::RawRngListEntry::BaseAddressx { addr } => {
+                                let addr = entry.unit.address(addr)?;
+                                base = Some(!is_tombstone_address(entry, addr));
+                            }
+                            gimli::RawRngListEntry::StartEnd { begin, .. }
+                            | gimli::RawRngListEntry::StartLength { begin, .. } => {
+                                if !is_tombstone_address(entry, begin) {
+                                    return Ok(true);
+                                }
+                            }
+                            gimli::RawRngListEntry::StartxEndx { begin, .. }
+                            | gimli::RawRngListEntry::StartxLength { begin, .. } => {
+                                let begin = entry.unit.address(begin)?;
+                                if !is_tombstone_address(entry, begin) {
+                                    return Ok(true);
+                                }
+                            }
+                        }
+                    }
+                }
             }
         }
         gimli::DW_TAG_variable => {

src/write/dwarf.rs

@@ -126,11 +126,20 @@ pub(crate) mod convert {
     impl Dwarf {
         /// Create a `write::Dwarf` by converting a `read::Dwarf`.
         ///
-        /// `convert_address` is a function to convert read addresses into the `Address`
-        /// type. For non-relocatable addresses, this function may simply return
-        /// `Address::Constant(address)`. For relocatable addresses, it is the caller's
-        /// responsibility to determine the symbol and addend corresponding to the address
-        /// and return `Address::Symbol { symbol, addend }`.
+        /// `convert_address` is a function to convert addresses read by
+        /// `Reader::read_address` into the `Address` type. For executable files,
+        /// it is sufficient to simply map the address to `Address::Constant`.
+        ///
+        /// Relocatable object files are more complicated because there are relocations
+        /// associated with the address. To handle this, you can use a `Reader`
+        /// implementation for which `Reader::read_address` stores the relocation
+        /// information in a map and returns the map key instead of an address. Then
+        /// `convert_address` can look up the mapping to produce an `Address`.
+        ///
+        /// Note that `convert_address` is also used for address and offset pairs in
+        /// DWARF v2-v4 range lists and location lists. In order for the parser to
+        /// correctly handle these, `Reader::read_address` must return the values 0 and -1
+        /// unchanged.
         ///
         /// `convert_address` should not be used for complex address transformations, as it
         /// will not be called for address offsets (such as in `DW_AT_high_pc`, line programs,

src/write/loc.rs

@@ -334,30 +334,23 @@ mod convert {
             while let Some(from_loc) = from.next()? {
                 let loc = match from_loc {
                     read::RawLocListEntry::AddressOrOffsetPair { begin, end, data } => {
-                        // These were parsed as addresses, even if they are offsets.
+                        // This matches the logic in `RangeList::from`. See the comments there.
                         let begin = convert_address(begin)?;
                         let end = convert_address(end)?;
                         let data = convert_expression(data)?;
-                        match (begin, end) {
-                            (Address::Constant(begin_offset), Address::Constant(end_offset)) => {
-                                if have_base_address {
-                                    Location::OffsetPair {
-                                        begin: begin_offset,
-                                        end: end_offset,
-                                        data,
-                                    }
-                                } else {
-                                    Location::StartEnd { begin, end, data }
-                                }
-                            }
-                            _ => {
-                                if have_base_address {
-                                    // At least one of begin/end is an address, but we also have
-                                    // a base address. Adding addresses is undefined.
-                                    return Err(ConvertError::InvalidRangeRelativeAddress);
-                                }
-                                Location::StartEnd { begin, end, data }
+                        if have_base_address {
+                            let (Address::Constant(begin_offset), Address::Constant(end_offset)) =
+                                (begin, end)
+                            else {
+                                return Err(ConvertError::InvalidRangeRelativeAddress);
+                            };
+                            Location::OffsetPair {
+                                begin: begin_offset,
+                                end: end_offset,
+                                data,
                             }
+                        } else {
+                            Location::StartEnd { begin, end, data }
                         }
                     }
                     read::RawLocListEntry::BaseAddress { addr } => {

src/write/range.rs

@@ -235,33 +235,43 @@ mod convert {
             convert_address: &dyn Fn(u64) -> Option<Address>,
         ) -> ConvertResult<Self> {
             let convert_address = |x| convert_address(x).ok_or(ConvertError::InvalidAddress);
+            // The CU DW_AT_low_pc was parsed with `Reader::read_address`.
+            // We could pass it to `convert_address`, but we rely on `read_address`
+            // returning 0 if and only if it was an unrelocated 0 value.
+            // If it is 0, then DWARF v2-4 ranges are address pairs unless there is a
+            // base address entry, otherwise they must be offset pairs.
+            // We don't handle the possibility of this being a tombstone since I don't
+            // think that can occur.
             let mut have_base_address = from_unit.low_pc != 0;
             let mut ranges = Vec::new();
             while let Some(from_range) = from.next()? {
                 let range = match from_range {
                     read::RawRngListEntry::AddressOrOffsetPair { begin, end } => {
-                        // These were parsed as addresses, even if they are offsets.
+                        // These were parsed with `Reader::read_address`, even if they are
+                        // offsets, so we need to apply the conversion function.
+                        // For executables, the converted values will be `Address::Constant`
+                        // for both offsets and addresses.
+                        // For relocatable objects, we expect offsets to be `Address::Constant`
+                        // and addresses to be `Address::Symbol`.
                         let begin = convert_address(begin)?;
                         let end = convert_address(end)?;
-                        match (begin, end) {
-                            (Address::Constant(begin_offset), Address::Constant(end_offset)) => {
-                                if have_base_address {
-                                    Range::OffsetPair {
-                                        begin: begin_offset,
-                                        end: end_offset,
-                                    }
-                                } else {
-                                    Range::StartEnd { begin, end }
-                                }
-                            }
-                            _ => {
-                                if have_base_address {
-                                    // At least one of begin/end is an address, but we also have
-                                    // a base address. Adding addresses is undefined.
-                                    return Err(ConvertError::InvalidRangeRelativeAddress);
-                                }
-                                Range::StartEnd { begin, end }
+                        // We must use the presence of a base address to disambiguate between
+                        // offsets and addresses for both executables and relocatable objects.
+                        // (This logic is also used in `LocationList::from`.)
+                        if have_base_address {
+                            let (Address::Constant(begin_offset), Address::Constant(end_offset)) =
+                                (begin, end)
+                            else {
+                                // We have a relocatable object file that uses both a base address
+                                // and an address pair.
+                                return Err(ConvertError::InvalidRangeRelativeAddress);
+                            };
+                            Range::OffsetPair {
+                                begin: begin_offset,
+                                end: end_offset,
                             }
+                        } else {
+                            Range::StartEnd { begin, end }
                         }
                     }
                     read::RawRngListEntry::BaseAddress { addr } => {