Thread: Fix for Win32 division involving INT_MIN
With no Win32 exception detection code in sight, I propose the following patch to prevent server crashes for unusual INT_MIN integer division. One interesting thing is that int4div already has code that a check for a similar division on all platforms, but _after_ the division, rather than before. --------------------------------------------------------------------------- Magnus Hagander wrote: > Confirmed here. > > What we get is Integer Overflow, on the instruction "idiv esi" in postgres!int4div+0x1f. (Per windows debugger.) Same doesnot happen on Linux. > > Tom - hints? ;-) Any idea why this happens on win32 but not linux? > > //Magnus > > > -----Original Message----- > > From: pgsql-bugs-owner@postgresql.org > > [mailto:pgsql-bugs-owner@postgresql.org] On Behalf Of Wang Haiyong > > Sent: Wednesday, April 05, 2006 4:34 AM > > To: pgsql-bugs@postgresql.org > > Subject: [BUGS] Bug in window xp > > > > Version(8.1.3) > > Bug in window xp: > > > > C:\Documents and Settings\openbase>pg_ctl start > > LOG: database system was shut down at 2006-4-04 15:54:43 ?????? > > LOG: checkpoint record is at 0/38C2E0 > > LOG: redo record is at 0/38C2E0; undo record is at 0/0; shutdown TRUE > > LOG: next transaction ID: 569; next OID: 24576 > > LOG: next MultiXactId: 1; next MultiXactOffset: 0 > > LOG: database system is ready > > LOG: transaction ID wrap limit is 2147484146, limited by > > database "postgres" > > > > C:\Documents and Settings\openbase> > > C:\Documents and Settings\openbase> > > C:\Documents and Settings\openbase> > > C:\Documents and Settings\openbase>psql > > Welcome to psql 8.1.3, the PostgreSQL interactive terminal. > > > > Type: \copyright for distribution terms > > \h for help with SQL commands > > \? for help with psql commands > > \g or terminate with semicolon to execute query > > \q to quit > > > > openbase=# SELECT (-2147483648) / (-1); > > LOG: server process (PID 3760) was terminated by signal 21 > > LOG: terminating any other active server processes > > LOG: all server processes terminated; reinitializing > > ??????????? > > ??????????????????? > > ??????????????? > > ??????????. ????: LOG: database system was interrupted at > > 2006-0-05 08:39:56 ?????? > > LOG: checkpoint record is at 0/38C2E0 > > LOG: redo record is at 0/38C2E0; undo record is at 0/0; shutdown TRUE > > LOG: next transaction ID: 569; next OID: 24576 > > LOG: next MultiXactId: 1; next MultiXactOffset: 0 > > LOG: database system was not properly shut down; automatic > > recovery in progres > > > > FATAL: the database system is starting up > > ??. > > !> LOG: record with zero length at 0/38C328 > > LOG: redo is not required > > LOG: database system is ready > > LOG: transaction ID wrap limit is 2147484146, limited by > > database "postgres" > > > > > > > > ??? > > ??????????? > > > > ?????????????????????? A1? > > ???110179 > > ???024?83661905 > > ?????www.neusoft.com > > > > ________________________________ > > > > Confidentiality Notice: The information contained in this > > e-mail and any accompanying attachment(s) is intended only > > for the use of the intended recipient and may be confidential > > and/or privileged of Neusoft Group Ltd., its subsidiaries > > and/or its affiliates. If any reader of this communication is > > not the intended recipient, unauthorized use, forwarding, > > printing, storing, disclosure or copying is strictly > > prohibited, and may be unlawful. If you have received this > > communication in error, please immediately notify the sender > > by return e-mail, and delete the original message and all > > copies from your system. Thank you. > > ________________________________ > > > > > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Have you searched our list archives? > > http://archives.postgresql.org > -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/utils/adt/int.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/adt/int.c,v retrieving revision 1.72 diff -c -c -r1.72 int.c *** src/backend/utils/adt/int.c 11 Mar 2006 01:19:22 -0000 1.72 --- src/backend/utils/adt/int.c 8 Jun 2006 21:04:23 -0000 *************** *** 770,775 **** --- 770,786 ---- (errcode(ERRCODE_DIVISION_BY_ZERO), errmsg("division by zero"))); + #ifdef WIN32 + /* + * Win32 doesn't throw a catchable exception for + * SELECT -2147483648 /* INT_MIN */ / (-1); + */ + if (arg2 == -1 && arg1 == INT_MIN) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("integer out of range"))); + #endif + result = arg1 / arg2; /*
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> With no Win32 exception detection code in sight, I propose the following
> patch to prevent server crashes for unusual INT_MIN integer division.
The overflow code tries hard to avoid assuming it knows what INT_MIN and
INT_MAX are --- this is maybe not so important for int4 but it is for
int8 (because of our support for int8-less machines). I don't
immediately see how to make this test without assuming you know the
value of INT_MIN, but we ought to try to come up with one.
We do see funny behavior on Intel chips even without Windows, so it'd
be better to not #ifdef WIN32 but use the same overflow test for
everyone.
I would imagine the same problem arises with int8, has anyone checked?
Also, the overflow tests in the intNmul routines seem vulnerable.
regards, tom lane
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > With no Win32 exception detection code in sight, I propose the following
> > patch to prevent server crashes for unusual INT_MIN integer division.
>
> The overflow code tries hard to avoid assuming it knows what INT_MIN and
> INT_MAX are --- this is maybe not so important for int4 but it is for
> int8 (because of our support for int8-less machines). I don't
> immediately see how to make this test without assuming you know the
> value of INT_MIN, but we ought to try to come up with one.
>
> We do see funny behavior on Intel chips even without Windows, so it'd
> be better to not #ifdef WIN32 but use the same overflow test for
> everyone.
> I would imagine the same problem arises with int8, has anyone checked?
Seems int8 is OK on Win32:
postgres=# SELECT (-9223372036854775808) / (-1);
ERROR: bigint out of range
> Also, the overflow tests in the intNmul routines seem vulnerable.
I reproduced the crash using int4 multiplication. Again int8
multiplication seemed OK.
I tried int2 and that seemed OK.
Updated patch attached.
--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/utils/adt/int.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/int.c,v
retrieving revision 1.72
diff -c -c -r1.72 int.c
*** src/backend/utils/adt/int.c 11 Mar 2006 01:19:22 -0000 1.72
--- src/backend/utils/adt/int.c 9 Jun 2006 02:36:32 -0000
***************
*** 735,740 ****
--- 735,751 ----
int32 arg2 = PG_GETARG_INT32(1);
int32 result;
+ #ifdef WIN32
+ /*
+ * Win32 doesn't throw a catchable exception for
+ * SELECT -2147483648 /* INT_MIN */ * (-1);
+ */
+ if (arg2 == -1 && arg1 == INT_MIN)
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("integer out of range")));
+ #endif
+
result = arg1 * arg2;
/*
***************
*** 770,775 ****
--- 781,797 ----
(errcode(ERRCODE_DIVISION_BY_ZERO),
errmsg("division by zero")));
+ #ifdef WIN32
+ /*
+ * Win32 doesn't throw a catchable exception for
+ * SELECT -2147483648 /* INT_MIN */ / (-1);
+ */
+ if (arg2 == -1 && arg1 == INT_MIN)
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("integer out of range")));
+ #endif
+
result = arg1 / arg2;
/*
Patch applied. Backpatch to 8.1.X. --------------------------------------------------------------------------- Bruce Momjian wrote: > Tom Lane wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > With no Win32 exception detection code in sight, I propose the following > > > patch to prevent server crashes for unusual INT_MIN integer division. > > > > The overflow code tries hard to avoid assuming it knows what INT_MIN and > > INT_MAX are --- this is maybe not so important for int4 but it is for > > int8 (because of our support for int8-less machines). I don't > > immediately see how to make this test without assuming you know the > > value of INT_MIN, but we ought to try to come up with one. > > > > We do see funny behavior on Intel chips even without Windows, so it'd > > be better to not #ifdef WIN32 but use the same overflow test for > > everyone. > > > I would imagine the same problem arises with int8, has anyone checked? > > Seems int8 is OK on Win32: > > postgres=# SELECT (-9223372036854775808) / (-1); > ERROR: bigint out of range > > > Also, the overflow tests in the intNmul routines seem vulnerable. > > I reproduced the crash using int4 multiplication. Again int8 > multiplication seemed OK. > > I tried int2 and that seemed OK. > > Updated patch attached. > > -- > Bruce Momjian http://candle.pha.pa.us > EnterpriseDB http://www.enterprisedb.com > > + If your life is a hard drive, Christ can be your backup. + > > ---------------------------(end of broadcast)--------------------------- > TIP 9: In versions below 8.0, the planner will ignore your desire to > choose an index scan if your joining column's datatypes do not > match -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +