From: | Zhihong Yu <zyu(at)yugabyte(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: avoid negating LONG_MIN in cash_out() |
Date: | 2022-08-11 17:55:48 |
Message-ID: | CALNJ-vQ+f0NWKia4OyiHymEUxc7Gv-9uF1O=P5BMndRj4=O=LA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Aug 11, 2022 at 10:40 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Zhihong Yu <zyu(at)yugabyte(dot)com> writes:
> > In cash_out(), we have the following code:
> > if (value < 0)
> > {
> > /* make the amount positive for digit-reconstruction loop */
> > value = -value;
>
> > The negation cannot be represented in type long when the value is
> LONG_MIN.
>
> Possibly not good, but it seems like the subsequent loop works anyway:
>
> regression=# select '-92233720368547758.08'::money;
> money
> -----------------------------
> -$92,233,720,368,547,758.08
> (1 row)
>
> Note that this exact test case appears in money.sql, so we know that
> it works everywhere, not only my machine.
>
> > It seems we can error out when LONG_MIN is detected instead of continuing
> > with computation.
>
> How could you think that that's an acceptable solution? Once the
> value is stored, we'd better be able to print it.
>
> regards, tom lane
>
Thanks for taking a look.
I raise this thread due to the following assertion :
src/backend/utils/adt/cash.c:356:11: runtime error: negation of
-9223372036854775808 cannot be represented in type 'Cash' (aka 'long');
cast to an unsigned type to negate this value to itself
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../../../../../../../src/postgres/src/backend/utils/adt/cash.c:356:11
Though '-92233720368547758.085'::money displays correct error message in
other builds, this statement wouldn't pass the build where
UndefinedBehaviorSanitizer is active.
I think we should fix this otherwise when there is new assertion triggered
due to future changes around Cash (or other types covered by money.sql), we
wouldn't see it.
I am open to other ways of bypassing the above assertion.
Cheers
From | Date | Subject | |
---|---|---|---|
Next Message | Zhihong Yu | 2022-08-11 18:05:18 | Re: avoid negating LONG_MIN in cash_out() |
Previous Message | Tom Lane | 2022-08-11 17:40:13 | Re: avoid negating LONG_MIN in cash_out() |