Re: Reducing the chunk header sizes on all memory context types

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, David Rowley <dgrowleyml@gmail.com>, Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>, Yura Sokolov <y.sokolov@postgrespro.ru>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2022-08-29T15:20:37Z
Lists: pgsql-hackers
Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
> I can reproduce it on my system (rpi4 running 32-bit raspbian).

Yeah, more or less same as what I'm testing on.

Seeing that the complaint is about pfree'ing a non-maxaligned
ReorderBufferChange pointer, I tried adding

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 89cf9f9389..dfa9b6c9ee 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -472,6 +472,8 @@ ReorderBufferGetChange(ReorderBuffer *rb)
        change = (ReorderBufferChange *)
                MemoryContextAlloc(rb->change_context, sizeof(ReorderBufferChange));
 
+       Assert(change == (void *) MAXALIGN(change));
+
        memset(change, 0, sizeof(ReorderBufferChange));
        return change;
 }

and this failed!

(gdb) f 3
#3  0x003cb888 in ReorderBufferGetChange (rb=0x24ed820) at reorderbuffer.c:475
475             Assert(change == (void *) MAXALIGN(change));
(gdb) p change
$1 = (ReorderBufferChange *) 0x24aaa14

So the bug is in fact in David's changes, and it consists in palloc
sometimes handing back non-maxaligned pointers.  I find it mildly
astonishing that we managed to get through core regression tests
without such a fault surfacing, but there you have it.

This machine has MAXALIGN 8 but 4-byte pointers, so there's something
wrong with that situation.

			regards, tom lane



Commits

  1. Harden memory context allocators against bogus chunk pointers.

  2. Improve our ability to detect bogus pointers passed to pfree et al.

  3. Remove MemoryContextContains().

  4. Remove uses of MemoryContextContains in nodeAgg.c and nodeWindowAgg.c.

  5. Temporarily make MemoryContextContains return false

  6. Make MemoryContextContains work correctly again

  7. Make more effort to put a sentinel at the end of allocated memory

  8. Fix some possibly latent bugs in slab.c

  9. Various cleanups of the new memory context header code

  10. Revert "Add missing padding from MemoryChunk struct"

  11. Use MAXALIGN() in calculations using sizeof(SlabBlock)

  12. Add missing padding from MemoryChunk struct

  13. Improve performance of and reduce overheads of memory management