Fix valgrind's "unaddressable bytes" whining about BRIN code.

brin_form_tuple calculated an exact tuple size, then palloc'd and
filled just that much.  Later, brin_doinsert or brin_doupdate would
MAXALIGN the tuple size and tell PageAddItem that that was the size
of the tuple to insert.  If the original tuple size wasn't a multiple
of MAXALIGN, the net result would be that PageAddItem would memcpy
a few more bytes than the palloc request had been for.

AFAICS, this is totally harmless in the real world: the error is a
read overrun not a write overrun, and palloc would certainly have
rounded the request up to a MAXALIGN multiple internally, so there's
no chance of the memcpy fetching off the end of memory.  Valgrind,
however, is picky to the byte level not the MAXALIGN level.

Fix it by pushing the MAXALIGN step back to brin_form_tuple.  (The other
possible source of tuples in this code, brin_form_placeholder_tuple,
was already producing a MAXALIGN'd result.)

In passing, be a bit more paranoid about internal allocations in
brin_form_tuple.

diff --git a/src/backend/access/brin/brin_pageops.c b/src/backend/access/brin/brin_pageops.c
index ce4cbadf21ab2ac3f2cd201c8a256d60749d2474..0b257d913b60d201ae0901bc0245c712534693a9 100644 (file)
--- a/src/backend/access/brin/brin_pageops.c
+++ b/src/backend/access/brin/brin_pageops.c
@@ -55,7 +55,7 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
        Buffer          newbuf;
        bool            extended = false;
 
-       newsz = MAXALIGN(newsz);
+       Assert(newsz == MAXALIGN(newsz));
 
        /* make sure the revmap is long enough to contain the entry we need */
        brinRevmapExtend(revmap, heapBlk);
@@ -273,7 +273,7 @@ brin_doinsert(Relation idxrel, BlockNumber pagesPerRange,
        ItemPointerData tid;
        bool            extended = false;
 
-       itemsz = MAXALIGN(itemsz);
+       Assert(itemsz == MAXALIGN(itemsz));
 
        /* Make sure the revmap is long enough to contain the entry we need */
        brinRevmapExtend(revmap, heapBlk);

diff --git a/src/backend/access/brin/brin_tuple.c b/src/backend/access/brin/brin_tuple.c
index 72356c066c72faacda2bce4b97b7c2027a72cab8..5a7462b59cfa4de1cd7ae8d9a4e3f492c2ff3be3 100644 (file)
--- a/src/backend/access/brin/brin_tuple.c
+++ b/src/backend/access/brin/brin_tuple.c
@@ -103,9 +103,10 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, BrinMemTuple *tuple,
 
        Assert(brdesc->bd_totalstored > 0);
 
-       values = palloc(sizeof(Datum) * brdesc->bd_totalstored);
-       nulls = palloc0(sizeof(bool) * brdesc->bd_totalstored);
-       phony_nullbitmap = palloc(sizeof(bits8) * BITMAPLEN(brdesc->bd_totalstored));
+       values = (Datum *) palloc(sizeof(Datum) * brdesc->bd_totalstored);
+       nulls = (bool *) palloc0(sizeof(bool) * brdesc->bd_totalstored);
+       phony_nullbitmap = (bits8 *)
+               palloc(sizeof(bits8) * BITMAPLEN(brdesc->bd_totalstored));
 
        /*
         * Set up the values/nulls arrays for heap_fill_tuple
@@ -144,6 +145,9 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, BrinMemTuple *tuple,
                        values[idxattno++] = tuple->bt_columns[keyno].bv_values[datumno];
        }
 
+       /* Assert we did not overrun temp arrays */
+       Assert(idxattno <= brdesc->bd_totalstored);
+
        /* compute total space needed */
        len = SizeOfBrinTuple;
        if (anynulls)
@@ -160,12 +164,15 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, BrinMemTuple *tuple,
 
        data_len = heap_compute_data_size(brtuple_disk_tupdesc(brdesc),
                                                                          values, nulls);
-
        len += data_len;
 
+       len = MAXALIGN(len);
+
        rettuple = palloc0(len);
        rettuple->bt_blkno = blkno;
        rettuple->bt_info = hoff;
+
+       /* Assert that hoff fits in the space available */
        Assert((rettuple->bt_info & BRIN_OFFSET_MASK) == hoff);
 
        /*