git.postgresql.org Git - postgresql.git/commitdiff

git projects / postgresql.git / commitdiff

Add offnum range checks to suppress compile warnings with UBSAN.

2025年12月15日 17:40:09 +0000 (12:40 -0500)

Late-model gcc with -fsanitize=undefined enabled issues warnings
about uses of PageGetItemId() when it can't prove that the
offsetNumber is > 0. The call sites where this happens are
checking that the offnum is <= PageGetMaxOffsetNumber(page), so
it seems reasonable to add an explicit check that offnum >= 1 too.

While at it, rearrange the code to be less contorted and avoid
duplicate checks on PageGetMaxOffsetNumber. Maybe the compiler
would optimize away the duplicate logic or maybe not, but the
existing coding has little to recommend it anyway.

There are multiple instances of this identical coding pattern in
heapam.c and heapam_xlog.c. Current gcc only complains about two
of them, but I fixed them all in the name of consistency.

Potentially this could be back-patched in the name of silencing
warnings; but I think enabling UBSAN is mainly something people
would do on HEAD, so for now it seems not worth the trouble.

Discussion: https://postgr.es/m/1699806.1765746897@sss.pgh.pa.us

src/backend/access/heap/heapam.c patch | blob | blame | history

src/backend/access/heap/heapam_xlog.c patch | blob | blame | history

src/backend/access/nbtree/nbtsearch.c patch | blob | blame | history

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c

index 225f9829f229d9fd258bb0e9374536e65a1b7e36..9636bb53ddd8d45391535cbad4750ec523ffbae2 100644 (file)

--- a/src/backend/access/heap/heapam.c

+++ b/src/backend/access/heap/heapam.c

@@ -6116,7 +6116,7 @@ heap_finish_speculative(Relation relation, const ItemPointerData *tid)

Buffer buffer;

Page page;

OffsetNumber offnum;

- ItemId lp = NULL;

+ ItemId lp;

HeapTupleHeader htup;

buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(tid));

@@ -6124,10 +6124,10 @@ heap_finish_speculative(Relation relation, const ItemPointerData *tid)

page = BufferGetPage(buffer);

offnum = ItemPointerGetOffsetNumber(tid);

- if (PageGetMaxOffsetNumber(page) >= offnum)

- lp = PageGetItemId(page, offnum);

- if (PageGetMaxOffsetNumber(page) < offnum || !ItemIdIsNormal(lp))

+ if (offnum < 1 || offnum > PageGetMaxOffsetNumber(page))

+ elog(ERROR, "offnum out of range");

+ lp = PageGetItemId(page, offnum);

+ if (!ItemIdIsNormal(lp))

elog(ERROR, "invalid lp");

htup = (HeapTupleHeader) PageGetItem(page, lp);

diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c

index a09fb4b803ad90c459c233c9ece5e242fa8ddc60..1823feff298f8216531d4f638e63d4b0002934b2 100644 (file)

--- a/src/backend/access/heap/heapam_xlog.c

+++ b/src/backend/access/heap/heapam_xlog.c

@@ -422,7 +422,7 @@ heap_xlog_delete(XLogReaderState *record)

xl_heap_delete *xlrec = (xl_heap_delete *) XLogRecGetData(record);

Buffer buffer;

Page page;

- ItemId lp = NULL;

+ ItemId lp;

HeapTupleHeader htup;

BlockNumber blkno;

RelFileLocator target_locator;

@@ -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);

@@ -817,7 +817,7 @@ heap_xlog_update(XLogReaderState *record, bool hot_update)

nbuffer;

Page page;

OffsetNumber offnum;

- ItemId lp = NULL;

+ ItemId lp;

HeapTupleData oldtup;

HeapTupleHeader htup;

uint16 prefixlen = 0,

@@ -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);

@@ -1087,7 +1087,7 @@ heap_xlog_confirm(XLogReaderState *record)

Buffer buffer;

Page page;

OffsetNumber offnum;

- ItemId lp = NULL;

+ ItemId lp;

HeapTupleHeader htup;

if (XLogReadBufferForRedo(record, 0, &buffer) == BLK_NEEDS_REDO)

@@ -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);

@@ -1126,7 +1126,7 @@ heap_xlog_lock(XLogReaderState *record)

Buffer buffer;

Page page;

OffsetNumber offnum;

- ItemId lp = NULL;

+ ItemId lp;

HeapTupleHeader htup;

@@ -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);

@@ -1200,7 +1200,7 @@ heap_xlog_lock_updated(XLogReaderState *record)

Buffer buffer;

Page page;

OffsetNumber offnum;

- ItemId lp = NULL;

+ ItemId lp;

HeapTupleHeader htup;

xlrec = (xl_heap_lock_updated *) XLogRecGetData(record);

@@ -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);

@@ -1263,7 +1263,7 @@ heap_xlog_inplace(XLogReaderState *record)

Buffer buffer;

Page page;

OffsetNumber offnum;

- ItemId lp = NULL;

+ ItemId lp;

HeapTupleHeader htup;

uint32 oldlen;

Size newlen;

@@ -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 aec71093661fbc5c9a83433765ef997e42674acd..7a416d60cea39d6313bb3e9cb34a8023945cb55a 100644 (file)

--- 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);

This is the main PostgreSQL git repository.

RSS Atom