Skip to content

[release/v1] Fix integer underflow in BITCOUNT ... BIT byte-boundary mask (backport #1994) - #1998

Merged
vazois merged 2 commits into
release/v1from
vazois/bitcount-backport-v1
Jul 30, 2026
Merged

[release/v1] Fix integer underflow in BITCOUNT ... BIT byte-boundary mask (backport #1994)#1998
vazois merged 2 commits into
release/v1from
vazois/bitcount-backport-v1

Conversation

@vazois

@vazois vazois commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Backport of #1994 to release/v1.

Summary

Fixes an integer underflow in BITCOUNT ... BIT (BitmapManagerBitCount.cs) that returned non-zero counts for unset bits whenever a BIT-mode range ended on a byte boundary (endOffset % 8 == 7).

The inclusive right bit index was computed as (int)((endOffset + 1) & 7), which wraps to 0 at byte boundaries and produces a wrapping byte-mask underflow. Fixed to use the same formula already used correctly in BitmapManagerBitPos.cs:

int rightBitIndex = (int)(endOffset & 7) + 1;

Tests

  • Added BitmapBitCountBitBoundaryUnderflowTest (byte 0x80) covering single-bit and boundary-crossing BIT ranges.
  • Updated BitmapBitCountLongBitOffsetParsingTest: its previous expectation of 8 was an artifact of the underflow. Under Garnet''s modulo-wrapping negative-offset semantics the clamped start wraps to bit index 1, so bits 1..7 of 0xFF = 7.
  • Verified both tests pass on release/v1 (net10.0). The NormalizeBitCountOffsets / MaxOffsetForBitmapLength / ProcessNegativeOffset semantics are identical on this branch.

Cherry-picked from squash-merge 52aef6e (#1994 on main).

Copilot AI review requested due to automatic review settings July 29, 2026 23:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR backports the fix for a BITCOUNT ... BIT edge case where ranges ending exactly on a byte boundary could produce a wrapping mask and incorrectly count unset bits as set.

Changes:

  • Fix BitmapManagerBitCount.BitIndexCount(byte* ...) to compute the right (exclusive) bit index as (endOffset & 7) + 1, preventing byte-boundary wrap to 0.
  • Add a regression test for the byte-boundary underflow scenario (including single-bit and boundary-crossing ranges).
  • Update an existing long-negative-offset parsing test expectation to reflect correct modulo-wrapping semantics after the fix.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/Garnet.test/GarnetBitmapTests.cs Adds a regression test for the byte-boundary BIT underflow and updates an existing assertion impacted by the corrected mask logic.
libs/server/Resp/Bitmap/BitmapManagerBitCount.cs Fixes right-bit-index calculation to avoid byte-boundary wraparound and mask underflow in BIT-mode counting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@vazois
vazois merged commit 67ab71d into release/v1 Jul 30, 2026
47 of 49 checks passed
@vazois
vazois deleted the vazois/bitcount-backport-v1 branch July 30, 2026 20:46
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