Re: Undefined behavior detected by new clang's ubsan
John Naylor <johncnaylorls@gmail.com>
From: John Naylor <johncnaylorls@gmail.com>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2026-01-20T07:48:03Z
Lists: pgsql-hackers
On Tue, Jan 20, 2026 at 2:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
> With UBSAN_OPTIONS=print_stacktrace=1, I can see:
> #0 0x607efd762a61 in qsort_arg .../src/port/../../src/include/lib/sort_template.h:314:15
> #1 0x607efd3fa268 in multirange_canonicalize .../src/backend/utils/adt/multirangetypes.c:488:2
> #2 0x607efd3fa268 in make_multirange .../src/backend/utils/adt/multirangetypes.c:655:16
Indeed, there are calls like "make_multirange(mltrngtypoid, rangetyp,
0, NULL);", where 0 is the count and NULL is the ranges. Then
multirange_canonicalize() has
qsort_arg(ranges, input_range_count, sizeof(RangeType *),
range_compare, rangetyp);
I haven't dug further, but I wonder if multirange_canonicalize() does
anything useful at all with "0, NULL" input from make_multirange().
Anyway, the complaint is about this place:
if (n < 7)
{
for (pm = a + ST_POINTER_STEP; pm < a + n * ST_POINTER_STEP;
pm += ST_POINTER_STEP)
...
I don't think it's great to pass a NULL pointer to a sort, but the
length could conceivably be zero for future degenerate cases, so we
could silence the warning by adding "if (n < 2) return;" before the
for-loop. The advantage of doing that anyway is it allows us to remove
all four of the "if (d_ > ST_POINTER_STEP)" branches in the recursion
part. That's better for readability.
--
John Naylor
Amazon Web Services
Commits
-
Future-proof sort template against undefined behavior
- 7467041cde9e 19 (unreleased) landed
-
Fix various instances of undefined behavior
- 176dffdf7d2a 19 (unreleased) landed
- fc56793c9ae5 14.21 landed
- b5e1cd2fdca1 18.2 landed
- 73ac2b37401d 16.12 landed
- 6b81a1c7c905 15.16 landed
- 1662cd0cb7ae 17.8 landed