On Sun, Jan 26, 2014 at 02:13:03PM +0100, Florian Pflug wrote:
> On Jan26, 2014, at 03:50 , Bruce Momjian <br...@momjian.us> wrote:
> > Patch attached.
>
> > + if ((float)tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon > INT_MAX)
> > + return -1;
>
> Is this bullet-proof? If float and int are both 32-bit, the float's mantissa
> will be less than 32-bit (24 or so, I think), and thus won't be able to
> represent INT_MAX accurately. This means there's a value of
> tm_year * MONTHS_PER_YEAR which is less than INT_MAX, yet for which
> tm_year * MONTHS_PER_YEAR + tm_mon will return tm_year * MONTHS_PER_YEAR
> unmodified if tm_mon is small enough (e.g, 1). But if tm_year *
> MONTHS_PER_YEAR
> was close enough to INT_MAX, the actual value of
> tm_year * MONTHS_PER_YEAR + tm_mon might still be larger than INT_MAX,
> the floating-point computation just won't detect it. In that case, the
> check would succeed, yet the actual integer computation would still overflow.
>
> It should be safe with "double" instead of "float".
You are correct. I have adjusted the code to use "double".
> > + if (SAMESIGN(span1->month, span2->month) &&
> > + !SAMESIGN(result->month, span1->month))
>
> This assumes wrapping semantics for signed integral types, which isn't
> guaranteed by the C standard - it says overflows of signed integral types
> produce undefined results. We currently depend on wrapping semantics for
> these types in more places, and therefore need GCC's "-fwrapv" anway, but
> I still wonder if adding more of these kinds of checks is a good idea.
Well, I copied this from int.c, so what I did was to mention the other
function I got this code from. If we ever change int.c we would need to
change this too.
The updated attached patch has overflow detection for interval
subtraction, multiply, and negate. There are also some comparison
cleanups.
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
new file mode 100644
index 6bf4cf6..b7d7d80
*** a/doc/src/sgml/datatype.sgml
--- b/doc/src/sgml/datatype.sgml
*************** SELECT E'\\xDEADBEEF';
*** 1587,1593 ****
</row>
<row>
<entry><type>interval [ <replaceable>fields</replaceable> ] [ (<replaceable>p</replaceable>) ]</type></entry>
! <entry>12 bytes</entry>
<entry>time interval</entry>
<entry>-178000000 years</entry>
<entry>178000000 years</entry>
--- 1587,1593 ----
</row>
<row>
<entry><type>interval [ <replaceable>fields</replaceable> ] [ (<replaceable>p</replaceable>) ]</type></entry>
! <entry>16 bytes</entry>
<entry>time interval</entry>
<entry>-178000000 years</entry>
<entry>178000000 years</entry>
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
new file mode 100644
index a61b40e..70284e9
*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
*************** DecodeInterval(char **field, int *ftype,
*** 2976,2981 ****
--- 2976,2983 ----
type = DTK_MONTH;
if (*field[i] == '-')
val2 = -val2;
+ if (((double)val * MONTHS_PER_YEAR + val2) > INT_MAX)
+ return DTERR_FIELD_OVERFLOW;
val = val * MONTHS_PER_YEAR + val2;
fval = 0;
}
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
new file mode 100644
index 4581862..3b8e8e8
*** a/src/backend/utils/adt/timestamp.c
--- b/src/backend/utils/adt/timestamp.c
***************
*** 41,46 ****
--- 41,47 ----
#error -ffast-math is known to break this code
#endif
+ #define SAMESIGN(a,b) (((a) < 0) == ((b) < 0))
/* Set at postmaster start */
TimestampTz PgStartTime;
*************** interval2tm(Interval span, struct pg_tm
*** 1694,1700 ****
#ifdef HAVE_INT64_TIMESTAMP
tfrac = time / USECS_PER_HOUR;
time -= tfrac * USECS_PER_HOUR;
! tm->tm_hour = tfrac; /* could overflow ... */
tfrac = time / USECS_PER_MINUTE;
time -= tfrac * USECS_PER_MINUTE;
tm->tm_min = tfrac;
--- 1695,1705 ----
#ifdef HAVE_INT64_TIMESTAMP
tfrac = time / USECS_PER_HOUR;
time -= tfrac * USECS_PER_HOUR;
! tm->tm_hour = tfrac;
! if (!SAMESIGN(tm->tm_hour, tfrac))
! ereport(ERROR,
! (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
! errmsg("interval out of range")));
tfrac = time / USECS_PER_MINUTE;
time -= tfrac * USECS_PER_MINUTE;
tm->tm_min = tfrac;
*************** recalc:
*** 1725,1731 ****
int
tm2interval(struct pg_tm * tm, fsec_t fsec, Interval *span)
{
! span->month = tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon;
span->day = tm->tm_mday;
#ifdef HAVE_INT64_TIMESTAMP
span->time = (((((tm->tm_hour * INT64CONST(60)) +
--- 1730,1740 ----
int
tm2interval(struct pg_tm * tm, fsec_t fsec, Interval *span)
{
! double total_months = (double)tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon;
!
! if (total_months > INT_MAX || total_months < INT_MIN)
! return -1;
! span->month = total_months;
span->day = tm->tm_mday;
#ifdef HAVE_INT64_TIMESTAMP
span->time = (((((tm->tm_hour * INT64CONST(60)) +
*************** interval_um(PG_FUNCTION_ARGS)
*** 2826,2833 ****
--- 2835,2855 ----
result = (Interval *) palloc(sizeof(Interval));
result->time = -interval->time;
+ /* overflow check copied from int4um */
+ if (interval->time != 0 && SAMESIGN(result->time, interval->time))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
result->day = -interval->day;
+ if (interval->day != 0 && SAMESIGN(result->day, interval->day))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
result->month = -interval->month;
+ if (interval->month != 0 && SAMESIGN(result->month, interval->month))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
PG_RETURN_INTERVAL_P(result);
}
*************** interval_pl(PG_FUNCTION_ARGS)
*** 2872,2879 ****
--- 2894,2919 ----
result = (Interval *) palloc(sizeof(Interval));
result->month = span1->month + span2->month;
+ /* overflow check copied from int4pl */
+ if (SAMESIGN(span1->month, span2->month) &&
+ !SAMESIGN(result->month, span1->month))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
result->day = span1->day + span2->day;
+ if (SAMESIGN(span1->day, span2->day) &&
+ !SAMESIGN(result->day, span1->day))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
result->time = span1->time + span2->time;
+ if (SAMESIGN(span1->time, span2->time) &&
+ !SAMESIGN(result->time, span1->time))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
PG_RETURN_INTERVAL_P(result);
}
*************** interval_mi(PG_FUNCTION_ARGS)
*** 2888,2895 ****
--- 2928,2954 ----
result = (Interval *) palloc(sizeof(Interval));
result->month = span1->month - span2->month;
+ /* overflow check copied from int4mi */
+ if (!SAMESIGN(span1->month, span2->month) &&
+ !SAMESIGN(result->month, span1->month))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
result->day = span1->day - span2->day;
+ if (!SAMESIGN(span1->day, span2->day) &&
+ !SAMESIGN(result->day, span1->day))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
result->time = span1->time - span2->time;
+ if (!SAMESIGN(span1->time, span2->time) &&
+ !SAMESIGN(result->time, span1->time))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
PG_RETURN_INTERVAL_P(result);
}
*************** interval_mul(PG_FUNCTION_ARGS)
*** 2906,2920 ****
Interval *span = PG_GETARG_INTERVAL_P(0);
float8 factor = PG_GETARG_FLOAT8(1);
double month_remainder_days,
! sec_remainder;
int32 orig_month = span->month,
orig_day = span->day;
Interval *result;
result = (Interval *) palloc(sizeof(Interval));
! result->month = (int32) (span->month * factor);
! result->day = (int32) (span->day * factor);
/*
* The above correctly handles the whole-number part of the month and day
--- 2965,2991 ----
Interval *span = PG_GETARG_INTERVAL_P(0);
float8 factor = PG_GETARG_FLOAT8(1);
double month_remainder_days,
! sec_remainder,
! result_double;
int32 orig_month = span->month,
orig_day = span->day;
Interval *result;
result = (Interval *) palloc(sizeof(Interval));
! result_double = span->month * factor;
! if (result_double > INT_MAX || result_double < INT_MIN)
! ereport(ERROR,
! (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
! errmsg("interval out of range")));
! result->month = (int32) result_double;
!
! result_double = span->day * factor;
! if (result_double > INT_MAX || result_double < INT_MIN)
! ereport(ERROR,
! (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
! errmsg("interval out of range")));
! result->day = (int32) result_double;
/*
* The above correctly handles the whole-number part of the month and day
*************** interval_mul(PG_FUNCTION_ARGS)
*** 2954,2960 ****
/* cascade units down */
result->day += (int32) month_remainder_days;
#ifdef HAVE_INT64_TIMESTAMP
! result->time = rint(span->time * factor + sec_remainder * USECS_PER_SEC);
#else
result->time = span->time * factor + sec_remainder;
#endif
--- 3025,3036 ----
/* cascade units down */
result->day += (int32) month_remainder_days;
#ifdef HAVE_INT64_TIMESTAMP
! result_double = rint(span->time * factor + sec_remainder * USECS_PER_SEC);
! if (result_double > INT64_MAX || result_double < INT64_MIN)
! ereport(ERROR,
! (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
! errmsg("interval out of range")));
! result->time = (int64) result_double;
#else
result->time = span->time * factor + sec_remainder;
#endif
diff --git a/src/interfaces/ecpg/pgtypeslib/interval.c b/src/interfaces/ecpg/pgtypeslib/interval.c
new file mode 100644
index efa775d..d603fd4
*** a/src/interfaces/ecpg/pgtypeslib/interval.c
--- b/src/interfaces/ecpg/pgtypeslib/interval.c
*************** recalc:
*** 1036,1041 ****
--- 1036,1043 ----
static int
tm2interval(struct tm * tm, fsec_t fsec, interval * span)
{
+ if ((double)tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon > INT_MAX)
+ return -1;
span->month = tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon;
#ifdef HAVE_INT64_TIMESTAMP
span->time = (((((((tm->tm_mday * INT64CONST(24)) +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
