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

Double the width of static allocations for empty tables #596

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

clarfonthey
Copy link
Contributor

See this discussion here: #578 (comment)

Effectively, the current code will actually need Group::WIDTH + 1 bytes for an empty table, but we only allocate Group::WIDTH bytes for it. This could potentially invoke UB based upon the invariants we set in the table, but because we happen to short-circuit empty tables in just the right ways in the actual code, we never actually trigger it.

Since the static allocations are aligned to the group width, we double the size returned, since it would otherwise just add that much extra padding anyway. And since the size of the static allocations added by this is effectively insignificant (8-16 bytes depending on the implementation chosen), it's fair to just do this anyway to make the code more robust.

@Amanieu
Copy link
Member

Amanieu commented Dec 10, 2024

I still don't think this is needed: the only place that needs WIDTH instead of WIDTH - 1 trailing control bytes is set_ctrl, and that requires write access which doesn't work with the static empty table anyways. The only other places which access the trailing control bytes are where Group::load is called and that only accesses WIDTH - 1 trailing bytes since the input index is always less than num_buckets.

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.

2 participants