Re: Undefined behavior detected by new clang's ubsan

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: John Naylor <johncnaylorls(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alexander Lakhin <exclusion(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Undefined behavior detected by new clang's ubsan
Date: 2026-02-04 11:37:24
Message-ID: aYMvdFTjMcXVD71b@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Feb 04, 2026 at 06:15:11PM +0700, John Naylor wrote:
> I logged the offending statement and found
>
> curchunk: 50
> TOAST_MAX_CHUNK_SIZE: 1996
> sliceoffset: 99994
> chcpystrt 194
>
> The expression for the offset to the pointer:
>
> (curchunk * TOAST_MAX_CHUNK_SIZE - sliceoffset) + chcpystrt
>
> is zero by way of
>
> (-194) + 194
>
> ...so the "+ (-194)" must get cast to "+ (size_t) (-194)" causing the
> overflow. So the parentheses are indeed at fault by forcing
> precedence. The entire offset expression is never negative for our
> regression tests (nor should it ever be), so it's kind of pointless to
> cast any subset of this to a signed type. I went ahead and pushed 0001
> to all versions. Thanks Alexander, for the report and the patch!

Ah. I've seen this one, actually, while playing with the TOAST patch
set for 64-bit values. The max chunk size varies as this code would
need to switch between two possible sizes, and I've been super puzzled
by the fact that I had to switch the code to use an unsigned integer
for the declaration of my max_chunk_size, and a signed integer quickly
failed.

Thanks for the fix!
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Zsolt Parragi 2026-02-04 11:42:12 Re: Custom oauth validator options
Previous Message Michael Paquier 2026-02-04 11:30:19 Re: Add expressions to pg_restore_extended_stats()