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

Speed up ELF symbol lookup #1116

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Speed up ELF symbol lookup #1116

wants to merge 1 commit into from

Conversation

wannacu
Copy link

@wannacu wannacu commented Oct 15, 2024

It's too slow to rebuild an ELF object that contains a large number of symbols. This happens because the frequent linear looking up was expensive.

It's too slow to rebuild an ELF object that contains a large number of
symbols. This happens because the frequent linear looking up was
expensive.
@romainthomas
Copy link
Member

Hi @wannacu

Thank you for this interesting PR!.
Some remarks though:

  1. It uses std::string_view in public headers which raises the minimum C++ version from 11 to 17. I would like to stick to C++11 for the API
  2. What if a user add a symbol? From your implementation it seems that the invariant is no longer valid isn't?

@wannacu
Copy link
Author

wannacu commented Oct 20, 2024

Hi @wannacu

Thank you for this interesting PR!. Some remarks though:

  1. It uses std::string_view in public headers which raises the minimum C++ version from 11 to 17. I would like to stick to C++11 for the API
  2. What if a user add a symbol? From your implementation it seems that the invariant is no longer valid isn't?

Thank you for your comment

  1. It is also acceptable to use std::string
  2. If the user adds a valid symbol with a unique name, the corresponding key will also be valid.

@peledins-zimperium
Copy link
Contributor

This is a very important fix and would enable removing symbols en-mass from LIEF.
However, there is a corner case not handled: there can be 2 symbols that go by same name, for example, when same static function is defined in 2 source files, e.g.:

objdump --syms ./a.out 

./a.out:	file format mach-o arm64

SYMBOL TABLE:
0000000100003f70 l     F __TEXT,__text __ZL3barv
0000000100003f98 l     F __TEXT,__text __ZL3barv
0000000100003f84 g     F __TEXT,__text __Z3foov
0000000100000000 g     F __TEXT,__text __mh_execute_header
0000000100003f50 g     F __TEXT,__text _main
0000000100004000 g     O __DATA,__common _z

bar() is static.

@peledins-zimperium
Copy link
Contributor

Oh, and the fix is for ELF only... similar should exist for MachO... and my example above is for machO.

@peledins-zimperium
Copy link
Contributor

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