Skip to content

Remove dladdr fallback and implementation #317

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

Merged
merged 4 commits into from
May 12, 2020
Merged

Remove dladdr fallback and implementation #317

merged 4 commits into from
May 12, 2020

Conversation

alexcrichton
Copy link
Member

This has been present for a very long time but I believe this has never
actually been that necessary. Non-MSVC platforms all use libbacktrace by
default, and libbacktrace will consult symbol tables of object files to
do what dladdr does, just inside of libbacktrace. Additionally gimli
implements the same logic. I believe that this means that dladdr isn't
necessary for resolving any symbols since our other strategies should
already be doing everything for us.

This commit makes the feature defunkt and otherwise removes the various
forms of fallback to dladdr.

This has been present for a very long time but I believe this has never
actually been that necessary. Non-MSVC platforms all use libbacktrace by
default, and libbacktrace will consult symbol tables of object files to
do what `dladdr` does, just inside of libbacktrace. Additionally gimli
implements the same logic. I believe that this means that `dladdr` isn't
necessary for resolving any symbols since our other strategies should
already be doing everything for us.

This commit makes the feature defunkt and otherwise removes the various
forms of fallback to dladdr.
@alexcrichton
Copy link
Member Author

Looks like the gimli implementation doesn't handle fat objects just yet, so that'll need to be handled first.

//
// It turns out, though, that for system loaded libraries these
// calculations are correct. For native executables, however, it
// appears correct. Lifting some logic from LLDB's source it has
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should one of these correct be incorrect?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, yes indeed.

philipc and others added 3 commits May 11, 2020 22:20
This commit switches the gimli feature from the `goblin` crate to the
`object` crate for parsing object files. The main motivation here is
trimming the dependencies of the `gimli-symbolize` feature to a bare
minimum. The `object` crate itself has no dependencies now and should be
a relatively easy drop-in replacement for the `goblin` crate.
This commit updates the object parsing code for macOS to support fat
libraries. This enables gimli to symbolize addresses coming from system
libraries which are currently installed frequently as fat libraries.

Closes #319
This commit fixes an issue where symbolication of system libraries
didn't work on macOS. Symbolication through the symbol table was always
off by a slide amount for the library. It's not entirely clear why this
kept happening or what was going on, but some poking in LLDB's source
revealed a way we can differentiate and figure out what addresses need
to be looked up in the symbol table. Some more information is contained
in the comments of the commit itself.

Closes #318
@alexcrichton
Copy link
Member Author

Ok, so I'm pretty confident that dladdr is not needed, so I'm going to merge this. When I originally added the dladdr implementation it was ages ago (about the first time I wrote all this). I'm pretty certain that I was calling libbacktrace correctly, misleading me to believe that it was often needed to call dladdr. Nowadays I'm pretty sure that libbacktrace and gimli do everything necessary to implement dladdr, making calling it unnecessary.

We may have a few edge cases to work through in the implementation of parsing symbol tables with gimli (or something like that), but it's something we can handle in time. And of course adding dladdr back in is pretty easy to do.

In any case this has also snowballed a bit to pick up the object switch from goblin as well as improvements targeted at macOS for parsing fat libraries and better handling of symbol tables in system libraries.

Thanks again @philipc for writing the patch to switch to object and helping out with the crate release and PRs merging!

@alexcrichton alexcrichton merged commit 1ffdc8a into master May 12, 2020
@alexcrichton alexcrichton deleted the rm-dladdr branch May 12, 2020 05:24
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 this pull request may close these issues.

2 participants