On 11.10.22 18:04, Tom Lane wrote:
Peter Eisentraut <peter.eisentr...@enterprisedb.com> writes:
On 14.09.22 06:53, Tom Lane wrote:
Actually ... an even-more-terrifyingly-plausible misuse is that the
supplied oldsize is different from the actual previous allocation.
We should try to check that. In MEMORY_CONTEXT_CHECKING builds
it should be possible to assert that oldsize == requested_size.
We don't have that data if !MEMORY_CONTEXT_CHECKING, but we could
at least assert that oldsize <= allocated chunk size.
I'm not very familiar with MEMORY_CONTEXT_CHECKING. Where would one get
these values?
Hmm ... the individual allocators have that info, but mcxt.c doesn't
have access to it. I guess we could invent an additional "method"
to return the requested size of a chunk, which is only available in
MEMORY_CONTEXT_CHECKING builds, or maybe in !MEMORY_CONTEXT_CHECKING
it returns the allocated size instead.
I'm not sure whether that amount of additional work would be useful
relative to the size of this patch. Is the patch as it stands now
making the code less robust than what the code is doing now?
In the meantime, here is an updated patch with the argument order
swapped and an additional assertion, as previously discussed.
From cc0069bf2b0b3d40dda462d441af5d8b07ef1f57 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 31 Oct 2022 09:17:57 +0100
Subject: [PATCH v2] Add repalloc0 and repalloc0_array
These zero out the space added by repalloc. This is a common pattern
that is quite hairy to code by hand.
Discussion:
https://www.postgresql.org/message-id/b66dfc89-9365-cb57-4e1f-b7d31813e...@enterprisedb.com
---
src/backend/executor/nodeHash.c | 8 ++-----
src/backend/libpq/be-fsstubs.c | 9 +++-----
src/backend/optimizer/util/placeholder.c | 13 ++++-------
src/backend/optimizer/util/relnode.c | 29 +++++++-----------------
src/backend/parser/parse_param.c | 10 +++-----
src/backend/storage/lmgr/lwlock.c | 9 ++------
src/backend/utils/adt/ruleutils.c | 9 ++------
src/backend/utils/cache/typcache.c | 10 ++------
src/backend/utils/mmgr/mcxt.c | 18 +++++++++++++++
src/include/utils/palloc.h | 2 ++
10 files changed, 46 insertions(+), 71 deletions(-)
diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index 6622b202c229..2e6cce4802e5 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -940,12 +940,8 @@ ExecHashIncreaseNumBatches(HashJoinTable hashtable)
else
{
/* enlarge arrays and zero out added entries */
- hashtable->innerBatchFile =
repalloc_array(hashtable->innerBatchFile, BufFile *, nbatch);
- hashtable->outerBatchFile =
repalloc_array(hashtable->outerBatchFile, BufFile *, nbatch);
- MemSet(hashtable->innerBatchFile + oldnbatch, 0,
- (nbatch - oldnbatch) * sizeof(BufFile *));
- MemSet(hashtable->outerBatchFile + oldnbatch, 0,
- (nbatch - oldnbatch) * sizeof(BufFile *));
+ hashtable->innerBatchFile =
repalloc0_array(hashtable->innerBatchFile, BufFile *, oldnbatch, nbatch);
+ hashtable->outerBatchFile =
repalloc0_array(hashtable->outerBatchFile, BufFile *, oldnbatch, nbatch);
}
MemoryContextSwitchTo(oldcxt);
diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c
index 3e5cada7eb53..106fdcdf817b 100644
--- a/src/backend/libpq/be-fsstubs.c
+++ b/src/backend/libpq/be-fsstubs.c
@@ -696,19 +696,16 @@ newLOfd(void)
newsize = 64;
cookies = (LargeObjectDesc **)
MemoryContextAllocZero(fscxt, newsize *
sizeof(LargeObjectDesc *));
- cookies_size = newsize;
}
else
{
/* Double size of array */
i = cookies_size;
newsize = cookies_size * 2;
- cookies = (LargeObjectDesc **)
- repalloc(cookies, newsize * sizeof(LargeObjectDesc *));
- MemSet(cookies + cookies_size, 0,
- (newsize - cookies_size) * sizeof(LargeObjectDesc
*));
- cookies_size = newsize;
+ cookies =
+ repalloc0_array(cookies, LargeObjectDesc *,
cookies_size, newsize);
}
+ cookies_size = newsize;
return i;
}
diff --git a/src/backend/optimizer/util/placeholder.c
b/src/backend/optimizer/util/placeholder.c
index c7bfa293c9ff..c55027377fe7 100644
--- a/src/backend/optimizer/util/placeholder.c
+++ b/src/backend/optimizer/util/placeholder.c
@@ -133,16 +133,11 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar
*phv)
while (phinfo->phid >= new_size)
new_size *= 2;
if (root->placeholder_array)
- {
- root->placeholder_array = (PlaceHolderInfo **)
- repalloc(root->placeholder_array,
- sizeof(PlaceHolderInfo *) *
new_size);
- MemSet(root->placeholder_array +
root->placeholder_array_size, 0,
- sizeof(PlaceHolderInfo *) * (new_size -
root->placeholder_array_size));
- }
+ root->placeholder_array =
+ repalloc0_array(root->placeholder_array,
PlaceHolderInfo *, root->placeholder_array_size, new_size);
else
- root->placeholder_array = (PlaceHolderInfo **)
- palloc0(new_size * sizeof(PlaceHolderInfo *));
+ root->placeholder_array =
+ palloc0_array(PlaceHolderInfo *, new_size);
root->placeholder_array_size = new_size;
}
root->placeholder_array[phinfo->phid] = phinfo;
diff --git a/src/backend/optimizer/util/relnode.c
b/src/backend/optimizer/util/relnode.c
index 1786a3daddc8..d7b4434e7f4a 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -157,31 +157,18 @@ expand_planner_arrays(PlannerInfo *root, int add_size)
new_size = root->simple_rel_array_size + add_size;
- root->simple_rel_array = (RelOptInfo **)
- repalloc(root->simple_rel_array,
- sizeof(RelOptInfo *) * new_size);
- MemSet(root->simple_rel_array + root->simple_rel_array_size,
- 0, sizeof(RelOptInfo *) * add_size);
+ root->simple_rel_array =
+ repalloc0_array(root->simple_rel_array, RelOptInfo *,
root->simple_rel_array_size, new_size);
- root->simple_rte_array = (RangeTblEntry **)
- repalloc(root->simple_rte_array,
- sizeof(RangeTblEntry *) * new_size);
- MemSet(root->simple_rte_array + root->simple_rel_array_size,
- 0, sizeof(RangeTblEntry *) * add_size);
+ root->simple_rte_array =
+ repalloc0_array(root->simple_rte_array, RangeTblEntry *,
root->simple_rel_array_size, new_size);
if (root->append_rel_array)
- {
- root->append_rel_array = (AppendRelInfo **)
- repalloc(root->append_rel_array,
- sizeof(AppendRelInfo *) * new_size);
- MemSet(root->append_rel_array + root->simple_rel_array_size,
- 0, sizeof(AppendRelInfo *) * add_size);
- }
+ root->append_rel_array =
+ repalloc0_array(root->append_rel_array, AppendRelInfo
*, root->simple_rel_array_size, new_size);
else
- {
- root->append_rel_array = (AppendRelInfo **)
- palloc0(sizeof(AppendRelInfo *) * new_size);
- }
+ root->append_rel_array =
+ palloc0_array(AppendRelInfo *, new_size);
root->simple_rel_array_size = new_size;
}
diff --git a/src/backend/parser/parse_param.c b/src/backend/parser/parse_param.c
index f668abfcb336..e80876aa25e0 100644
--- a/src/backend/parser/parse_param.c
+++ b/src/backend/parser/parse_param.c
@@ -145,14 +145,10 @@ variable_paramref_hook(ParseState *pstate, ParamRef *pref)
{
/* Need to enlarge param array */
if (*parstate->paramTypes)
- *parstate->paramTypes = (Oid *)
repalloc(*parstate->paramTypes,
-
paramno * sizeof(Oid));
+ *parstate->paramTypes =
repalloc0_array(*parstate->paramTypes, Oid,
+
*parstate->numParams, paramno);
else
- *parstate->paramTypes = (Oid *) palloc(paramno *
sizeof(Oid));
- /* Zero out the previously-unreferenced slots */
- MemSet(*parstate->paramTypes + *parstate->numParams,
- 0,
- (paramno - *parstate->numParams) * sizeof(Oid));
+ *parstate->paramTypes = palloc0_array(Oid, paramno);
*parstate->numParams = paramno;
}
diff --git a/src/backend/storage/lmgr/lwlock.c
b/src/backend/storage/lmgr/lwlock.c
index 0fc0cf6ebbd9..532cd67f4e3c 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -668,13 +668,8 @@ LWLockRegisterTranche(int tranche_id, const char
*tranche_name)
MemoryContextAllocZero(TopMemoryContext,
newalloc * sizeof(char *));
else
- {
- LWLockTrancheNames = (const char **)
- repalloc(LWLockTrancheNames, newalloc *
sizeof(char *));
- memset(LWLockTrancheNames + LWLockTrancheNamesAllocated,
- 0,
- (newalloc - LWLockTrancheNamesAllocated) *
sizeof(char *));
- }
+ LWLockTrancheNames =
+ repalloc0_array(LWLockTrancheNames, const char
*, LWLockTrancheNamesAllocated, newalloc);
LWLockTrancheNamesAllocated = newalloc;
}
diff --git a/src/backend/utils/adt/ruleutils.c
b/src/backend/utils/adt/ruleutils.c
index 70d723e80cac..c5a49d0be342 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -4855,14 +4855,9 @@ expand_colnames_array_to(deparse_columns *colinfo, int n)
if (n > colinfo->num_cols)
{
if (colinfo->colnames == NULL)
- colinfo->colnames = (char **) palloc0(n * sizeof(char
*));
+ colinfo->colnames = palloc0_array(char *, n);
else
- {
- colinfo->colnames = (char **)
repalloc(colinfo->colnames,
-
n * sizeof(char *));
- memset(colinfo->colnames + colinfo->num_cols, 0,
- (n - colinfo->num_cols) * sizeof(char *));
- }
+ colinfo->colnames = repalloc0_array(colinfo->colnames,
char *, colinfo->num_cols, n);
colinfo->num_cols = n;
}
}
diff --git a/src/backend/utils/cache/typcache.c
b/src/backend/utils/cache/typcache.c
index 808f9ebd0d2f..b69366fa29de 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -1714,14 +1714,8 @@ ensure_record_cache_typmod_slot_exists(int32 typmod)
{
int32 newlen = pg_nextpower2_32(typmod + 1);
- RecordCacheArray = (TupleDesc *) repalloc(RecordCacheArray,
-
newlen * sizeof(TupleDesc));
- memset(RecordCacheArray + RecordCacheArrayLen, 0,
- (newlen - RecordCacheArrayLen) * sizeof(TupleDesc));
- RecordIdentifierArray = (uint64 *)
repalloc(RecordIdentifierArray,
-
newlen * sizeof(uint64));
- memset(RecordIdentifierArray + RecordCacheArrayLen, 0,
- (newlen - RecordCacheArrayLen) * sizeof(uint64));
+ RecordCacheArray = repalloc0_array(RecordCacheArray, TupleDesc,
RecordCacheArrayLen, newlen);
+ RecordIdentifierArray = repalloc0_array(RecordIdentifierArray,
uint64, RecordCacheArrayLen, newlen);
RecordCacheArrayLen = newlen;
}
}
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index f526ca82c15d..9595a148d1ba 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1395,6 +1395,24 @@ repalloc_extended(void *pointer, Size size, int flags)
return ret;
}
+/*
+ * repalloc0
+ * Adjust the size of a previously allocated chunk and zero out
the added
+ * space.
+ */
+void *
+repalloc0(void *pointer, Size oldsize, Size size)
+{
+ void *ret;
+
+ /* catch wrong argument order */
+ Assert(size >= oldsize);
+
+ ret = repalloc(pointer, size);
+ memset((char *) ret + oldsize, 0, (size - oldsize));
+ return ret;
+}
+
/*
* MemoryContextAllocHuge
* Allocate (possibly-expansive) space within the specified
context.
diff --git a/src/include/utils/palloc.h b/src/include/utils/palloc.h
index 8eee0e293857..72d4e70dc649 100644
--- a/src/include/utils/palloc.h
+++ b/src/include/utils/palloc.h
@@ -80,6 +80,7 @@ extern void *palloc_extended(Size size, int flags);
extern pg_nodiscard void *repalloc(void *pointer, Size size);
extern pg_nodiscard void *repalloc_extended(void *pointer,
Size size, int flags);
+extern pg_nodiscard void *repalloc0(void *pointer, Size oldsize, Size size);
extern void pfree(void *pointer);
/*
@@ -103,6 +104,7 @@ extern void pfree(void *pointer);
* objects of type "type"
*/
#define repalloc_array(pointer, type, count) ((type *) repalloc(pointer,
sizeof(type) * (count)))
+#define repalloc0_array(pointer, type, oldcount, count) ((type *)
repalloc0(pointer, sizeof(type) * (oldcount), sizeof(type) * (count)))
/*
* The result of palloc() is always word-aligned, so we can skip testing
--
2.37.3
