addr2line icon indicating copy to clipboard operation
addr2line copied to clipboard

Add more convenience APIs

Open philipc opened this issue 6 years ago • 4 comments
trafficstars

In https://github.com/jonhoo/inferno/pull/14#issuecomment-459752419, @jonhoo said:

The un_inline function introduced in this PR has to:

* open the file

* pass it to `object::File::parse`

* pass that to `Context::new`

* call `Context::find_frames`

* iterate over the frames (with an extra `unwrap` it seems)

* tease out the function name from each frame using (somewhat simplified)
  ```rust
  addr2line::demangle_auto(frame.function.map(|func| func.raw_name()), func.language))
  ```

To me at least, that's very similar to the flow of examples/addr2line.rs, and it seems unfortunate to have to repeat the whole boilerplate if this is a common use-case for addr2line (looking up function stack by address). In particular, I wonder whether addr2line could provide a convenience API for "create a new Context from a file" (maybe using mmap and from_sections internally if possible), and "iterate over the functions of an address" (including demangling the raw_name). Something like Context::from_file(&File) -> Context and Context::resolve_iter(u64) -> impl Iterator<Item = Function>, where Function is a struct that holds the demangled name, file, line number, etc.

I haven't given any thought to what exactly the API should be, but making things easier is certainly a good goal.

philipc avatar Feb 02 '19 02:02 philipc

https://github.com/jonhoo/inferno/pull/14#issuecomment-459965201

@jonhoo Oh, I see. What you're suggesting makes sense, but the reason I originally structured the API this way is to avoid pulling in heavy external dependencies that are not strictly needed. For instance memmap is obviously just one of many ways to obtain code. Then there is object which is a very convenient way to deal with binaries across platforms but by no means a hard dependency of addr2line - here Context::new already is the convenience function in the sense that it fetches and constructs all the gimli sections for you. Constructing the entire Object in addr2line however is the point where the tradeoff between convenience vs flexibility is not worth it any more imo.

Bringing a file into memory and then calling object::File::parse on it may be a common usecase but I feel like this is best served by an example to copy from rather than an actual black-box function.

The frame iteration is basically a bog-standard for loop - except it has to be a FallibleIterator because the library lazily parses the DWARF whenever necessary and this might fail (e.g. malformed DWARF). This is impossible to abstract over without sacrificing the laziness and eagerly parsing and buffering all the frames before returning initially.

The demangling is only implemented this way in the addr2line example because you can turn it off. In your example you may just use FunctionName::demangle.

The crate already does a lot in terms of abstracting a complex task into simple parts. I agree that it could use some ergonomic improvements but consider that all of these papercuts at least have a reason. For instance the Frame struct is divided between function and location because those are completely independent in DWARF - one may exist while the other does not.

That being said, I very much agree with the basic sentiment: making things easier is certainly a good goal.

main-- avatar Feb 02 '19 13:02 main--

@main-- let's continue the discussion here :)

I wasn't intending to imply that the design decisions of the API don't make sense. Not at all! I am all too aware of why the pieces fit together the way they do. I do think there's value in providing an additional ergonomic layer for those who just want to "do the standard thing" though, and I'm pretty sure we could reduce a decent amount of boilerplate that way.

jonhoo avatar Feb 02 '19 16:02 jonhoo

In particular, I wonder whether addr2line could provide a convenience API for "create a new Context from a file" (maybe using mmap and from_sections internally if possible), [...] Something like Context::from_file(&File) -> Context [...]

Our support for loading DWARF is lacking. We need to handle things like split DWARF. This is not specific to addr2line, other consumers of DWARF need to do it too. Long term I think we want:

  • a crate that can load DWARF sections into gimli::Dwarf. This will either use moria, or be part of moria (not sure which, moria doesn't currently have a gimli dependency).
  • an API in addr2line that accepts gimli::Dwarf

I think any convenience functions for using memmap should be in this other crate.

For now, it's only 4 lines in inferno. The only complexity in inferno is from the error handling, and doing that in inferno gives you more flexibilty. Doing the memmap in addr2line is more complexity because we have to then solve the file/memmap lifetime problem. I don't think this is worth it.

[...] "iterate over the functions of an address" (including demangling the raw_name). Something like [...] Context::resolve_iter(u64) -> impl Iterator<Item = Function>, where Function is a struct that holds the demangled name, file, line number, etc.

I can't see what you're asking for here. How is the existing Frame different from this? It contains name, file, line number, and the name has a method for the demangled name. One thing I might have proposed is a convenience method that returns this all as a String formatted in the standard method for a backtrace, but looking at inferno you wouldn't use that anyway because you're doing other processing on the name.

philipc avatar Feb 21 '19 06:02 philipc

I can't see what you're asking for here. How is the existing Frame different from this?

Yeah, looking at the code a second time with some distance, I take that back. The existing API there is fine :)

jonhoo avatar Feb 21 '19 13:02 jonhoo

I don't think anything is going to change in this crate to support this. wholesym provides a nice higher level API, and uses this crate under the hood while also supporting much more.

philipc avatar Apr 12 '24 06:04 philipc