From f2369bc610a19563cc00054ccfe9089fac469641 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Fri, 4 Oct 2019 16:10:56 +0200 Subject: [PATCH] Use Size instead of int64 to track allocated memory Commit 5dd7fc1519 added block-level memory accounting, but used int64 variable to track the amount of allocated memory. That is incorrect, because we have Size for exactly these purposes, but it was mostly harmless until c477f3e449 which changed how we handle with repalloc() when downsizing the chunk. Previously we've ignored these cases and just kept using the original chunk, but now we need to update the accounting, and the code was doing this: context->mem_allocated += blksize - oldblksize; Both blksize and oldblksize are Size (so unsigned) which means the subtraction underflows, producing a very high positive value. On 64-bit platforms (where Size has the same size as mem_alllocated) this happens to work because the result wraps to the right value, but on (some) 32-bit platforms this fails. This fixes two things - it changes mem_allocated (and related variables) to Size, and it splits the update to two separate steps, to prevent any underflows. Discussion: https://www.postgresql.org/message-id/15151.1570163761%40sss.pgh.pa.us --- src/backend/utils/mmgr/aset.c | 6 ++++-- src/backend/utils/mmgr/generation.c | 2 +- src/include/nodes/memnodes.h | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index ea23ac2ccb..f729d9b6de 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -1154,7 +1154,9 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size) return NULL; } - context->mem_allocated += blksize - oldblksize; + /* updated separately, not to underflow when (oldblksize > blksize) */ + context->mem_allocated -= oldblksize; + context->mem_allocated += blksize; block->freeptr = block->endptr = ((char *) block) + blksize; @@ -1427,7 +1429,7 @@ AllocSetCheck(MemoryContext context) const char *name = set->header.name; AllocBlock prevblock; AllocBlock block; - int64 total_allocated = 0; + Size total_allocated = 0; for (prevblock = NULL, block = set->blocks; block != NULL; diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c index 2d24ab68bc..b60a19f5a5 100644 --- a/src/backend/utils/mmgr/generation.c +++ b/src/backend/utils/mmgr/generation.c @@ -753,7 +753,7 @@ GenerationCheck(MemoryContext context) GenerationContext *gen = (GenerationContext *) context; const char *name = context->name; dlist_iter iter; - int64 total_allocated = 0; + Size total_allocated = 0; /* walk all blocks in this context */ dlist_foreach(iter, &gen->blocks) diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h index df0ae3625c..854bd5d225 100644 --- a/src/include/nodes/memnodes.h +++ b/src/include/nodes/memnodes.h @@ -79,7 +79,7 @@ typedef struct MemoryContextData /* these two fields are placed here to minimize alignment wastage: */ bool isReset; /* T = no space alloced since last reset */ bool allowInCritSection; /* allow palloc in critical section */ - int64 mem_allocated; /* track memory allocated for this context */ + Size mem_allocated; /* track memory allocated for this context */ const MemoryContextMethods *methods; /* virtual function table */ MemoryContext parent; /* NULL if no parent (toplevel context) */ MemoryContext firstchild; /* head of linked list of children */