bitmapset_fixes_v2.patch

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index e13ecaa155..e0eeefa8bd 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -16,6 +16,15 @@
  * containing only a single word (likely the majority of them) this halves the
  * number of loop condition checks.
  *
+ * Because manipulation of a bitmapset can often be done without having to
+ * reallocate the memory for the set, we support REALLOCATE_BITMAPSETS as a
+ * debug option to have the code *always* reallocate the bitmapset when the
+ * set *could* be reallocated as a result of the modification.  This increases
+ * the likelihood of finding bugs in cases where there's more than one
+ * reference to a set and the additional ones are left pointing to freed
+ * memory.  Such bugs are unlikely to be otherwise found by our regression
+ * tests as the vast majority of bitmapsets will only ever contain a single
+ * bitmapword.
  *
  * Copyright (c) 2003-2023, PostgreSQL Global Development Group
  *
@@ -72,6 +81,45 @@
 #error "invalid BITS_PER_BITMAPWORD"
 #endif
 
+#ifdef USE_ASSERT_CHECKING
+/*
+ * bms_is_valid_set - for cassert builds to check for valid sets
+ */
+static bool
+bms_is_valid_set(const Bitmapset *a)
+{
+	/* NULL is the correct representation of an empty set */
+	if (a == NULL)
+		return true;
+
+	/* check the node tag is set correctly.  Freed pointer, maybe? */
+	if (!IsA(a, Bitmapset))
+		return false;
+
+	/* trailing zero words are not allowed */
+	if (a->words[a->nwords - 1] == 0)
+		return false;
+
+	return true;
+}
+#endif
+
+#ifdef REALLOCATE_BITMAPSETS
+/*
+ * bms_copy_and_free
+ *		Only required in REALLOCATE_BITMAPSETS builds.  Provide a simple way to
+ *		return a freshly allocated set similar to what would happen if the set
+ *		had to be enlarged to add additional words.
+ */
+static Bitmapset *
+bms_copy_and_free(Bitmapset *a)
+{
+	Bitmapset  *c = bms_copy(a);
+
+	bms_free(a);
+	return c;
+}
+#endif
 
 /*
  * bms_copy - make a palloc'd copy of a bitmapset
@@ -82,9 +130,11 @@ bms_copy(const Bitmapset *a)
 	Bitmapset  *result;
 	size_t		size;
 
+	Assert(bms_is_valid_set(a));
+
 	if (a == NULL)
 		return NULL;
-	Assert(IsA(a, Bitmapset));
+
 	size = BITMAPSET_SIZE(a->nwords);
 	result = (Bitmapset *) palloc(size);
 	memcpy(result, a, size);
@@ -99,8 +149,8 @@ bms_equal(const Bitmapset *a, const Bitmapset *b)
 {
 	int			i;
 
-	Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
-	Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL)
@@ -140,8 +190,8 @@ bms_compare(const Bitmapset *a, const Bitmapset *b)
 {
 	int			i;
 
-	Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
-	Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL)
@@ -216,8 +266,8 @@ bms_union(const Bitmapset *a, const Bitmapset *b)
 	int			otherlen;
 	int			i;
 
-	Assert(a == NULL || IsA(a, Bitmapset));
-	Assert(b == NULL || IsA(b, Bitmapset));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL)
@@ -257,8 +307,8 @@ bms_intersect(const Bitmapset *a, const Bitmapset *b)
 	int			resultlen;
 	int			i;
 
-	Assert(a == NULL || IsA(a, Bitmapset));
-	Assert(b == NULL || IsA(b, Bitmapset));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL || b == NULL)
@@ -307,8 +357,8 @@ bms_difference(const Bitmapset *a, const Bitmapset *b)
 	Bitmapset  *result;
 	int			i;
 
-	Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
-	Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL)
@@ -316,8 +366,6 @@ bms_difference(const Bitmapset *a, const Bitmapset *b)
 	if (b == NULL)
 		return bms_copy(a);
 
-	Assert(IsA(a, Bitmapset) && IsA(b, Bitmapset));
-
 	/*
 	 * In Postgres' usage, an empty result is a very common case, so it's
 	 * worth optimizing for that by testing bms_nonempty_difference().  This
@@ -374,8 +422,8 @@ bms_is_subset(const Bitmapset *a, const Bitmapset *b)
 {
 	int			i;
 
-	Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
-	Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL)
@@ -383,8 +431,6 @@ bms_is_subset(const Bitmapset *a, const Bitmapset *b)
 	if (b == NULL)
 		return false;
 
-	Assert(IsA(a, Bitmapset) && IsA(b, Bitmapset));
-
 	/* 'a' can't be a subset of 'b' if it contains more words */
 	if (a->nwords > b->nwords)
 		return false;
