Add UnitRef::shared_attrs

[jyn514]

Often, consumers of a DWARF file will want to know if a given function has an attribute, either directly or indirectly. Currently, finding that information is tedious and prone to error. Add a helper function in gimli to make this easier.

This does not attempt to have the maximum possible performance; consumers who care about that can still implement the logic themselves. In particular, the caches are recalculated each time shared_attrs is called, introducing an allocation.

The logic works roughly as follows (inspired by addr2line::Function::parse_children):

• Create a cache of all DWARF units known to the UnitRef
• Given an entry offset, iterate over all its attributes and pass them to a callback chosen by the caller.
• For each indirect attribute, such as DW_AT_abstract_origin, follow the pointer to the abstract source and do the same thing again there.

I have tested this downstream (see rust-lang/rust#134831), but I have not yet added unit tests. Let me know how you would like this to be tested.

[philipc]

I agree that it would be nice to make this easier, but I don't think this is the API we should be providing. I would like to see at least the following:

• any cache that is created should allow reuse between calls
• if this is something that addr2line also does, then the API should be suitable for use by addr2line as well, without loss of performance

Also note that UnitRef is a thin wrapper around Unit, so we shouldn't be adding large methods like this directly on UnitRef; they should be simple calls to something else.

[philipc]

It appears you want to use this in backtrace which is already using addr2line, so a simpler alternative for now may to be add a method that does this to addr2line, so that it can use the cache that already exists there.

[jyn514]

It appears you want to use this in backtrace which is already using addr2line, so a simpler alternative for now may to be add a method that does this to addr2line, so that it can use the cache that already exists there.

hmm, I don't mind adding it there, but I worry that it's slightly out of scope for addr2line? If you're happy to take the PR I'm happy to make it though, I'll start work on that.

[philipc]

I worry that it's slightly out of scope for addr2line?

Yes it is slightly, but so is find_dwarf_and_unit.

[jyn514]

a simpler alternative for now may to be add a method that does this to addr2line, so that it can use the cache that already exists there.

this turned out to a be a little more complicated than expected, so I'm going to try and make this new shared_attrs API more performant instead.

I can make the cache reusable, that's not too much trouble (although it makes the API slightly more complicated). addr2line is also doing a little dance where it uses unit.entries_raw instead of the simpler entries API. do you know why it does that? EntriesRaw has this comment:

EntriesRaw lacks some features of EntriesCursor, such as the ability to skip to the next sibling DIE. However, this also allows it to optimize better, since it does not need to perform the extra bookkeeping required to support these features, and thus it is suitable for cases where performance is important.

but addr2line::Function::parse_children is still skipping to the next sibling DIE, so I'm not sure it's saving time in practice? has this been benchmarked?

[philipc]

but addr2line::Function::parse_children is still skipping to the next sibling DIE, so I'm not sure it's saving time in practice? has this been benchmarked?

The comment means that EntriesRaw does not have the equivalent of EntriesCursor::next_sibling, so you have to implement it yourself, and it is possible to do that in a way that is faster than EntriesCursor could do it. The use of EntriesRaw was benchmarked.

[jyn514]

Are there existing tests for Functions::parse_inline_functions? I modified this API to be flexible enough for addr2line to use, but did not use EntriesRaw, and cargo bench in the addr2line crate shows that there was a 50% speedup. That seems unlikely; I suspect I broke something. But I'm not sure what I broke, because all the tests pass.

[philipc]

No, Functions::parse_inlined_functions exists only for the benchmark, and simply calls Function::parse for every function. There are tests that result in Function::parse being called (e.g. anything that results in find_frames being called). There may be code paths in Function::parse that aren't tested; addr2line's regression testing history isn't good.

Can you show your changes?

[jyn514]

sure thing - here are the addr2line changes: https://github.com/gimli-rs/addr2line/compare/master...jyn514:addr2line:use-gimli-shared-attrs?expand=1
and i've just now pushed the gimli changes to this branch.

[philipc]

Okay so addr2line needs better tests. Try running this and compare before and after your change:

target/debug/addr2line --all -afi -e testinput/dwarf/base-clang-g2

I see differences such as:

 0x00000000000011d0
-_ZN3Foo3barEPKc
-/object/testfiles/dwarf/base.cpp:15
 _ZN3Foo3fooEi
-/object/testfiles/dwarf/base.cpp:21
+/object/testfiles/dwarf/base.cpp:15

so it appears to not be handling inlined functions correctly.

[philipc]

Sorry I wasn't thinking straight. These are already tested and shown to fail by cargo test --features bin, so please run that instead.

Once that is working though, it's possible there's still other things that aren't tested, so it's worth running addr2line --all -afi on a larger binary on your system (create an issue if you find anything not covered by cargo test --features bin).