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