@@ -411,8 +457,8 @@ bms_subset_compare(const Bitmapset *a, const Bitmapset *b)
 	int			shortlen;
 	int			i;
 
-	Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
-	Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL)
@@ -424,8 +470,6 @@ bms_subset_compare(const Bitmapset *a, const Bitmapset *b)
 	if (b == NULL)
 		return BMS_SUBSET2;
 
-	Assert(IsA(a, Bitmapset) && IsA(b, Bitmapset));
-
 	/* Check common words */
 	result = BMS_EQUAL;			/* status so far */
 	shortlen = Min(a->nwords, b->nwords);
@@ -477,14 +521,14 @@ bms_is_member(int x, const Bitmapset *a)
 	int			wordnum,
 				bitnum;
 
+	Assert(bms_is_valid_set(a));
+
 	/* XXX better to just return false for x<0 ? */
 	if (x < 0)
 		elog(ERROR, "negative bitmapset member not allowed");
 	if (a == NULL)
 		return false;
 
-	Assert(IsA(a, Bitmapset));
-
 	wordnum = WORDNUM(x);
 	bitnum = BITNUM(x);
 	if (wordnum >= a->nwords)
@@ -509,12 +553,12 @@ bms_member_index(Bitmapset *a, int x)
 	int			result = 0;
 	bitmapword	mask;
 
+	Assert(bms_is_valid_set(a));
+
 	/* return -1 if not a member of the bitmap */
 	if (!bms_is_member(x, a))
 		return -1;
 
-	Assert(IsA(a, Bitmapset));
-
 	wordnum = WORDNUM(x);
 	bitnum = BITNUM(x);
 
@@ -549,8 +593,8 @@ bms_overlap(const Bitmapset *a, const Bitmapset *b)
 	int			shortlen;
 	int			i;
 
-	Assert(a == NULL || IsA(a, Bitmapset));
-	Assert(b == NULL || IsA(b, Bitmapset));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL || b == NULL)
@@ -576,7 +620,7 @@ bms_overlap_list(const Bitmapset *a, const List *b)
 	int			wordnum,
 				bitnum;
 
-	Assert(a == NULL || IsA(a, Bitmapset));
+	Assert(bms_is_valid_set(a));
 
 	if (a == NULL || b == NIL)
 		return false;
@@ -607,8 +651,8 @@ bms_nonempty_difference(const Bitmapset *a, const Bitmapset *b)
 {
 	int			i;
 
-	Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
-	Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL)
@@ -640,11 +684,11 @@ bms_singleton_member(const Bitmapset *a)
 	int			nwords;
 	int			wordnum;
 
+	Assert(bms_is_valid_set(a));
+
 	if (a == NULL)
 		elog(ERROR, "bitmapset is empty");
 
-	Assert(IsA(a, Bitmapset));
-
 	nwords = a->nwords;
 	wordnum = 0;
 	do
@@ -683,9 +727,11 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 	int			nwords;
 	int			wordnum;
 
+	Assert(bms_is_valid_set(a));
+
 	if (a == NULL)
 		return false;
-	Assert(IsA(a, Bitmapset));
+
 	nwords = a->nwords;
 	wordnum = 0;
 	do
@@ -717,9 +763,11 @@ bms_num_members(const Bitmapset *a)
 	int			nwords;
 	int			wordnum;
 
+	Assert(bms_is_valid_set(a));
+
 	if (a == NULL)
 		return 0;
-	Assert(IsA(a, Bitmapset));
+
 	nwords = a->nwords;
 	wordnum = 0;
 	do
@@ -745,9 +793,11 @@ bms_membership(const Bitmapset *a)
 	int			nwords;
 	int			wordnum;
 
+	Assert(bms_is_valid_set(a));
+
 	if (a == NULL)
 		return BMS_EMPTY_SET;
-	Assert(IsA(a, Bitmapset));
+
 	nwords = a->nwords;
 	wordnum = 0;
 	do
@@ -786,11 +836,13 @@ bms_add_member(Bitmapset *a, int x)
 	int			wordnum,
 				bitnum;
 
+	Assert(bms_is_valid_set(a));
+
 	if (x < 0)
 		elog(ERROR, "negative bitmapset member not allowed");
 	if (a == NULL)
 		return bms_make_singleton(x);
