-
Notifications
You must be signed in to change notification settings - Fork 937
Parse both .rela.dyn and .rela.plt of libraries. #1074
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
Conversation
Also parse R_X86_64_64 relocations in x86-64 builds.
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.
Great job on creating a test case that produces different relocation types in one binary.
src/symbols_linux.cpp
Outdated
| # define R_DIRECT -1 | ||
| #elif defined(__aarch64__) | ||
| # define R_GLOB_DAT R_AARCH64_GLOB_DAT | ||
| # define R_DIRECT -1 |
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.
Isn't there an AArch64 equivalent?
I see it generates R_AARCH64_ABS64
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.
I originally added something for all archs, but given I cannot test them, I focused on x86-64. I can add it for arm64.
| } else if (rel != NULL && relsz != 0) { | ||
| } | ||
|
|
||
| if (rel != NULL && relsz != 0) { |
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.
Can the same function occur in multiple relocations? I believe it can. We don't necessarily need to handle such cases right now, but if this happens, .rela.plt should take precedence.
src/symbols_linux.cpp
Outdated
| ElfSymbol* sym = (ElfSymbol*)(symtab + ELF_R_SYM(r->r_info) * syment); | ||
| if (sym->st_name != 0) { | ||
| _cc->addImport((void**)(base + r->r_offset), strtab + sym->st_name); | ||
| _cc->tryAddImport((void**)(base + r->r_offset), strtab + sym->st_name); |
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.
Maybe just switch the order of these two loops? This will ensure proper precedence automatically without 'override' logic in CodeCache?
This reverts commit 73c334a.
Signed-off-by: Andrei Pangin <1749416+apangin@users.noreply.github.com>
Signed-off-by: Andrei Pangin <1749416+apangin@users.noreply.github.com>
It is possible to have symbols in both
.rela.dynand.rela.pltsections, depending how the symbols are used. Furthermore,.rela.dyncan haveR_X86_64_64relocation type among others.async-profiler did not parse
.rela.dynsection if.rela.pltwas present.Support parsing
.rela.dynR_X86_64_64symbols and both.rela.dynand.rela.pltin the same ELF binary, to enable internal usage (hooking) of the imported functions.Related issues
#1060
Motivation and context
While investigating #1060 and the possible fix in #1065, noticed that
R_X86_64_64relocation type is not supported.How has this been tested?
New test added in
symbolsLinuxTest.cpp.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.