Обсуждение: "24" < INT_MIN returns TRUE ???
Hi,
did anyone compile latest CVS with v1.31
utils/adt/numutils.c?
Marc activated some range checks in pg_atoi() and now I have
a very interesting behaviour on a Linux box running gcc 2.8.1
glibc-2.
Inside of pg_atoi(), the value is read into a long. Comparing
a small positive long like 24 against INT_MIN returns TRUE -
dunno how. Putting INT_MIN into another long variable and
comparing the two returns the expected FALSE - so what's
going on here? long, int32 and int have all 4 bytes here.
Someone any clue?
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#========================================= wieck@debis.com (Jan Wieck) #
> Hi, > > did anyone compile latest CVS with v1.31 > utils/adt/numutils.c? > > Marc activated some range checks in pg_atoi() and now I have > a very interesting behaviour on a Linux box running gcc 2.8.1 > glibc-2. > > Inside of pg_atoi(), the value is read into a long. Comparing > a small positive long like 24 against INT_MIN returns TRUE - > dunno how. Putting INT_MIN into another long variable and > comparing the two returns the expected FALSE - so what's > going on here? long, int32 and int have all 4 bytes here. I just reversed out the patch. It was causing initdb to fail! I don't understand why it fails either, but have sent the report back to the patch author. I would love to hear the answer on this one. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
>> Inside of pg_atoi(), the value is read into a long. Comparing
>> a small positive long like 24 against INT_MIN returns TRUE -
>> dunno how. Putting INT_MIN into another long variable and
>> comparing the two returns the expected FALSE - so what's
>> going on here? long, int32 and int have all 4 bytes here.
I believe the problem is that the compiler is deciding that INT_MIN
is of type "unsigned int" or "unsigned long", whereupon the type
promotion rules will cause the comparison to be done in
unsigned-long arithmetic. And indeed, 24 < 0x80000000 in unsigned
arithmetic. When you compare two long variables,
you get the desired behavior of signed long comparison.
Do you have <limits.h>, and if so how does it define INT_MIN?
The default INT_MIN provided at the top of numutils.c is clearly
prone to cause this problem:#ifndef INT_MIN#define INT_MIN (-0x80000000L)#endif
If long is 32 bits then the constant 0x80000000L will be classified
as unsigned long by an ANSI-compliant compiler, whereupon the test
in pg_atoi fails.
The two systems I have here both avoid this problem in <limits.h>,
but I wonder whether you guys have different or no <limits.h>.
I recommend a two-pronged approach to dealing with this bug:
1. The default INT_MIN ought to read#define INT_MIN (-INT_MAX-1)
so that it is classified as a signed rather than unsigned long.
(This is how both of my systems define it in <limits.h>.)
2. The two tests in pg_atoi ought to readif (l < (long) INT_MIN)...if (l > (long) INT_MAX)
The second change should ensure we get a signed-long comparison
even if <limits.h> has provided a broken definition of INT_MIN.
The first change is not necessary to fix this particular bug
if we also apply the second change, but I think leaving INT_MIN
the way it is is trouble waiting to happen.
Bruce, I cannot check this since my system won't show the failure;
would you un-reverse-out the prior patch, add these changes, and
see if it works for you?
regards, tom lane
I said:
> Do you have <limits.h>, and if so how does it define INT_MIN?
Actually, looking closer, it doesn't matter whether you have <limits.h>,
because there is yet a *third* bug in numutils.c:
#ifdef HAVE_LIMITS#include <limits.h>#endif
should be
#ifdef HAVE_LIMITS_H...
because that is how configure and config.h spell the configuration
symbol. Thus, <limits.h> is never included on *any* platform,
and our broken default INT_MIN is always used.
Whoever wrote this code was not having a good day...
regards, tom lane
> I said: > > Do you have <limits.h>, and if so how does it define INT_MIN? > > Actually, looking closer, it doesn't matter whether you have <limits.h>, > because there is yet a *third* bug in numutils.c: > > #ifdef HAVE_LIMITS > #include <limits.h> > #endif > > should be > > #ifdef HAVE_LIMITS_H > ... > > because that is how configure and config.h spell the configuration > symbol. Thus, <limits.h> is never included on *any* platform, > and our broken default INT_MIN is always used. Yes, I caught this when you made that comment about the LIMIT test. I am checking all the other HAVE_ tests. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> Yes, I caught this when you made that comment about the LIMIT test. I > am checking all the other HAVE_ tests. OK, patch reapplyed, and change made to #define test, and default MAX/MIN values. It works fine now. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026