-	Assert(IsA(a, Bitmapset));
+
 	wordnum = WORDNUM(x);
 	bitnum = BITNUM(x);
 
@@ -799,15 +851,8 @@ bms_add_member(Bitmapset *a, int x)
 	{
 		int			oldnwords = a->nwords;
 		int			i;
-#ifdef REALLOCATE_BITMAPSETS
-		Bitmapset  *tmp = a;
 
-		a = (Bitmapset *) palloc(BITMAPSET_SIZE(wordnum + 1));
-		memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
-		pfree(tmp);
-#else
 		a = (Bitmapset *) repalloc(a, BITMAPSET_SIZE(wordnum + 1));
-#endif
 		a->nwords = wordnum + 1;
 		/* zero out the enlarged portion */
 		i = oldnwords;
@@ -816,19 +861,14 @@ bms_add_member(Bitmapset *a, int x)
 			a->words[i] = 0;
 		} while (++i < a->nwords);
 	}
-#ifdef REALLOCATE_BITMAPSETS
-	else
-	{
-		Bitmapset  *tmp = a;
-
-		a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
-		memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
-		pfree(tmp);
-	}
-#endif
 
 	a->words[wordnum] |= ((bitmapword) 1 << bitnum);
+
+#ifdef REALLOCATE_BITMAPSETS
+	return bms_copy_and_free(a);
+#else
 	return a;
+#endif
 }
 
 /*
@@ -843,26 +883,17 @@ bms_del_member(Bitmapset *a, int x)
 {
 	int			wordnum,
 				bitnum;
-#ifdef REALLOCATE_BITMAPSETS
-	Bitmapset  *tmp = a;
-#endif
+
+	Assert(bms_is_valid_set(a));
 
 	if (x < 0)
 		elog(ERROR, "negative bitmapset member not allowed");
 	if (a == NULL)
 		return NULL;
 
-	Assert(IsA(a, Bitmapset));
-
 	wordnum = WORDNUM(x);
 	bitnum = BITNUM(x);
 
-#ifdef REALLOCATE_BITMAPSETS
-	a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
-	memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
-	pfree(tmp);
-#endif
-
 	/* member can't exist.  Return 'a' unmodified */
 	if (unlikely(wordnum >= a->nwords))
 		return a;
@@ -900,8 +931,8 @@ bms_add_members(Bitmapset *a, const Bitmapset *b)
 	int			otherlen;
 	int			i;
 
-	Assert(a == NULL || IsA(a, Bitmapset));
-	Assert(b == NULL || IsA(b, Bitmapset));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL)
@@ -916,13 +947,6 @@ bms_add_members(Bitmapset *a, const Bitmapset *b)
 	}
 	else
 	{
-#ifdef REALLOCATE_BITMAPSETS
-		Bitmapset  *tmp = a;
-
-		a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
-		memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
-		pfree(tmp);
-#endif
 		result = a;
 		other = b;
 	}
@@ -935,7 +959,12 @@ bms_add_members(Bitmapset *a, const Bitmapset *b)
 	} while (++i < otherlen);
 	if (result != a)
 		pfree(a);
+
+#ifdef REALLOCATE_BITMAPSETS
+	return bms_copy_and_free(result);
+#else
 	return result;
+#endif
 }
 
 /*
@@ -955,7 +984,7 @@ bms_add_range(Bitmapset *a, int lower, int upper)
 				ushiftbits,
 				wordnum;
 
-	Assert(a == NULL || IsA(a, Bitmapset));
+	Assert(bms_is_valid_set(a));
 
 	/* do nothing if nothing is called for, without further checking */
 	if (upper < lower)
@@ -975,16 +1004,9 @@ bms_add_range(Bitmapset *a, int lower, int upper)
 	{
 		int			oldnwords = a->nwords;
 		int			i;
-#ifdef REALLOCATE_BITMAPSETS
-		Bitmapset  *tmp = a;
 
-		a = (Bitmapset *) palloc(BITMAPSET_SIZE(uwordnum + 1));
-		memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
-		pfree(tmp);
-#else
 		/* ensure we have enough words to store the upper bit */
 		a = (Bitmapset *) repalloc(a, BITMAPSET_SIZE(uwordnum + 1));
-#endif
 		a->nwords = uwordnum + 1;
 		/* zero out the enlarged portion */
 		i = oldnwords;
@@ -1021,7 +1043,11 @@ bms_add_range(Bitmapset *a, int lower, int upper)
 		a->words[uwordnum] |= (~(bitmapword) 0) >> ushiftbits;
 	}
 
