Support Dwarf 5 .debug_rnglists and .debug_loclists #272

rocallahan · 2017-12-27T09:35:13Z

gcc 7.2.1, for example, uses these when you specify -gdwarf-5.

rocallahan · 2017-12-27T09:37:49Z

Note that these sections have an indirection scheme involving .debug_addr but I haven't encountered that being used yet so I haven't added support for it (nor do I fully understand it).

coveralls · 2017-12-27T09:53:57Z

Coverage decreased (-0.4%) to 93.632% when pulling 2d8d1ad on rocallahan:downstream into 7025c09 on gimli-rs:master.

philipc

Thanks! This is looking good.

There's a few minor things to fix that I've left comments for, some of these will apply to both locations and ranges.

The main thing we still need to work out is how we get a location list given an attribute with class loclist (and similar for range lists). It'd be great if you could modify the dwarfdump example to do this, and test it on the binaries that gcc creates. But we can also leave that for followup work if you want, as long as we have an idea of how it is meant to work.

philipc · 2017-12-28T02:55:10Z

src/loc.rs

+    /// Return true if this section is empty
+    pub fn is_empty(&self) -> bool {
+        self.debug_loc_section.is_empty()
+    }


Can you move this into the DebugLoc<R> impl? Same for the other sections.

philipc · 2017-12-28T03:06:05Z

src/loclists.rs

