Skip to content

Switching the backtrace crate to using object #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alexcrichton opened this issue Apr 21, 2020 · 7 comments
Closed

Switching the backtrace crate to using object #215

alexcrichton opened this issue Apr 21, 2020 · 7 comments

Comments

@alexcrichton
Copy link
Contributor

Hello! Currently the backtrace crate is using the goblin crate to parse object files when using the gimli-symbolize feature, but unfortunately this brings in a pretty weighty dependency tree:

backtrace v0.3.46 (/home/alex/code/backtrace-rs)
├── goblin v0.2.1
│   ├── log v0.4.8
│   │   └── cfg-if v0.1.10
│   ├── plain v0.2.3
│   └── scroll v0.10.1
│       └── scroll_derive v0.10.1
│           ├── proc-macro2 v1.0.10
│           │   └── unicode-xid v0.2.0
│           ├── quote v1.0.3
│           │   └── proc-macro2 v1.0.10 (*)
│           └── syn v1.0.17
│               ├── proc-macro2 v1.0.10 (*)
│               ├── quote v1.0.3 (*)
│               └── unicode-xid v0.2.0
...

I'm looking to slim down the dependency tree of backtrace in the goal of eventually (in the long term) turning on gimli-symbolize by default in the standard library. Today I was poking around with the object crate and it's almost what I wanted but it seemed to lack a few things. In any case @fitzgen recommended I open up an issue here, so I was hoping if we could chat a bit here and see if it's possible to pull in object for use in backtrace?

Some things I've noticed while trying to port to object are:

  • Would it be possible to drop the target-lexicon dependency for this use case? It's not really causing issues but having as few dependencies as possible for libstd I think is better.
  • I think I got ELF working, but it's not necessarily exactly how I imagined it. The two main bits I think which feel a bit odd are that Elf::parse parses a lot more of the elf header than is necessary for the backtrace use case. For example (at least right now), gimli-symbolize doesn't need relocations. Additionally it stores a list of parsed Symbol object, but those are pretty large structures and might be somewhat unwieldy. Ideally I'd have a sorted array of usize, SymbolIndex which I could then look up in the original file if necessary. Would it be possile to cut down the amount of work here?
  • I tried to get Windows/COFF/PE working as well, but I unfortunately didn't make it too too far. I found that the cross-platform-ness of the traits in read were a bit of a barrier here because of how different PE was from ELF. For example symbols don't have sizes so we try to explicitly work around that. Additionally the address calculation for a symbol is somewhat different (I think this comes from libbacktrace? I sort of forget...) and requires access to the header to read image_base, but I couldn't figure out how to read that header field.

To be clear, I don't want to place undue burden on this crate at all since the use case in backtrace is pretty particular and specific. I'd be happy to put most of the code in the backtrace crate itself, but I do still think it'd be nice to share pieces where possible. For example would it be possible to have a stripped-down version of this crate with just the ability to read the headers/etc? That's sort of what goblin gives today, and is largely all that backtrace would want. That way backtrace could read/decode the fields it wants but otherwise leave the rest alone. It'd be on the backtrace crate to handle each file format in a unified manner rather than having the object crate present a cross-platform interface too.

In any case curious what y'all think of this! If this isn't suited well to the object crate that's totally ok too, I'll keep hunting!

@philipc
Copy link
Contributor

philipc commented Apr 21, 2020

This is definitely something I want to support, and I already started working on this as part of rust-lang/backtrace-rs#287, but I haven't had much time lately to finish it off. I'm currently working on some gimli issues, and then I hope to get back to this.

Would it be possible to drop the target-lexicon dependency for this use case? It's not really causing issues but having as few dependencies as possible for libstd I think is better.

Yes, this is something I've been considering.

I think I got ELF working, but it's not necessarily exactly how I imagined it. The two main bits I think which feel a bit odd are that Elf::parse parses a lot more of the elf header than is necessary for the backtrace use case.

This is the bit I've been working on already. I can publish what I've done later, but the idea so far is to split up Elf::parse into smaller functions that only parse what is needed.

I tried to get Windows/COFF/PE working as well, but I unfortunately didn't make it too too far. I found that the cross-platform-ness of the traits in read were a bit of a barrier here because of how different PE was from ELF. For example would it be possible to have a stripped-down version of this crate with just the ability to read the headers/etc?

Yes, initially my goal was to avoid using the traits, and provide lower level functions instead (the same as for splitting up Elf::parse).

Longer term in order to support rust-lang/backtrace-rs#287, I want object to provide the same sort of symbol map functionality that backtrace requires, but using the traits to access that would still be optional.

@philipc
Copy link
Contributor

philipc commented Apr 22, 2020

Here's my WIP backtrace ELF changes and the corresponding object branch. I've tried to avoid doing anything unnecessary (such are relocations). I think only minor thing is the SHT_SYMTAB_SHNDX handling for symbols, since I don't think backtrace cares about which section a symbol is for, but I doubt that has any noticeable effect.

I'm not sure what I still had left to do for the ELF support. My changes still keep the symbol map in backtrace, but I want to see if object can provide the same functionality without adding any overhead. This would definitely need to avoid using the object::Symbol structures. That's not something that needs to be done immediately though.

One change I've done in that commit is to only use the dynamic symbols if the debug symbols aren't present, instead of chaining them. This is the same as what libbacktrace does. Do we ever have a case where some objects in the same binary have stripped symbols, and others don't? I can undo that change if you think it's needed.

@alexcrichton
Copy link
Contributor Author

Oh awesome, thanks for all that!

Your changes look great to me and I'd be happy to work on implementing the macos/windows support with that. For symbols and stuff with ELF, I think mirroring libbacktrace is fine. TBH I'm skirting the edge between "I'm implement this stuff" and "I know nothing about this stuff". I don't know a ton about object files and what everything precisely is so some of what's there is just throwing things at the wall (like chaining the symbols together) as opposed to actually knowing what I'm doing!

The APIs exposed from object look great though on your branch, and that seems like it'd be exactly what backtrace would need to switch over.

@philipc
Copy link
Contributor

philipc commented May 2, 2020

I've got the rough macos/windows support done now (backtrace-rs branch). I'll need to polish the object APIs a bit more though.

@alexcrichton
Copy link
Contributor Author

Nice! That all looks perfect to me, thanks so much for helping out with that!

Also, to clarify, that's compiling against a branch of this crate, right?

@philipc
Copy link
Contributor

philipc commented May 4, 2020

Yeah, still on my branch (which I've force pushed and will continue to force push until it's ready).

alexcrichton added a commit to alexcrichton/object that referenced this issue May 7, 2020
This commit should finish out gimli-rs#215 by removing the dependencies of the
`object` crate if it's built with few enough features. While hopefully
not exercised by most users this should be enough for the `backtrace`
crate to depend on `object` and not pull in any transitive dependencies.
One step closer to moving into libstd!

Closes gimli-rs#215
@philipc
Copy link
Contributor

philipc commented May 8, 2020

Fixed by #216 and #218.

@philipc philipc closed this as completed May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants