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

vdef: introduce v_counted_by_(field) #4039

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

asadsa92
Copy link
Contributor

A new attribute can be used for flexible arrays to make the compiler smarter:

This would improve the array bound sanitizer.

Note: this has not been tested locally, but this should be safe until a
compiler supports it. Once supported, we would detect and make the adjustments
as needed.

A new attribute can be used for flexible arrays to make the compiler smarter:
- https://clang.llvm.org/docs/AttributeReference.html#counted-by
- https://gcc.gnu.org/pipermail/gcc-patches/2023-August/628459.html

This would improve the array bound sanitizer.

Note: this has not been tested locally, but this should be safe until a
compiler supports it. Once supported, we would detect and make the adjustments
as needed.

Signed-off-by: Asad Sajjad Ahmed <asadsa@varnish-software.com>
Signed-off-by: Asad Sajjad Ahmed <asadsa@varnish-software.com>
@nigoroll
Copy link
Member

I am generally 👍🏽 , this could potentially increase our code quality. BUT

this has not been tested locally

locally installing gcc-trunk and testing the changes should not be asking too much, rather than putting the burdon on a future incarnation of some other maintainer.

@dridi dridi marked this pull request as draft January 23, 2024 09:11
@asadsa92
Copy link
Contributor Author

this has not been tested locally

locally installing gcc-trunk and testing the changes should not be asking too much, rather than putting the burdon on a future incarnation of some other maintainer.

I agree with the reasoning to test it before merge to not uncover work later on, I was partly thinking this could be easily tested in CCI. The attribute is maybe too new. Anyways, thanks, I will look into it.

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.

None yet

2 participants