Buildfarm member thorntail was recently resurrected with a very
recent gcc (Debian 15.2.0-9), and since then it's been giving
these compiler warnings:
In file included from ../../../../src/include/storage/bufmgr.h:21,
from ../../../../src/include/access/bufmask.h:21,
from heapam_xlog.c:17:
In function 'PageGetItemId',
inlined from 'heap_xlog_update' at heapam_xlog.c:885:9:
../../../../src/include/storage/bufpage.h:246:16: warning: array subscript -1
is below array bounds of 'ItemIdData[]' [-Warray-bounds=]
246 | return &((PageHeader) page)->pd_linp[offsetNumber - 1];
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../src/include/storage/bufpage.h: In function 'heap_xlog_update':
../../../../src/include/storage/bufpage.h:170:25: note: while referencing
'pd_linp'
170 | ItemIdData pd_linp[FLEXIBLE_ARRAY_MEMBER]; /* line pointer
array */
| ^~~~~~~
In function 'PageGetItemId',
inlined from 'heap_xlog_delete' at heapam_xlog.c:455:9,
inlined from 'heap_redo' at heapam_xlog.c:1321:4:
../../../../src/include/storage/bufpage.h:246:16: warning: array subscript -1
is below array bounds of 'ItemIdData[]' [-Warray-bounds=]
246 | return &((PageHeader) page)->pd_linp[offsetNumber - 1];
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../src/include/storage/bufpage.h: In function 'heap_redo':
../../../../src/include/storage/bufpage.h:170:25: note: while referencing
'pd_linp'
170 | ItemIdData pd_linp[FLEXIBLE_ARRAY_MEMBER]; /* line pointer
array */
| ^~~~~~~
In file included from ../../../../src/include/access/itup.h:19,
from ../../../../src/include/access/nbtree.h:18,
from nbtsearch.c:18:
In function 'PageGetItemId',
inlined from '_bt_get_endpoint' at nbtsearch.c:2151:41:
../../../../src/include/storage/bufpage.h:246:16: warning: array subscript -1
is below array bounds of 'ItemIdData[]' [-Warray-bounds=]
246 | return &((PageHeader) page)->pd_linp[offsetNumber - 1];
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../src/include/storage/bufpage.h: In function '_bt_get_endpoint':
../../../../src/include/storage/bufpage.h:170:25: note: while referencing
'pd_linp'
170 | ItemIdData pd_linp[FLEXIBLE_ARRAY_MEMBER]; /* line pointer
array */
| ^~~~~~~
I've been able to replicate this locally using Fedora rawhide's
compiler (Red Hat 15.2.1-4) and thorntail's build settings
'CC' => 'ccache gcc -fsanitize=undefined -fsanitize-undefined-trap-on-error',
'CFLAGS' => '-O2 -funwind-tables',
So it seems like we ought to do something about this before more
platforms start complaining. The problem evidently is that the
compiler can't satisfy itself that offsetNumber > 0. These call
sites do mostly have range checks on the offset, but they're
one-sided, checking only for offset too large.
Looking at the instances in heapam_xlog.c, they are very
badly coded anyway IMO, having duplicate range checks and
using the same elog message for two distinguishable problems.
So I propose the attached patch, which cleans up all copies
of that coding pattern (in this file anyway) even though
right now only two are producing warnings.
regards, tom lane
diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c
index a09fb4b803a..0ab5ef3f63c 100644
--- a/src/backend/access/heap/heapam_xlog.c
+++ b/src/backend/access/heap/heapam_xlog.c
@@ -451,10 +451,10 @@ heap_xlog_delete(XLogReaderState *record)
{
page = BufferGetPage(buffer);
- if (PageGetMaxOffsetNumber(page) >= xlrec->offnum)
- lp = PageGetItemId(page, xlrec->offnum);
-
- if (PageGetMaxOffsetNumber(page) < xlrec->offnum || !ItemIdIsNormal(lp))
+ if (xlrec->offnum < 1 || xlrec->offnum > PageGetMaxOffsetNumber(page))
+ elog(PANIC, "offnum out of range");
+ lp = PageGetItemId(page, xlrec->offnum);
+ if (!ItemIdIsNormal(lp))
elog(PANIC, "invalid lp");
htup = (HeapTupleHeader) PageGetItem(page, lp);
@@ -881,10 +881,10 @@ heap_xlog_update(XLogReaderState *record, bool hot_update)
{
page = BufferGetPage(obuffer);
offnum = xlrec->old_offnum;
- if (PageGetMaxOffsetNumber(page) >= offnum)
- lp = PageGetItemId(page, offnum);
-
- if (PageGetMaxOffsetNumber(page) < offnum || !ItemIdIsNormal(lp))
+ if (offnum < 1 || offnum > PageGetMaxOffsetNumber(page))
+ elog(PANIC, "offnum out of range");
+ lp = PageGetItemId(page, offnum);
+ if (!ItemIdIsNormal(lp))
elog(PANIC, "invalid lp");
htup = (HeapTupleHeader) PageGetItem(page, lp);
@@ -1095,10 +1095,10 @@ heap_xlog_confirm(XLogReaderState *record)
page = BufferGetPage(buffer);
offnum = xlrec->offnum;
- if (PageGetMaxOffsetNumber(page) >= offnum)
- lp = PageGetItemId(page, offnum);
-
- if (PageGetMaxOffsetNumber(page) < offnum || !ItemIdIsNormal(lp))
+ if (offnum < 1 || offnum > PageGetMaxOffsetNumber(page))
+ elog(PANIC, "offnum out of range");
+ lp = PageGetItemId(page, offnum);
+ if (!ItemIdIsNormal(lp))
elog(PANIC, "invalid lp");
htup = (HeapTupleHeader) PageGetItem(page, lp);
@@ -1155,10 +1155,10 @@ heap_xlog_lock(XLogReaderState *record)
page = BufferGetPage(buffer);
offnum = xlrec->offnum;
- if (PageGetMaxOffsetNumber(page) >= offnum)
- lp = PageGetItemId(page, offnum);
-
- if (PageGetMaxOffsetNumber(page) < offnum || !ItemIdIsNormal(lp))
+ if (offnum < 1 || offnum > PageGetMaxOffsetNumber(page))
+ elog(PANIC, "offnum out of range");
+ lp = PageGetItemId(page, offnum);
+ if (!ItemIdIsNormal(lp))
elog(PANIC, "invalid lp");
htup = (HeapTupleHeader) PageGetItem(page, lp);
@@ -1231,10 +1231,10 @@ heap_xlog_lock_updated(XLogReaderState *record)
page = BufferGetPage(buffer);
offnum = xlrec->offnum;
- if (PageGetMaxOffsetNumber(page) >= offnum)
- lp = PageGetItemId(page, offnum);
-
- if (PageGetMaxOffsetNumber(page) < offnum || !ItemIdIsNormal(lp))
+ if (offnum < 1 || offnum > PageGetMaxOffsetNumber(page))
+ elog(PANIC, "offnum out of range");
+ lp = PageGetItemId(page, offnum);
+ if (!ItemIdIsNormal(lp))
elog(PANIC, "invalid lp");
htup = (HeapTupleHeader) PageGetItem(page, lp);
@@ -1275,10 +1275,10 @@ heap_xlog_inplace(XLogReaderState *record)
page = BufferGetPage(buffer);
offnum = xlrec->offnum;
- if (PageGetMaxOffsetNumber(page) >= offnum)
- lp = PageGetItemId(page, offnum);
-
- if (PageGetMaxOffsetNumber(page) < offnum || !ItemIdIsNormal(lp))
+ if (offnum < 1 || offnum > PageGetMaxOffsetNumber(page))
+ elog(PANIC, "offnum out of range");
+ lp = PageGetItemId(page, offnum);
+ if (!ItemIdIsNormal(lp))
elog(PANIC, "invalid lp");
htup = (HeapTupleHeader) PageGetItem(page, lp);
diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c
index aec71093661..7a416d60cea 100644
--- a/src/backend/access/nbtree/nbtsearch.c
+++ b/src/backend/access/nbtree/nbtsearch.c
@@ -2148,6 +2148,9 @@ _bt_get_endpoint(Relation rel, uint32 level, bool rightmost)
else
offnum = P_FIRSTDATAKEY(opaque);
+ if (offnum < 1 || offnum > PageGetMaxOffsetNumber(page))
+ elog(PANIC, "offnum out of range");
+
itup = (IndexTuple) PageGetItem(page, PageGetItemId(page, offnum));
blkno = BTreeTupleGetDownLink(itup);
