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

read/elf: exclude zero sized STT_NOTYPE symbols from is_definitions #601

Merged
merged 3 commits into from
Nov 29, 2023

Conversation

philipc
Copy link
Contributor

@philipc philipc commented Nov 26, 2023

These are generally markers, not definitions. In particular, this excludes ARM mapping symbols ('$*').

Closes #599
See also #447, but further work is still needed to fully address this issue.

We'll need this for testing at least, and it's probably better in general.
Ideally there wouldn't be duplicates, but they do happen.
@pcc
Copy link
Contributor

pcc commented Nov 27, 2023

This will also exclude hand-written assembly when written without directives.

.globl foo
foo:
...

However, this seems unlikely to be a problem in practice. I checked an arm64 Linux kernel binary that I had to hand (probably the heaviest common user of hand-written assembly) for 0-sized non-$-prefixed NOTYPE symbols, like so:

readelf -sW path/to/vmlinux |grep '\b0 NOTYPE' | grep -v ' \$' | less

Most all of the symbols appeared to be "marker symbols", but there were a few exceptions, e.g.

 11402: ffff800080086800     0 NOTYPE  LOCAL  DEFAULT    2 el1_sync
 11404: ffff800080086844     0 NOTYPE  LOCAL  DEFAULT    2 el1_trap
 11405: ffff800080086830     0 NOTYPE  LOCAL  DEFAULT    2 wa_epilogue
 11408: ffff800080086864     0 NOTYPE  LOCAL  DEFAULT    2 el1_irq
 11409: ffff800080086864     0 NOTYPE  LOCAL  DEFAULT    2 el1_fiq
 11410: ffff800080086884     0 NOTYPE  LOCAL  DEFAULT    2 el1_error
 11411: ffff8000800868a4     0 NOTYPE  LOCAL  DEFAULT    2 el2_sync
 11412: ffff800080086920     0 NOTYPE  LOCAL  DEFAULT    2 el2_error

(These were all function symbols.)

So it seems like folks usually get this right, and users can always fix their assembly to get things working again (on ARM it is an ABI requirement to use the correct symbol type). So I reckon this approach is fine to start with.

@philipc
Copy link
Contributor Author

philipc commented Nov 27, 2023

Thanks for checking that. We may be able to fix those hand assembly cases with better prioritization logic as in #447 (comment), and the ARM mapping symbol names are a candidate for use in prioritization.

@bjorn3
Copy link
Contributor

bjorn3 commented Nov 28, 2023

Marker symbols can still be linked against. Shouldn't this method include all symbols you can link against?

@philipc
Copy link
Contributor Author

philipc commented Nov 28, 2023

From the documentation for is_definition: Return true if the symbol is a definition of a function or data object that has a known address.

Currently the purpose of the is_definition method is to assist with finding all the symbols that should be present in a mapping from address to symbol. I don't think marker symbols fit in that category.

Determining which symbols can be linked against isn't something I have thought about before. I'd have to look at some linkers to see exactly which symbols fit in that category for each file format. Do you have a need for that?

These are generally markers, not definitions.
In particular, this excludes ARM mapping symbols ('$*').
@philipc philipc merged commit 8d82d96 into gimli-rs:master Nov 29, 2023
12 checks passed
@philipc philipc deleted the elf-def branch November 29, 2023 05:54
mcbegamerxx954 pushed a commit to mcbegamerxx954/object that referenced this pull request Jun 15, 2024
read/elf: exclude zero sized STT_NOTYPE symbols from is_definitions
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