Skip to content

Fix BITFIELD signed 64-bit overflow detection for OVERFLOW SAT/FAIL #1968

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.

Sign up for GitHub

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

Merged
vazois merged 2 commits into from
Jul 22, 2026
+58 −8
Merged

Fix BITFIELD signed 64-bit overflow detection for OVERFLOW SAT/FAIL #1968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d123d73
Fix BITFIELD signed 64-bit overflow detection for OVERFLOW SAT/FAIL
hexonal Jul 22, 2026
3ac5c62
Merge branch 'main' into fix-bitfield-64bit-overflow
vazois Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions libs/server/Resp/Bitmap/BitmapManagerBitfield.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,12 @@ private static bool CheckBitfieldOverflow(long value, long incrBy, out long resu
//underflow if sign bit is zero
var underflow = (result & signbit) == 0 && value < 0 && incrBy < 0;
//if operands are both positive possibility of overflow
//overflow if any of the 64-bitcount most significant bits are set.
var overflow = (ulong)(result & ~mask) > 0 && value >= 0 && incrBy > 0;
//overflow if any of the 64-bitcount most significant bits are set. At
//bitCount == 64 there are no such bits to check (mask covers the whole
//word), so overflow instead shows up as the sign bit flipping negative.
var overflow = bitCount == 64
? result < 0 && value >= 0 && incrBy > 0
: (ulong)(result & ~mask) > 0 && value >= 0 && incrBy > 0;

switch (overflowType)
{
Expand Down
58 changes: 52 additions & 6 deletions test/standalone/Garnet.test.complexstring/GarnetBitmapTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1513,7 +1513,7 @@ private static (long, bool) CheckSignedBitfieldOverflowRedis(long value, long in
switch (overflowType)
{
case 0://wrap
if ((bitCount < 64 && incrBy > maxAdd) || (value >= 0 && incrBy > 0 && incrBy > maxVal) ||
if ((bitCount < 64 && incrBy > maxAdd) || (bitCount == 64 && value >= 0 && incrBy > 0 && (value + incrBy) < 0) ||
((bitCount < 64 && incrBy < maxSub) || (value < 0 && incrBy < 0 && incrBy < maxSub)))
{
ulong signb = 1UL << (bitCount - 1);
Expand All @@ -1530,13 +1530,13 @@ private static (long, bool) CheckSignedBitfieldOverflowRedis(long value, long in
}
return ((value + incrBy), false);
case 1://sat
if ((bitCount < 64 && incrBy > maxAdd) || (value >= 0 && incrBy > 0 && incrBy > maxVal))
if ((bitCount < 64 && incrBy > maxAdd) || (bitCount == 64 && value >= 0 && incrBy > 0 && (value + incrBy) < 0))
return (maxVal, true);
if ((bitCount < 64 && incrBy < maxSub) || (value < 0 && incrBy < 0 && incrBy < maxSub))
return (minVal, true);
return ((value + incrBy), false);
case 2://fail // detect overflow/underflow do not do anything else
if ((bitCount < 64 && incrBy > maxAdd) || (value >= 0 && incrBy > 0 && incrBy > maxVal) ||
if ((bitCount < 64 && incrBy > maxAdd) || (bitCount == 64 && value >= 0 && incrBy > 0 && (value + incrBy) < 0) ||
((bitCount < 64 && incrBy < maxSub) || (value < 0 && incrBy < 0 && incrBy < maxSub)))
return (0, true);
return ((value + incrBy), false);
Expand All @@ -1554,8 +1554,12 @@ private static (long, bool) CheckSignedBitfieldOverflow(long value, long incrBy,
//underflow if sign bit is zero
bool underflow = (result & signbit) == 0 && value < 0 && incrBy < 0;
//if operands are both positive possibility of overflow
//overflow if any of the 64-bitcount most significant bits are set.
bool overflow = (ulong)(result & ~mask) > 0 && value >= 0 && incrBy > 0;
//overflow if any of the 64-bitcount most significant bits are set. At
//bitCount == 64 there are no such bits to check (mask covers the whole word),
//so overflow instead shows up as the sign bit flipping negative.
bool overflow = bitCount == 64
? result < 0 && value >= 0 && incrBy > 0
: (ulong)(result & ~mask) > 0 && value >= 0 && incrBy > 0;

switch (overflowType)
{
Expand Down Expand Up @@ -2543,7 +2547,49 @@ public void BitmapBitfieldBoundaryTest()
ClassicAssert.AreEqual(expected: new byte[] { 0x80, 0x80 }, actual: result);
}

[Order(41)]
/// <summary>
/// Regression test for a signed 64-bit-specific gap in overflow detection: at
/// bitCount == 64 there are no "extra" high bits above the field to check (the field
/// spans the whole word), so the general-purpose overflow check silently never fired,
/// making OVERFLOW SAT clamp to nothing (silently wrapping instead) and OVERFLOW FAIL
/// silently succeed (instead of rejecting the write) whenever a signed 64-bit INCRBY
/// actually overflowed. WRAP was unaffected, since its result value already comes from
/// plain two's-complement long addition regardless of the overflow flag.
/// </summary>
[Test, Order(41)]
[Category("BITFIELD")]
public void BitmapBitfieldSigned64OverflowTest()
{
using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());
var db = redis.GetDatabase(0);
var key = "BitmapBitfieldSigned64OverflowTest";

// Overflow direction: SAT clamps to long.MaxValue and stores the clamped value.
db.Execute("BITFIELD", (RedisKey)key, "SET", "i64", 0, long.MaxValue - 5);
var satResult = (long)db.Execute("BITFIELD", (RedisKey)key, "OVERFLOW", "SAT", "INCRBY", "i64", 0, 100);
ClassicAssert.AreEqual(long.MaxValue, satResult);
ClassicAssert.AreEqual(long.MaxValue, (long)db.Execute("BITFIELD", (RedisKey)key, "GET", "i64", 0));

// Overflow direction: FAIL reports the overflow via a nil reply. (Whether the stored
// value itself is left untouched on FAIL is a separate, pre-existing question that
// applies at every bitCount, not something this fix changes - not asserted here.)
db.Execute("BITFIELD", (RedisKey)key, "SET", "i64", 0, long.MaxValue - 5);
var failResult = (RedisResult[])db.Execute("BITFIELD", (RedisKey)key, "OVERFLOW", "FAIL", "INCRBY", "i64", 0, 100);
ClassicAssert.IsTrue(failResult[0].IsNull);

// Underflow direction: SAT clamps to long.MinValue and stores the clamped value.
db.Execute("BITFIELD", (RedisKey)key, "SET", "i64", 0, long.MinValue + 5);
var satUnderflowResult = (long)db.Execute("BITFIELD", (RedisKey)key, "OVERFLOW", "SAT", "INCRBY", "i64", 0, -100);
ClassicAssert.AreEqual(long.MinValue, satUnderflowResult);
ClassicAssert.AreEqual(long.MinValue, (long)db.Execute("BITFIELD", (RedisKey)key, "GET", "i64", 0));

// Underflow direction: FAIL reports the overflow via a nil reply.
db.Execute("BITFIELD", (RedisKey)key, "SET", "i64", 0, long.MinValue + 5);
var failUnderflowResult = (RedisResult[])db.Execute("BITFIELD", (RedisKey)key, "OVERFLOW", "FAIL", "INCRBY", "i64", 0, -100);
ClassicAssert.IsTrue(failUnderflowResult[0].IsNull);
}

[Order(42)]
[Test]
[Category("BITFIELD")]
public void BitmapBitFieldInvalidOptionsTest([Values(RespCommand.BITFIELD, RespCommand.BITFIELD_RO)] RespCommand testCmd)
Expand Down
Loading