Re: Undefined behavior detected by new clang's ubsan

On Mon, Feb 2, 2026 at 6:30 PM John Naylor <johncnaylorls@gmail.com> wrote:
>
> On Wed, Jan 21, 2026 at 5:05 PM John Naylor <johncnaylorls@gmail.com> 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.

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!

I'll push 0002 in the near future.

--
John Naylor
Amazon Web Services