|Hi, Hackers! I noticed that the range assertion in bms_prev_member()
performs the multiplication in signed int arithmetic before converting
its result to unsigned int: (unsigned int) (a->nwords *
BITS_PER_BITMAPWORD) For a Bitmapset capable of containing INT_MAX,
nwords is INT_MAX / BITS_PER_BITMAPWORD + 1. On a 64-bit build this is
33554432, and multiplying it by 64 cannot be represented by int. A small
UBSan test reports: runtime error: signed integer overflow: 33554432 *
64 cannot be represented in type 'int' PostgreSQL normally builds with
-fwrapv, so this does not currently produce an incorrect assertion
result on GCC/Clang builds using the standard project flags.
Nevertheless, the expression relies on signed wraparound despite the
surrounding code intentionally using unsigned arithmetic to handle the
INT_MAX boundary. The attached patch casts nwords before the
multiplication, matching the calculation a few lines below: (unsigned
int) a->nwords * BITS_PER_BITMAPWORD I did not add a regression test
because reaching this boundary requires allocating an approximately 256
MB Bitmapset, and the affected expression is only present in
assertion-enabled builds. The modified bitmapset.c compiles successfully
and git diff --check passes. |
||
--
Best regards,
Lev Nikolaev,
Tantor Labs LLC,
https://tantorlabs.com/
diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index 26f457a3509..4a12b047789 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -1500,7 +1500,7 @@ bms_prev_member(const Bitmapset *a, int prevbit)
return -2;
/* Validate callers didn't give us something out of range */
- Assert(prevbit < 0 || prevbit <= (unsigned int) (a->nwords * BITS_PER_BITMAPWORD));
+ Assert(prevbit < 0 || prevbit <= (unsigned int) a->nwords * BITS_PER_BITMAPWORD);
/*
* Transform -1 (or any negative number) to the highest possible bit we
