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



Thu, 8 Jul 1999 18:14:02 +0200 (MET DST)
wieck@debis.com (Jan Wieck)
"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.

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, Pennsylvania 19026

Fri, 9 Jul 1999 00:01:27 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] "24" < INT_MIN returns TRUE ???
>> 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 read
	if (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

Fri, 09 Jul 1999 10:49:36 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] "24" < INT_MIN returns TRUE ??? 
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

Fri, 09 Jul 1999 11:02:12 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] "24" < INT_MIN returns TRUE ??? 
> 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, Pennsylvania 19026

Fri, 9 Jul 1999 13:07:04 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] "24" < INT_MIN returns TRUE ???
> 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, Pennsylvania 19026

Fri, 9 Jul 1999 13:36:14 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] "24" < INT_MIN returns TRUE ???