Skip to content
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

Improve handling of dynamic ELF symbols #450

Closed
wants to merge 5 commits into from

Conversation

d-e-s-o
Copy link
Collaborator

@d-e-s-o d-e-s-o commented Dec 19, 2023

Please see individual commits for descriptions.

There can be overlap between dynamic and static symbols. For us that
means that when iterating over all symbols we would see duplicates being
reported. On top of that, we waste memory by having the same information
available in two arrays.
With this change we make sure to filter out any dynamic symbols that are
already present as static symbols to fix both issues.

Signed-off-by: Daniel Müller <deso@posteo.net>
@d-e-s-o d-e-s-o requested a review from anakryiko December 19, 2023 00:24
This change introduces the SymbolTableCache type, as a helper for
working with symbol tables. In the future this type will allow us to
more easily add proper support for dynamic symbol lookup.

Signed-off-by: Daniel Müller <deso@posteo.net>
With the introduction and usage of OnceCell a while back, we can
simplify the ElfParser::for_each_sym() method by using straight
iteration.

Signed-off-by: Daniel Müller <deso@posteo.net>
Since basically its inception (85b0f16), support for dynamic
symbols in any shape or form has been flawed: we only considered dynamic
symbols as a fallback if no static (?) symbols are present. However, it
is entirely possible to have exclusive dynamic symbols in use, which
would be ignored entirely.
This change fixes this shortcoming, by considering both tables when
looking up symbols from address or by name.

Signed-off-by: Daniel Müller <deso@posteo.net>
This change adds a test for the lookup of a dynamic symbol by name as
well as by address.

Signed-off-by: Daniel Müller <deso@posteo.net>
@d-e-s-o d-e-s-o force-pushed the topic/dynsyms branch 2 times, most recently from c9a2b9e to 65d6e65 Compare December 20, 2023 15:24
Copy link
Member

@anakryiko anakryiko left a comment

Choose a reason for hiding this comment

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

Makes sense, LGTM.

src/elf/parser.rs Show resolved Hide resolved
@danielocfb danielocfb mentioned this pull request Jan 2, 2024
@danielocfb
Copy link
Collaborator

Superseded by #472.

@danielocfb danielocfb closed this Jan 2, 2024
@danielocfb danielocfb deleted the topic/dynsyms branch January 2, 2024 21:55
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.

3 participants