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

is_import() incorrectly checks st_value == 0 #288

Open
richlowe opened this issue Oct 19, 2021 · 2 comments · May be fixed by #436
Open

is_import() incorrectly checks st_value == 0 #288

richlowe opened this issue Oct 19, 2021 · 2 comments · May be fixed by #436

Comments

@richlowe
Copy link

You want to be checking whether it's global/weak and has a shndx of 0 (SHN_UNDEF)

@m4b
Copy link
Owner

m4b commented Oct 24, 2021

I believe this is correct; the idea is that it returns whether the symbol is something a dynamic linker could import; relying on section header information, while nice, is not actually required by a dynamic linker (since section headers can be stripped)

@philipc
Copy link
Collaborator

philipc commented Oct 24, 2021

I don't have an opinion on the correct fix, but here's some things to think about.

I'm not sure how much sense it makes to try to classify symbols as an import based on what the dynamic linker does:

  • I don't think the dynamic linker ever needs to classify symbols as imports. It can't iterate through the symbols because there is no dynamic entry for the symbol table size. Instead, the dynamic linker only tries to import symbols that are referenced by relocations.
  • Even if the symbol referenced by the relocation is defined, every symbol with default visibility can ultimately be imported from another library instead.

I tried looking in glibc. It has a condition for determining an export which has portions that check both SHN_UNDEF and st_value == 0, among other things. Note that the st_value == 0 check is combined with SHN_ABS and STT_TLS checks.

@Berrysoft Berrysoft linked a pull request Nov 15, 2024 that will close this issue
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 a pull request may close this issue.

3 participants