+#ifdef REALLOCATE_BITMAPSETS
+	return bms_copy_and_free(a);
+#else
 	return a;
+#endif
 }
 
 /*
@@ -1033,15 +1059,9 @@ bms_int_members(Bitmapset *a, const Bitmapset *b)
 	int			lastnonzero;
 	int			shortlen;
 	int			i;
-#ifdef REALLOCATE_BITMAPSETS
-	Bitmapset  *tmp = a;
-#endif
 
-	Assert(a == NULL || IsA(a, Bitmapset));
-	Assert(b == NULL || IsA(b, Bitmapset));
-
-	Assert(a == NULL || IsA(a, Bitmapset));
-	Assert(b == NULL || IsA(b, Bitmapset));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL)
@@ -1052,12 +1072,6 @@ bms_int_members(Bitmapset *a, const Bitmapset *b)
 		return NULL;
 	}
 
-#ifdef REALLOCATE_BITMAPSETS
-	a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
-	memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
-	pfree(tmp);
-#endif
-
 	/* Intersect b into a; we need never copy */
 	shortlen = Min(a->nwords, b->nwords);
 	lastnonzero = -1;
@@ -1089,12 +1103,9 @@ Bitmapset *
 bms_del_members(Bitmapset *a, const Bitmapset *b)
 {
 	int			i;
-#ifdef REALLOCATE_BITMAPSETS
-	Bitmapset  *tmp = a;
-#endif
 
-	Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
-	Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL)
@@ -1102,12 +1113,6 @@ bms_del_members(Bitmapset *a, const Bitmapset *b)
 	if (b == NULL)
 		return a;
 
-#ifdef REALLOCATE_BITMAPSETS
-	a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
-	memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
-	pfree(tmp);
-#endif
-
 	/* Remove b's bits from a; we need never copy */
 	if (a->nwords > b->nwords)
 	{
@@ -1160,15 +1165,9 @@ bms_join(Bitmapset *a, Bitmapset *b)
 	Bitmapset  *other;
 	int			otherlen;
 	int			i;
-#ifdef REALLOCATE_BITMAPSETS
-	Bitmapset  *tmp = a;
-#endif
 
-	Assert(a == NULL || IsA(a, Bitmapset));
-	Assert(b == NULL || IsA(b, Bitmapset));
-
-	Assert(a == NULL || IsA(a, Bitmapset));
-	Assert(b == NULL || IsA(b, Bitmapset));
+	Assert(bms_is_valid_set(a));
+	Assert(bms_is_valid_set(b));
 
 	/* Handle cases where either input is NULL */
 	if (a == NULL)
@@ -1176,12 +1175,6 @@ bms_join(Bitmapset *a, Bitmapset *b)
 	if (b == NULL)
 		return a;
 
-#ifdef REALLOCATE_BITMAPSETS
-	a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
-	memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
-	pfree(tmp);
-#endif
-
 	/* Identify shorter and longer input; use longer one as result */
 	if (a->nwords < b->nwords)
 	{
@@ -1231,7 +1224,7 @@ bms_next_member(const Bitmapset *a, int prevbit)
 	int			wordnum;
 	bitmapword	mask;
 
-	Assert(a == NULL || IsA(a, Bitmapset));
+	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return -2;
@@ -1292,7 +1285,7 @@ bms_prev_member(const Bitmapset *a, int prevbit)
 	int			ushiftbits;
 	bitmapword	mask;
 
-	Assert(a == NULL || IsA(a, Bitmapset));
+	Assert(bms_is_valid_set(a));
 
 	/*
 	 * If set is NULL or if there are no more bits to the right then we've
@@ -1337,6 +1330,8 @@ bms_prev_member(const Bitmapset *a, int prevbit)
 uint32
 bms_hash_value(const Bitmapset *a)
 {
+	Assert(bms_is_valid_set(a));
+
 	if (a == NULL)
 		return 0;				/* All empty sets hash to 0 */
 	return DatumGetUInt32(hash_any((const unsigned char *) a->words,
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 16c383ba7f..3920a535d2 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -336,8 +336,9 @@
 /* #define COPY_PARSE_PLAN_TREES */
 
 /*
- * Define this to force Bitmapset reallocation on each modification.  Helps
- * to find hangling pointers to Bitmapset's.
+ * Define this to force Bitmapset reallocation on modifications which *could*
+ * result in the set having to be reallocated.  This helps find dangling
+ * pointers to Bitmapset's.
  */
 /* #define REALLOCATE_BITMAPSETS */