KEMBAR78
Parse both .rela.dyn and .rela.plt of libraries. by krk · Pull Request #1074 · async-profiler/async-profiler · GitHub
Skip to content

Conversation

@krk
Copy link
Contributor

@krk krk commented Dec 6, 2024

It is possible to have symbols in both .rela.dyn and .rela.plt sections, depending how the symbols are used. Furthermore, .rela.dyn can have R_X86_64_64 relocation type among others.

async-profiler did not parse .rela.dyn section if .rela.plt was present.

Support parsing .rela.dyn R_X86_64_64 symbols and both .rela.dyn and .rela.plt in 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_64 relocation 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.

Also parse R_X86_64_64 relocations in x86-64 builds.
Copy link
Member

@apangin apangin left a 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.

# define R_DIRECT -1
#elif defined(__aarch64__)
# define R_GLOB_DAT R_AARCH64_GLOB_DAT
# define R_DIRECT -1
Copy link
Member

@apangin apangin Dec 6, 2024

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

Copy link
Contributor Author

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) {
Copy link
Member

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.

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);
Copy link
Member

@apangin apangin Dec 6, 2024

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?

krk and others added 5 commits December 6, 2024 19:12
Signed-off-by: Andrei Pangin <1749416+apangin@users.noreply.github.com>
Signed-off-by: Andrei Pangin <1749416+apangin@users.noreply.github.com>
@apangin apangin merged commit 7e57061 into async-profiler:master Dec 7, 2024
10 checks passed
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