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

Rename Binary meta data variant to Elf #222

Merged
merged 10 commits into from
Jun 9, 2023

Conversation

danielocfb
Copy link
Collaborator

This change renames the Binary meta data variant to Elf. For the time being we decided to err on the side of having more specific variants, which may be easier to extend in a sensible way in the future (e.g., if future supported binary formats don't have a build ID it's more natural to have a new variant as opposed to fudging everything as a "binary" and then having optional fields).

@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Patch coverage: 84.63% and project coverage change: +84.38 🎉

Comparison is base (21498b6) 0.00% compared to head (b8c45f4) 84.38%.

Additional details and impacted files
@@            Coverage Diff            @@
##           main     #222       +/-   ##
=========================================
+ Coverage      0   84.38%   +84.38%     
=========================================
  Files         0       26       +26     
  Lines         0     3754     +3754     
=========================================
+ Hits          0     3168     +3168     
- Misses        0      586      +586     
Impacted Files Coverage Δ
src/c_api/inspect.rs 95.47% <ø> (ø)
src/c_api/symbolize.rs 88.54% <ø> (ø)
src/lib.rs 77.77% <ø> (ø)
src/mmap.rs 96.15% <ø> (ø)
src/normalize/normalizer.rs 82.89% <78.16%> (ø)
src/symbolize/symbolizer.rs 54.66% <84.21%> (ø)
src/c_api/normalize.rs 90.00% <95.14%> (ø)
src/normalize/meta.rs 100.00% <100.00%> (ø)
src/zip.rs 83.59% <100.00%> (ø)

... and 17 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

This change adds a test for the APK address normalization logic.

Signed-off-by: Daniel Müller <deso@posteo.net>
@danielocfb danielocfb force-pushed the topic/elf-meta-data branch 2 times, most recently from ac10db4 to 85f45b5 Compare June 8, 2023 20:39
@danielocfb
Copy link
Collaborator Author

Adding a bunch more stuff here that we will need for APK address normalization but which hinges on the original change this PR was created for.

Now that we use the zip module as part of normalization and
symbolization, let's remove the #[allow(unused)] annotation.

Signed-off-by: Daniel Müller <deso@posteo.net>
This change renames the Binary meta data variant to Elf. For the time
being we decided to err on the side of having more specific variants,
which may be easier to extend in a sensible way in the future (e.g., if
future supported binary formats don't have a build ID it's more natural
to have a new variant as opposed to fudging everything as a "binary" and
then having optional fields).

Signed-off-by: Daniel Müller <deso@posteo.net>
This change renames the read_build_id() function to
read_build_id_from_elf, as we will need to introduce a few more variants
accepting different arguments in the future.

Signed-off-by: Daniel Müller <deso@posteo.net>
This change reintroduced the read_build_id() function that we renamed
earlier and which now accepts an ElfParser instance. Having this variant
available will be necessary in the future, but it's a reasonable
separation of concerns even now.

Signed-off-by: Daniel Müller <deso@posteo.net>
This change introduces an internal trait, BuildIdReader, that replaces
the current usage of the get_build_id BuildIdFn member in
NormalizationHandler objects. Basically, with upcoming changes we need
to keep around more than one variant of a build-id-reader function and
the trait clubs those together for easier handling.

Signed-off-by: Daniel Müller <deso@posteo.net>
This change factors out a small helper function, make_elf_meta(), that
takes care of creating a UserAddrMeta for an ELF file.

Signed-off-by: Daniel Müller <deso@posteo.net>
With upcoming changes we will add support for normalizing addresses
belonging to ELF binaries that reside in APKs to the crate. Such
addresses require additional meta data as output of the address
normalization process, which includes the APK path.
This change introduces a new meta data variant, ApkElf, that we will use
for representing such addresses.

Signed-off-by: Daniel Müller <deso@posteo.net>
This patch prepares the code base (the normalizer and symbolizer logic
specifically) for the addition of APK support. Most importantly, we
introduce and hook up stubs that will later be used for dealing with
APKs, in addition to the existing logic that works solely on ELF files.

Signed-off-by: Daniel Müller <deso@posteo.net>
This change adds support for normalizing addresses in APKs, which has
relevance particularly for usage on Android. In a nutshell, we:
- detect whether an address lies inside an APK by checking the
  respective maps entry for the .apk/.zip extension
- we then calculate the file offset for the address and look up the
  corresponding member inside the APK
- we then perform ELF normalization on said address using the discovered
  APK entry
- lastly we convey the result to the user via the ApkElf meta data,
  which was introduced earlier

Signed-off-by: Daniel Müller <deso@posteo.net>
@danielocfb
Copy link
Collaborator Author

Added the remaining APK normalization stuff now as well, as it all builds on top of each other.

Copy link
Member

@anakryiko anakryiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@d-e-s-o d-e-s-o merged commit cc40026 into libbpf:main Jun 9, 2023
@d-e-s-o d-e-s-o deleted the topic/elf-meta-data branch June 9, 2023 05:30
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