dwarfdump: handle relocations

[philipc]

This applies relocations as we read values, as suggested in #180 (comment).

I've seen relocations for the read_address and read_offset hooks. I expect DWARF 2 to need relocations for the read_sized_offset hook. I'm not sure that the read_length hook is required, but it doesn't hurt.

Fixes #180

[coveralls]

Coverage decreased (-0.06%) to 85.499% when pulling 7af90a4 on philipc:reloc into 5398258 on gimli-rs:master.

[fitzgen]

Overall, this looks really great, and it will be so nice to have this functionality implemented. Most of these changes are also great, but I do have one piece of feedback.

I'm skeptical on using a new trait for hooking underlying calls, instead of using composition the way that iterators, futures, etc work.

Here is a sketch of what I would expect this to look like:

#[derive(Clone, Debug)]
pub struct RelocReader<'a, R> {
    reader: R,
    section: R,
    relocations: &'a RelocationMap,
}

impl Reader for RelocReader<R>
where
    R: Reader,
{
    type Endian = R::Endian;
    type Offset = R::Offset;

    // For the methods that need relocs applied, we wrap the underlying reader
    // directly (instead of indirectly via a new hooks trait).

    fn read_address(&mut self, address_size: u8) -> gimli::Result<u64> {
        let offset = self.reader.offset_from(&self.section);
        let value = self.reader.read_address(address_size)?;
        Ok(self.relocate(offset, value))
    }

    fn read_length(&mut self, format: gimli::Format) -> gimli::Result<usize> {
        let offset = self.reader.offset_from(&self.section);
        let value = self.reader.read_length(format)?;
        <usize as gimli::ReaderOffset>::from_u64(self.relocate(offset, value as u64))
    }

    // etc...

    // And then delegate the other methods that don't need relocs applied
    // directly to the underlying reader.

    fn endian(&self) -> Self::Endian {
        self.reader.endian()
    }

    fn len(&self) -> Self::Offset {
        self.reader.len()
    }

    // etc...
}

What are your thoughts on this approach vs the hooks trait?

[philipc]

Yeah I'd love to have a better way of doing the hooking.

The problem I want to solve is that I want whatever is doing the default delegations to be in gimli, so that we don't have a ton of boilerplate in every client that wants to handle relocations. For your suggestion, RelocReader would have to be in gimli, which means we need to also put RelocationMap in gimli, but I'm worried that RelocationMap is too dependent on the client's relocation parsing to be able to do that... unless we make RelocationMap be a trait too?

Is there some way to avoid the delegation boilerplate with a macro? It's not something I've looked into.

[philipc]

Adding a struct RelocReader and trait RelocationMap to gimli is basically the same as the struct HookedReader and trait ReaderHook but less flexible, so I don't think that gains us anything.

There's a couple of crates with delegation macros, but they barely reduce the boilerplate in my opinion.

I've updated the PR. I've deleted the hook stuff from gimli, and dwarfdump does the full Reader impl now.

[philipc]

@fitzgen can you take another look at this?

[fitzgen]

Looks great!