-
Notifications
You must be signed in to change notification settings - Fork 65
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 symbols lookup #93
Conversation
This commit improves how the symbols like solist are searched. This allows to phones that have linker64 with symbols with llvm suffix to still have maps hiding. closes #63
d25d6a4
to
f7b7615
Compare
Don't do linear lookup at the begining, try ElfLookup or GnuLookup could be faster. |
Sorry, my bad, hash algorithm won't work when the key is unknown. We indeed need linear searching. |
Anything to add up? Or do you think we can modify this code to make it more readable? Seems a bit clogged to me, but I don't think there's a way to improve. |
I'll read it later. Don't rush, I'll give my review. |
if (auto i = symtabs_.find(name); i != symtabs_.end()) { | ||
return i->second->st_value; | ||
} else { | ||
return 0; | ||
} | ||
} | ||
|
||
/* INFO: This expects it to only have one find. This is done as | ||
we only need for that and further complexity to add | ||
support for more than one find is bad (for us). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve the sentence be clearer.
} | ||
|
||
for (auto symtab : symtabs_) { | ||
if (symtab.first.size() < name.size()) continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name.size()
value can be cached.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those methods are O(1). So I suppose they are inside the string already so that there's no "overhead" in getting them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you are inside a for
loop (quite big), so this size is computed handreds of times.
snprintf(somain_sym_name, sizeof(somain_sym_name), "__dl__ZL6somain%s", llvm_sufix); | ||
|
||
char get_realpath_sym_name[sizeof("__dl__ZNK6soinfo12get_realpathEv") + sizeof(llvm_sufix)]; | ||
snprintf(get_realpath_sym_name, sizeof(get_realpath_sym_name), "__dl__ZNK6soinfo12get_realpathEv%s", llvm_sufix); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No suffix needed for get_realpath_sym_name
|
||
char get_soname_sym_name[sizeof("__dl__ZNK6soinfo10get_sonameEv") + sizeof(llvm_sufix)]; | ||
snprintf(get_soname_sym_name, sizeof(get_soname_sym_name), "__dl__ZNK6soinfo10get_sonameEv%s", llvm_sufix); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither for get_soname_sym_name
Superseded by #95. |
Changes
This commit improves how the symbols like solist are searched, to search by its prefix instead of full name.
Why
This allows to phones that have linker64 with symbols with llvm suffix to still have maps hiding.
Checkmarks