Re: Fix overflow in pg_size_pretty

On Sat, Jul 27, 2024 at 11:42 PM David Rowley <dgrowleyml@gmail.com> wrote:
>
> I didn't test to see where that's coming from, but I did test the two
> attached .c files.  int.c uses the 0 - (unsigned int) var method and
> int2.c uses (unsigned int) (-var).  Using clang and -ftrapv, I get:
>
> $ clang int.c -o int -O2 -ftrapv
> $ ./int
> 2147483648
> $ clang int2.c -o int2 -O2 -ftrapv
> $ ./int2
> Illegal instruction
>
> Similar with gcc:
> $ gcc int.c -o int -O2 -ftrapv
> $ ./int
> 2147483648
> $ gcc int2.c -o int2 -O2 -ftrapv
> $ ./int2
> Aborted
>
> I suspect your trap must be coming from somewhere else. It looks to me
> like the "uint64 usize = size < 0 ? 0 - (uint64) size : (uint64)
> size;" will be fine.

My mistake, you're absolutely right. The trap is coming from
`pg_strtoint64_safe()`.

    return -((int64) tmp);

Which I had already addressed in the other thread and completely forgot
about.

I did some more research and it looks like unsigned integer arithmetic
is guaranteed to wrap around, unlike signed integer arithmetic [0].

Attached is an updated patch with your approach. I removed the 0 from
the negative case because I think it was unnecessary, but happy to add
it back in if I missed something.

Thanks for the review!

Thanks,
Joseph Koshakow

[0]
https://www.gnu.org/software/autoconf/manual/autoconf-2.63/html_node/Integer-Overflow-Basics.html