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

0-length arrays in Buddy ranges #672

Merged
merged 3 commits into from
Sep 9, 2024
Merged

0-length arrays in Buddy ranges #672

merged 3 commits into from
Sep 9, 2024

Conversation

nwf-msr
Copy link
Contributor

@nwf-msr nwf-msr commented Sep 4, 2024

Buddy ranges can be instantiated with MAX_SIZE_BITS == MIN_SIZE_BITS (or, in principle, <=). If this happens, the resulting class will contain 0-length arrays, and code that appears to attempt to access an element therein trips gcc's -Warray-bounds.

This PR contains two commits: one that adds a static_assert to ensure that Buddy's arrays are always of positive size, and one possible way to fix the case I'm tripping over. The latter works because the Buddy's MIN_SIZE_BITS is instantiated at MIN_CHUNK_BITS, and so the change herein then ensures that its MAX_SIZE_BITS is strictly greater than MIN_CHUNK_BITS.

A different, and possibly better, solution would be to drop the LargeBuddyRange from CentralMetaRange when max_page_chunk_size_bits is computed to be equal to MIN_CHUNK_BITS.

Please advise.

@nwf-msr nwf-msr requested a review from mjp41 September 4, 2024 18:19
This fixes the 0-length arrays discussed (and made into assertion failures) in
the next commit.  This works because the Buddy's `MIN_SIZE_BITS` is instantiated
at `MIN_CHUNK_BITS`, and so we ensure that we instantiate the LargeBuddyRange
only with `max_page_chunk_size_bits` above `MIN_CHUNK_BITS`.
Now that the case leading to several 0-sized arrays in Buddy ranges, that then
cause gcc's -Warray-bounds to trip, has been removed, add a static assert so
that we can catch this with better error messages next time.
Copy link
Member

@mjp41 mjp41 left a comment

Choose a reason for hiding this comment

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

LGTM

@nwf
Copy link
Collaborator

nwf commented Sep 5, 2024

You OK to merge even though macos CI is still pending?

@mjp41
Copy link
Member

mjp41 commented Sep 5, 2024

You OK to merge even though macos CI is still pending?

Yeah. I think Mac os 11 support has been removed. I think we need to remove and possibly add the next version

@nwf-msr nwf-msr merged commit 7fbca11 into microsoft:main Sep 9, 2024
45 of 47 checks passed
@nwf-msr nwf-msr deleted the 202409-gcc-0-arrays branch September 9, 2024 16:25
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