On Wed, Jan 21, 2026 at 5:05 PM John Naylor <[email protected]> wrote: > heaptoast.c > memcpy(VARDATA(result) + > - (curchunk * TOAST_MAX_CHUNK_SIZE - sliceoffset) + chcpystrt, > + (int)(curchunk * TOAST_MAX_CHUNK_SIZE - sliceoffset) + chcpystrt,
Recall, the error was "runtime error: addition of unsigned offset to 0x7395fbd3d204 overflowed to 0x7395fbd3d142" It looks like "- 194" got turned into "+ (SIZE_MAX - 193)". Curiously, just removing the parentheses is enough to pass make check for me.: - (curchunk * TOAST_MAX_CHUNK_SIZE - sliceoffset) + chcpystrt, + curchunk * TOAST_MAX_CHUNK_SIZE - sliceoffset + chcpystrt, That's obviously equivalent in math, and IIUC in C precedence, so I'm not sure what to think of this. For v2 I've just done the above, but I'm curious if this raises anyone else's eyebrow. 0001 is backpatchable to v14, and doesn't change the sort template, and just guards NULL at the call site. The sort template change 0002 is a master-only patch. I don't think it would make any difference for performance, but to remove any doubt we could bump the insertion sort threshold, which is a good idea anyway. -- John Naylor Amazon Web Services
From d10cbcaabfdb62cf6565a1fa8c697827db94c79a Mon Sep 17 00:00:00 2001 From: John Naylor <[email protected]> Date: Wed, 21 Jan 2026 16:44:46 +0700 Subject: [PATCH v2 1/2] Fix various instances of undefined behavior Mostly this involves checking for NULL pointer before doing operations that add a non-zero offset. The exception is an overflow warning in heap_fetch_toast_slice(). XXX WIP Per clang 21 undefined behavior sanitizer. Coauthored-by: Alexander Lakhin <[email protected]> Reported-by: Alexander Lakhin <[email protected]> Tested-by: Discussion: https://postgr.es/m/[email protected] Backpatch-through: 14 --- contrib/pg_trgm/trgm_gist.c | 5 ++++- src/backend/access/heap/heaptoast.c | 2 +- src/backend/utils/adt/multirangetypes.c | 5 +++-- src/backend/utils/sort/sharedtuplestore.c | 3 ++- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/contrib/pg_trgm/trgm_gist.c b/contrib/pg_trgm/trgm_gist.c index 2f0d61985a5..685275a0f9b 100644 --- a/contrib/pg_trgm/trgm_gist.c +++ b/contrib/pg_trgm/trgm_gist.c @@ -701,10 +701,13 @@ gtrgm_penalty(PG_FUNCTION_ARGS) if (ISARRKEY(newval)) { char *cache = (char *) fcinfo->flinfo->fn_extra; - TRGM *cachedVal = (TRGM *) (cache + MAXALIGN(siglen)); + TRGM *cachedVal = NULL; Size newvalsize = VARSIZE(newval); BITVECP sign; + if (cache != NULL) + cachedVal = (TRGM *) (cache + MAXALIGN(siglen)); + /* * Cache the sign data across multiple calls with the same newval. */ diff --git a/src/backend/access/heap/heaptoast.c b/src/backend/access/heap/heaptoast.c index e28fe47a449..6ddf6c6cf9f 100644 --- a/src/backend/access/heap/heaptoast.c +++ b/src/backend/access/heap/heaptoast.c @@ -768,7 +768,7 @@ heap_fetch_toast_slice(Relation toastrel, Oid valueid, int32 attrsize, chcpyend = (sliceoffset + slicelength - 1) % TOAST_MAX_CHUNK_SIZE; memcpy(VARDATA(result) + - (curchunk * TOAST_MAX_CHUNK_SIZE - sliceoffset) + chcpystrt, + curchunk * TOAST_MAX_CHUNK_SIZE - sliceoffset + chcpystrt, chunkdata + chcpystrt, (chcpyend - chcpystrt) + 1); diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c index 07e2a81d46a..08b6549f77e 100644 --- a/src/backend/utils/adt/multirangetypes.c +++ b/src/backend/utils/adt/multirangetypes.c @@ -485,8 +485,9 @@ multirange_canonicalize(TypeCacheEntry *rangetyp, int32 input_range_count, int32 output_range_count = 0; /* Sort the ranges so we can find the ones that overlap/meet. */ - qsort_arg(ranges, input_range_count, sizeof(RangeType *), range_compare, - rangetyp); + if (ranges != NULL) + qsort_arg(ranges, input_range_count, sizeof(RangeType *), + range_compare, rangetyp); /* Now merge where possible: */ for (i = 0; i < input_range_count; i++) diff --git a/src/backend/utils/sort/sharedtuplestore.c b/src/backend/utils/sort/sharedtuplestore.c index 8f35a255263..04189f708fa 100644 --- a/src/backend/utils/sort/sharedtuplestore.c +++ b/src/backend/utils/sort/sharedtuplestore.c @@ -323,7 +323,8 @@ sts_puttuple(SharedTuplestoreAccessor *accessor, void *meta_data, /* Do we have space? */ size = accessor->sts->meta_data_size + tuple->t_len; - if (accessor->write_pointer + size > accessor->write_end) + if (accessor->write_pointer == NULL || + accessor->write_pointer + size > accessor->write_end) { if (accessor->write_chunk == NULL) { -- 2.52.0
From f3c4463f51ec6fb78b52818572b57f86b14db58b Mon Sep 17 00:00:00 2001 From: John Naylor <[email protected]> Date: Wed, 21 Jan 2026 15:21:31 +0700 Subject: [PATCH v2 2/2] Future-proof sort template against undefined behavior Commit XXXYYYZZZ added a NULL array pointer check before performing a qsort in order to prevent undefined behavior when passing NULL pointer and zero length. To head off future degenerate cases, check that there are at least two elements to sort before proceeding with insertion sort. This has the added advantage of allowing us to remove four equivalent checks that guarded against recursion/iteration. There might be a tiny performance penalty from unproductive recursions, but we can buy that back by increasing the insertion sort threshold. That is left for future work. Discussion: https://postgr.es/m/[email protected] --- src/include/lib/sort_template.h | 40 +++++++++++++++++---------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/include/lib/sort_template.h b/src/include/lib/sort_template.h index e02aa73cd4d..22b2092d03b 100644 --- a/src/include/lib/sort_template.h +++ b/src/include/lib/sort_template.h @@ -311,6 +311,14 @@ loop: DO_CHECK_FOR_INTERRUPTS(); if (n < 7) { + /* + * Not strictly necessary, but a caller may pass a NULL pointer input + * and zero length, and this silences warnings about applying offsets + * to NULL pointers. + */ + if (n < 2) + return; + for (pm = a + ST_POINTER_STEP; pm < a + n * ST_POINTER_STEP; pm += ST_POINTER_STEP) for (pl = pm; pl > a && DO_COMPARE(pl - ST_POINTER_STEP, pl) > 0; @@ -387,29 +395,23 @@ loop: if (d1 <= d2) { /* Recurse on left partition, then iterate on right partition */ - if (d1 > ST_POINTER_STEP) - DO_SORT(a, d1 / ST_POINTER_STEP); - if (d2 > ST_POINTER_STEP) - { - /* Iterate rather than recurse to save stack space */ - /* DO_SORT(pn - d2, d2 / ST_POINTER_STEP) */ - a = pn - d2; - n = d2 / ST_POINTER_STEP; - goto loop; - } + DO_SORT(a, d1 / ST_POINTER_STEP); + + /* Iterate rather than recurse to save stack space */ + /* DO_SORT(pn - d2, d2 / ST_POINTER_STEP) */ + a = pn - d2; + n = d2 / ST_POINTER_STEP; + goto loop; } else { /* Recurse on right partition, then iterate on left partition */ - if (d2 > ST_POINTER_STEP) - DO_SORT(pn - d2, d2 / ST_POINTER_STEP); - if (d1 > ST_POINTER_STEP) - { - /* Iterate rather than recurse to save stack space */ - /* DO_SORT(a, d1 / ST_POINTER_STEP) */ - n = d1 / ST_POINTER_STEP; - goto loop; - } + DO_SORT(pn - d2, d2 / ST_POINTER_STEP); + + /* Iterate rather than recurse to save stack space */ + /* DO_SORT(a, d1 / ST_POINTER_STEP) */ + n = d1 / ST_POINTER_STEP; + goto loop; } } #endif -- 2.52.0
