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

capi: Add object sizes to input types #505

Merged
merged 5 commits into from
Feb 6, 2024
Merged

capi: Add object sizes to input types #505

merged 5 commits into from
Feb 6, 2024

Conversation

danielocfb
Copy link
Collaborator

In order to ensure some level of forward & backward compatibility, this change extends all our input types with a member holding the objects size. Users need to set this member to the size of the type. The library can then ignore unsupported fields (forward compatibility) or provide reasonable defaults for fields not provided (backward compatibility).

With an upcoming change the blazesym-c crate will also optionally
generate additional artifacts as part of its build. To prevent these
files from causing unnecessary rebuilds, make sure to adjust their
modified time stamps accordingly. Please refer to commit c0c3afb
("Eliminate unnecessary rebuilds in build script") for all details.

Signed-off-by: Daniel Müller <deso@posteo.net>
In order to ensure some level of forward & backward compatibility, this
change extends all our input types with a member holding the object
type's size. Users need to set this member to the size of the type. The
library can then ignore unsupported fields (forward compatibility) or
provide reasonable defaults for fields not provided (backward
compatibility).

Signed-off-by: Daniel Müller <deso@posteo.net>
Because blazesym-c aims to be forward compatible, it needs to accept
objects that are larger (i.e., anticipate extensions) than what the
library currently defines. However, the library should not attempt to
satisfy a request that has options present that it does not understand.
To enforce this constraint, put in some checks that make sure that any
bytes past the last currently defined member (but captured by the
type_size field) are set to zero.

Signed-off-by: Daniel Müller <deso@posteo.net>
To allow for a limited set of future extensions, make sure to reserve
some unused space in various output related data types.

Signed-off-by: Daniel Müller <deso@posteo.net>
@danielocfb danielocfb requested a review from anakryiko January 30, 2024 21:59
Copy link

codecov bot commented Jan 30, 2024

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (c51d478) 93.53% compared to head (f96dc6d) 93.50%.
Report is 6 commits behind head on main.

Files Patch % Lines
capi/src/symbolize.rs 94.14% 11 Missing ⚠️
src/dwarf/function.rs 82.60% 8 Missing ⚠️
capi/src/lib.rs 89.47% 2 Missing ⚠️
capi/src/inspect.rs 96.96% 1 Missing ⚠️
capi/src/normalize.rs 97.61% 1 Missing ⚠️
src/dwarf/units.rs 96.96% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #505      +/-   ##
==========================================
- Coverage   93.53%   93.50%   -0.04%     
==========================================
  Files          45       45              
  Lines        7980     8203     +223     
==========================================
+ Hits         7464     7670     +206     
- Misses        516      533      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielocfb
Copy link
Collaborator Author

Well, let me add logic for checking for zero "extensions" as well...

@danielocfb danielocfb marked this pull request as draft January 30, 2024 22:45
@d-e-s-o d-e-s-o marked this pull request as ready for review January 31, 2024 23:00
@d-e-s-o
Copy link
Collaborator

d-e-s-o commented Jan 31, 2024

This should be ready for review now. Can you please take a look @anakryiko ?

@d-e-s-o
Copy link
Collaborator

d-e-s-o commented Feb 1, 2024

I addressed first set of comments (pertaining BLAZE_INPUT macro and separate commit for modified time stamp modification). Let's discuss the rest. Thanks for the review!

@d-e-s-o
Copy link
Collaborator

d-e-s-o commented Feb 5, 2024

Comments should have been addressed now. Can you please take another look when you get a chance, @anakryiko ?

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.

alright, thanks! LGTM.

Apparently the nightly toolchain in recent versions has some bug
pertaining handling of the stdsimd feature. Pin the version we use to a
known good one for the time being and until this issue is resolved.

Signed-off-by: Daniel Müller <deso@posteo.net>
@d-e-s-o d-e-s-o merged commit db01d98 into libbpf:main Feb 6, 2024
31 checks passed
@d-e-s-o d-e-s-o deleted the topic/capi-compat branch February 6, 2024 02:33
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