Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: watari(dot)yuya(at)gmail(dot)com
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )
Date: 2019-10-01 06:41:48
Message-ID: 20191001.154148.173553045.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello.

At Fri, 27 Sep 2019 16:43:53 +0900, Yuya Watari <watari(dot)yuya(at)gmail(dot)com> wrote in <CAJ2pMkaLTOxFjTim=GV8u=jG++sb9W6GNSgyFxPVDSQMVfRv5g(at)mail(dot)gmail(dot)com>
> Hello,
>
> I add further information. This issue also has a problem about
> *overflow checking*.
>
> The original code is as follows.
>
> src/backend/utils/adt/timestamp.c:3222
> -----
> if (result_double > PG_INT64_MAX || result_double < PG_INT64_MIN)
> ereport(ERROR,
> (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
> errmsg("interval out of range")));
> result->time = (int64) result_double;
> -----
>
> I think the code should be "result_double >= (double) PG_INT64_MAX",
> that is we have to use >= rather than >. I attached the modified
> patch.

Yeah, good catch! It is surely a bogus comparison. But I suspect
that how a number is rounded off depends on architechture. (But
not sure.) There seems be a variant of (double/float)->int32,
say, in interval_div.

I found a trick seems workable generically (*1). (2.0 *
(PG_INT64_MAX/2 + 1)) will generate the value next to the
PG_INT64_MAX based on some assumptions
(*1). IS_DOUBLE_SAFE_IN_INT64() below would be able to check if
the value can be converted into int64 safely or not.

====
/*
* Check if a double value can be casted into int64.
*
* This macro is assuming that FLT_RADIX == 2 so that the * 2.0 trick works,
* PG_INT64_MAX is so below DBL_MAX that the doubled value can be represented
* in double and DBL_MANT_DIG is equal or smaller than DBL_MAX_EXP so that
* ceil() returns expected result.
*/
#define MIN_DOUBLE_OVER_INT64_MAX (2.0 * (PG_INT64_MAX / 2 + 1))
#define MAX_DOUBLE_UNDER_INT64_MIN (2.0 * (PG_INT64_MIN / 2 - 1))

#if -PG_INT64_MAX != PG_INT64_MIN
#define IS_DOUBLE_SAFE_IN_INT64(x) \
((x) < MIN_DOUBLE_OVER_INT64_MAX && ceil(x) >= PG_INT64_MIN)
#else
#define IS_DOUBLE_SAFE_IN_INT64(x) \
((x) < MIN_DOUBLE_OVER_INT64_MAX && (x) > MAX_DOUBLE_UNDER_INT64_MIN)
#endif
====

I haven't fully confirmed if it is really right.

*1: https://stackoverflow.com/questions/526070/handling-overflow-when-casting-doubles-to-integers-in-c

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2019-10-01 07:15:29 Re: Commit fest 2019-09
Previous Message Antonin Houska 2019-10-01 06:28:03 Re: Attempt to consolidate reading of XLOG page