+    /// `FallibleIterator`](./index.html#using-with-fallibleiterator).
+    pub fn locations(
+        &self,
+        offset: DebugLocOffset<R::Offset>,


This should probably should be a DebugLocListsOffset.

It's easier to use if we share the same offset type for DebugLoc::locations and DebugLocLists::locations. Basically gcc is creating DW_AT_locations with DW_FORM_sec_offset; for Dwarf4 those are offsets into .debug_loc, and in Dwarf5 those are offsets into .debug_loclists. In my code that uses gimli, we try loading both sections and call locations on whichever one is non-empty, passing the same offset.

The difference in usability is small though, so I can make this change if you really want it.

Though I can imagine cases where people would want to store the location offset somewhere in way that's independent of Dwarf version; in that case having different offset types would force them to create their own enum.

Ok, using DebugLocOffset is fine then.

Is it possible for a binary to have a mix of Dwarf4 and Dwarf5 units, so that neither section is empty?

I don't know, but in that case I think DW_AT_location with DW_FORM_sec_offset would be ambiguous; I don't see any way to tell which section to look it up in.

I assume we would base it on the compilation unit version. I'll investigate a bit later.

Yes, it is possible to have mixed Dwarf4 and Dwarf5, and I think you're probably right about using the compilation unit version.

I'll fix this.

philipc · 2017-12-28T03:07:33Z

src/rnglists.rs

+    /// `FallibleIterator`](./index.html#using-with-fallibleiterator).
+    pub fn ranges(
+        &self,
+        offset: DebugRangesOffset<R::Offset>,


This should be a DebugRngListsOffset.

philipc · 2017-12-28T03:17:23Z

src/loclists.rs

+}
+
+fn parse_header<R: Reader>(input: &mut R) -> Result<LocListsHeader> {
+    let (_, format) = parser::parse_initial_length(input)?;


We need to split the input at the length that is parsed, and return that. Have a look at what ArangeParser::parse_header does for an example.

This probably isn't necessary, since we don't continue parsing that input currently.

philipc · 2017-12-28T03:17:26Z

src/loclists.rs

+        return Err(Error::UnknownVersion(version as u64));
+    }
+    let address_size = input.read_u8()?;
+    let _segment_selector_size = input.read_u8()?;


If we're not going to implement segment selector support then we need to return an error if this is non-zero. Also, I tried to see what would be involved in supporting this, and I can't see where the segment selector is defined in the location list entries. Do you know anything about this?

No. I'll add the zero check. But I think we should panic for unsupported segment sizes.

philipc · 2017-12-28T03:46:21Z

src/loclists.rs

+                },
+                _ => {
+                    // We don't support AddressIndex-based entries yet
+                    panic!("Unsupported loclists entry: {:?}", raw_loc);


It's better to return an error instead.

philipc · 2017-12-28T04:43:25Z

src/loclists.rs

+
+impl<R: Reader> RawLocListEntry<R> {
+    /// Parse a range entry from `.debug_rnglists`
+    #[doc(hidden)]


Shouldn't need this attribute on a private function.

philipc · 2017-12-28T09:49:21Z

src/loclists.rs

+    /// `FallibleIterator`](./index.html#using-with-fallibleiterator).
+    pub fn raw_locations(
+        &self,
+        offset: DebugLocOffset<R::Offset>,


This should probably should be a DebugLocListsOffset. However, I can't see in the standard where there are any attributes that contain an offset to a location list header, which is what this function currently wants. The loclist and loclistsptr classes are either a section offset for an offset table, an index into that offset table, or an offset to a location list.

So I'm not sure that this method is useful. But I'm also not sure how the location list headers are meant to be used in practice. Is the DWARF consumer meant to iterate over all location list headers and record their offset ranges, and then when there is a loclist or loclistsptr attribute, it needs to search to see which header to use?

The offset here is an offset within the section to the start of a location list. There is no such thing as a "location list header" as far as I know. This corresponds to class loclistptr or the DW_FORM_sec_offset form of class loclist in the Dwarf5 spec 7.5.5.

The section's offset table is used with DW_FORM_loclistx and I haven't found gcc using that yet (and the .debug_loclists sections I've seen so far don't have an offset table) so I'm not sure how it's supposed to work. I think we'd support that by adding alternative iterator construction methods, although possibly we could just detect that the offset is within the offset table.

raw_locations is actually parsing the same section header every time we create a location list iterator. This is slightly inefficient, but the alternative is to parse the section header when we construct DebugLocLists, in which case the construction needs to return a Result, which means we can't use From, which means we can't implement the Section trait, which seems bad.

As for how these are used, see above. We take a DW_AT_location DW_FORM_sec_offset and pass it directly in here.

rocallahan · 2017-12-28T10:33:15Z

The main thing we still need to work out is how we get a location list given an attribute with class loclist (and similar for range lists). It'd be great if you could modify the dwarfdump example to do this, and test it on the binaries that gcc creates. But we can also leave that for followup work if you want, as long as we have an idea of how it is meant to work.

An attribute with DW_FORM_sec_offset is just a direct offset into the section and can be used by passing the offset to the iterator construction methods defined here.

There's a different approach using DW_FORM_loclistx and DW_FORM_rnglistx but I don't fully understand how that works, and gcc 7.2.1 isn't using those in the examples I've looked at so far. So I was hoping to ignore that for now, until I have at least one example I can test with.

rocallahan · 2017-12-28T10:43:48Z

I can modify dwarfdump to handle DW_FORM_sec_offset.

philipc · 2017-12-28T11:07:36Z

An attribute with DW_FORM_sec_offset is just a direct offset into the section and can be used by passing the offset to the iterator construction methods defined here.

Do you mean methods such as DebugLocLists::locations? My concern is that this method requires an offset to a location list table header, but for something like DW_AT_location, the DW_FORM_sec_offset value will be an offset to an individual location list within that table.

philipc · 2017-12-28T11:20:32Z

Ah, I was misreading what DebugLocLists::locations does. So it only ever parses the header for the first location list table, and uses that for all location lists.

rocallahan · 2017-12-28T11:21:53Z

I think it's more accurate to say that the section has a header and the individual location lists do not.

rocallahan · 2017-12-28T11:22:36Z

See sections 7.28 and 7.29 in the spec.

rocallahan · 2017-12-28T23:20:47Z

Updating dwarfdump is a pain because currently it replicates the logic that translates raw entries into RangeListEntry/LocationListEntry, and the new entry types mean more code gets replicated.

rocallahan · 2017-12-28T23:21:48Z

I guess I can avoid the replication by iterating over the raw and cooked entries together...

philipc · 2017-12-28T23:37:07Z

Ah yeah that code is a bit messy. Dumping only the cooked entries would be fine too. It doesn't need to match what's there already.

rocallahan · 2017-12-28T23:48:03Z

Dumping the raw entries could actually be useful though.

rocallahan · 2017-12-28T23:49:05Z

I'm encountering a good example of where deny(missing_docs) gets annoying, e.g.

    /// DW_LLE_startx_endx
    StartxEndx {
        /// start of range
        begin: AddressIndex,
        /// end of range
        end: AddressIndex,
        /// expression
        data: Expression<R>,
    },

Never mind I guess...

rocallahan · 2017-12-29T01:40:52Z

I removed the is_empty methods since clients should use the compilation unit version to figure out which section to use.

coveralls · 2017-12-29T01:53:26Z

Coverage decreased (-0.1%) to 93.918% when pulling 1f17b1f on rocallahan:downstream into 04aa240 on gimli-rs:master.

gcc 7.2.1 uses Sdata for a FileIndex so this needs to work.

coveralls · 2017-12-29T05:10:24Z

Coverage decreased (-0.1%) to 93.918% when pulling aa69664 on rocallahan:downstream into 04aa240 on gimli-rs:master.

fitzgen · 2017-12-29T18:43:39Z

Just wanted to pop in and say thanks for the PR @rocallahan !

philipc · 2017-12-30T04:27:56Z

Thanks @rocallahan !

rocallahan · 2017-12-30T04:31:37Z

You're welcome!

Fix attribute name; it's DW_AT_rnglists_base in the DWARF5 spec

c74996f

philipc requested changes Dec 28, 2017

View reviewed changes

rocallahan added 5 commits December 29, 2017 17:54

Implement DebugRngLists for Dwarf 5

0dfb9d6

Support coercing Sdata to unsigned values and Udata to signed values

eb69491

gcc 7.2.1 uses Sdata for a FileIndex so this needs to work.

Fix test name

ef4d8a3

Support Dwarf 5 .debug_loclists

67f6cd4

Add .debug_loclists/.debug_rnglists support to dwarfdump

aa69664

philipc approved these changes Dec 30, 2017

View reviewed changes

philipc merged commit 21041c7 into gimli-rs:master Dec 30, 2017

Support Dwarf 5 .debug_rnglists and .debug_loclists #272

Support Dwarf 5 .debug_rnglists and .debug_loclists #272

Conversation

rocallahan commented Dec 27, 2017

rocallahan commented Dec 27, 2017

coveralls commented Dec 27, 2017

philipc left a comment

Choose a reason for hiding this comment

philipc Dec 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rocallahan commented Dec 28, 2017

rocallahan commented Dec 28, 2017

philipc commented Dec 28, 2017

philipc commented Dec 28, 2017

rocallahan commented Dec 28, 2017

rocallahan commented Dec 28, 2017

rocallahan commented Dec 28, 2017

rocallahan commented Dec 28, 2017

philipc commented Dec 28, 2017

rocallahan commented Dec 28, 2017

rocallahan commented Dec 28, 2017

rocallahan commented Dec 29, 2017

coveralls commented Dec 29, 2017

coveralls commented Dec 29, 2017

fitzgen commented Dec 29, 2017

philipc commented Dec 30, 2017

rocallahan commented Dec 30, 2017

philipc Dec 28, 2017 •

edited

Loading