Re: avoid negating LONG_MIN in cash_out()

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 18:05:18
Message-ID: CALNJ-vQ0VfOL7qj3R+uaxdtRFZjDJJzdzeT5ocUyqoSW+5pyPA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Aug 11, 2022 at 10:55 AM Zhihong Yu <zyu(at)yugabyte(dot)com> wrote:

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

Here is sample output with patch:

# SELECT '-92233720368547758.085'::money;
ERROR: value "-92233720368547758.085" is out of range for type money
LINE 1: SELECT '-92233720368547758.085'::money;

FYI

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-08-11 18:15:30 Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints
Previous Message Zhihong Yu 2022-08-11 17:55:48 Re: avoid negating LONG_MIN in cash_out()