Thread: CVS tip problems

CVS tip problems

From
Oliver Elphick
Date:
CVS tip built on Debian unstable, i386, Linux 2.6.5 SMP.
gcc 3.3.3

./configure --with-openssl  --with-pam --with-krb5 --with-gnu-ld
--with-python --with-perl --with-tcl --with-pgport=5342
--enable-thread-safety --enable-nls --enable-integer-datetimes
--enable-debug --enable-cassert --enable-depend

1.  There are regression failures on timestamptz and horology which seem
to have come about either on input or output of timestamps with
fractional seconds.  I tried various inputs and found that certain
timestamps with fractional seconds had one second added to the time.
This appears to be confined to the period from midnight at the start of
Dec 14 1901 GMT to midnight at the start of Jan 01 2000 GMT

junk=# select cast('Dec 13 15:59:59.50 1901 PST' as timestamptz);
      timestamptz
------------------------
 1901-12-13 23:59:59.50
(1 row)

junk=# select cast('Dec 13 16:00:59.50 1901 PST' as timestamptz);
        timestamptz
---------------------------
 1901-12-14 00:01:00.50+00
(1 row)

junk=# select cast('Dec 13 23:59:59.50 1901 GMT' as timestamptz);
      timestamptz
------------------------
 1901-12-13 23:59:59.50
(1 row)

junk=# select cast('Dec 14 00:00:00.50 1901 GMT' as timestamptz);
        timestamptz
---------------------------
 1901-12-14 00:00:01.50+00
(1 row)

I tried debugging this but got a segmentation fault and apparent stack
corruption in gdb, with the reported break point not anywhere I had set
one.  I don't know what to do about that.


2.  If the postmaster is not running, there is garbage in psql's error
message:

olly@linda$ export PGPORT=5342
olly@linda$ export PATH=/usr/local/pgsql/bin:$PATH
olly@linda$ psql junk
psql: could not connect to server: ,*@È        %@
        Is the server running locally and accepting
        connections on Unix domain socket "/tmp/.s.PGSQL.5342"?

olly@linda$ psql -h localhost junk
psql: could not connect to server: ,*@È        %@
        Is the server running on host "localhost" and accepting
        TCP/IP connections on port 5342?


3.  There is a compilation warning that a constant will not fit into a
long in adt.c.  There are two more files where INT64CONST() is required
but not supplied.  Patch attached.

--
Oliver Elphick                                          olly@lfix.co.uk
Isle of Wight                              http://www.lfix.co.uk/oliver
GPG: 1024D/A54310EA  92C8 39E7 280E 3631 3F0E  1EC0 5664 7A2F A543 10EA
                 ========================================
     "Do all things without murmurings and disputings;
      that ye may be blameless and harmless, the sons of
      God, without rebuke, in the midst of a crooked and
      perverse nation, among whom ye shine as lights in the
      world."    Philippians 2:14,15

Attachment

Re: CVS tip problems

From
Tom Lane
Date:
Oliver Elphick <olly@lfix.co.uk> writes:
> 1.  There are regression failures on timestamptz and horology which seem
> to have come about either on input or output of timestamps with
> fractional seconds.

Hmm ... I'm seeing slightly different misbehaviors in the
integer-datetimes and float-datetimes cases.  I think we are about to
have to dig down to the root of the off-by-one-second cases that Tom
Lockhart noticed but never really solved.  I suspect that the issue has
to do with platform-specific rounding of negative values to integer, but
I haven't got the full story quite yet.

> I tried debugging this but got a segmentation fault and apparent stack
> corruption in gdb, with the reported break point not anywhere I had set
> one.  I don't know what to do about that.

Recompiling with -O0 might help debuggability.

> 2.  If the postmaster is not running, there is garbage in psql's error
> message:

Works OK for me --- could you trace that one more fully?

> 3.  There is a compilation warning that a constant will not fit into a
> long in adt.c.  There are two more files where INT64CONST() is required
> but not supplied.  Patch attached.

Not sure about these ... does anyone else get these warnings?
        regards, tom lane


Re: CVS tip problems

From
Tom Lane
Date:
Oliver Elphick <olly@lfix.co.uk> writes:
> 1.  There are regression failures on timestamptz and horology which seem
> to have come about either on input or output of timestamps with
> fractional seconds.

I believe I've fixed this.

> 2.  If the postmaster is not running, there is garbage in psql's error
> message:

I can't duplicate that here.  It looks to me like the probable
explanation is a broken or incompatible version of strerror_r() on your
machine.  Does the failure go away if you build without thread-safety?

> 3.  There is a compilation warning that a constant will not fit into a
> long in adt.c.  There are two more files where INT64CONST() is required
> but not supplied.  Patch attached.

Patch applied, thanks.
        regards, tom lane


Re: CVS tip problems

From
Oliver Elphick
Date:
On Mon, 2004-05-31 at 19:55, Tom Lane wrote:
> Oliver Elphick <olly@lfix.co.uk> writes:
> > 1.  There are regression failures on timestamptz and horology which seem
> > to have come about either on input or output of timestamps with
> > fractional seconds.
> 
> I believe I've fixed this.

All regression tests pass now.

> > 2.  If the postmaster is not running, there is garbage in psql's error
> > message:
> 
> I can't duplicate that here.  It looks to me like the probable
> explanation is a broken or incompatible version of strerror_r() on your
> machine.  Does the failure go away if you build without thread-safety?

Yes it does.

I'll see if I can run with a debugging libc and find it.

-- 
Oliver Elphick                                          olly@lfix.co.uk
Isle of Wight                              http://www.lfix.co.uk/oliver
GPG: 1024D/A54310EA  92C8 39E7 280E 3631 3F0E  1EC0 5664 7A2F A543 10EA
========================================   "How precious also are thy thoughts unto me, O God! how     great is the sum
ofthem! If I should count them, they     are more in number than the sand; when I awake, I am      still with thee."
Psalms139: 17,18 
 



Re: CVS tip problems

From
Tom Lane
Date:
Oliver Elphick <olly@lfix.co.uk> writes:
> On Mon, 2004-05-31 at 19:55, Tom Lane wrote:
>> I can't duplicate that here.  It looks to me like the probable
>> explanation is a broken or incompatible version of strerror_r() on your
>> machine.  Does the failure go away if you build without thread-safety?

> Yes it does.
> I'll see if I can run with a debugging libc and find it.

First you might want to check which flavor of strerror_r() your platform
has --- does it return int or char* ?  The Linux man page for
strerror_r() says
  strerror_r() with prototype as given above is specified by  SUSv3,  and  was  in  use  under Digital Unix and HP
Unix.An incompatible function,  with prototype
 
      char *strerror_r(int errnum, char *buf, size_t n);
  is a GNU extension used by glibc (since 2.0), and must be  regarded  as  obsolete  in view of SUSv3.  The GNU version
may,but need not, use the  user-supplied buffer.  If it does, the result may be truncated in  case  the  supplied
bufferis too small. The result is always NUL-terminated.
 

The code we have appears to assume that the result will always be placed
in the user-supplied buffer, which is apparently NOT what the glibc
version does.
        regards, tom lane


Re: CVS tip problems

From
Bruce Momjian
Date:
Tom Lane wrote:
> Oliver Elphick <olly@lfix.co.uk> writes:
> > On Mon, 2004-05-31 at 19:55, Tom Lane wrote:
> >> I can't duplicate that here.  It looks to me like the probable
> >> explanation is a broken or incompatible version of strerror_r() on your
> >> machine.  Does the failure go away if you build without thread-safety?
> 
> > Yes it does.
> > I'll see if I can run with a debugging libc and find it.
> 
> First you might want to check which flavor of strerror_r() your platform
> has --- does it return int or char* ?  The Linux man page for
> strerror_r() says
> 
>    strerror_r() with prototype as given above is specified by  SUSv3,  and
>    was  in  use  under Digital Unix and HP Unix. An incompatible function,
>    with prototype
> 
>        char *strerror_r(int errnum, char *buf, size_t n);
> 
>    is a GNU extension used by glibc (since 2.0), and must be  regarded  as
>    obsolete  in view of SUSv3.  The GNU version may, but need not, use the
>    user-supplied buffer.  If it does, the result may be truncated in  case
>    the  supplied buffer is too small. The result is always NUL-terminated.
> 
> The code we have appears to assume that the result will always be placed
> in the user-supplied buffer, which is apparently NOT what the glibc
> version does.

What does "may, but need not, use the user-supplied buffer" supposed to
mean in practical terms.  How do they expect us to use it?

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 


Re: CVS tip problems

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> The code we have appears to assume that the result will always be placed
>> in the user-supplied buffer, which is apparently NOT what the glibc
>> version does.

> What does "may, but need not, use the user-supplied buffer" supposed to
> mean in practical terms.  How do they expect us to use it?

AFAICS they expect you to use the function's return value.

The current PG code is really erroneous for *both* strerror_r specs,
since the SUS-spec version doesn't promise to put anything into the
buffer if it returns a failure code.  I think you will have to write
some autoconf code to detect which return type is provided, and do
the right things with the return value in both cases.
        regards, tom lane


Re: CVS tip problems

From
Oliver Elphick
Date:
On Tue, 2004-06-01 at 01:33, Tom Lane wrote:
> First you might want to check which flavor of strerror_r() your platform
> has --- does it return int or char* ?  The Linux man page for
> strerror_r() says

>From the definition in /usr/include/string.h, glibc 2.3.2 still has the
version that returns char* 

-- 
Oliver Elphick                                          olly@lfix.co.uk
Isle of Wight                              http://www.lfix.co.uk/oliver
GPG: 1024D/A54310EA  92C8 39E7 280E 3631 3F0E  1EC0 5664 7A2F A543 10EA
========================================   "Thou will show me the path of life; in thy presence      is fullness of
joy;at thy right hand there are       pleasures for evermore."         Psalms 16:11 
 



Re: CVS tip problems

From
Oliver Elphick
Date:
On Tue, 2004-06-01 at 01:33, Tom Lane wrote:
> First you might want to check which flavor of strerror_r() your platform
> has --- does it return int or char* ?  

I made the following change to the strerror_r call, which makes it work
correctly with threading enabled:

--- src/port/thread.c   23 Apr 2004 18:15:55 -0000      1.20
+++ src/port/thread.c   1 Jun 2004 07:18:26 -0000
@@ -71,7 +71,8 @@#if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && defined(HAVE_STRERROR_R)       /* reentrant
strerror_ris available */       /* some early standards had strerror_r returning char * */
 
-       strerror_r(errnum, strerrbuf, buflen);
+       char buf[256];
+       StrNCpy(strerrbuf, strerror_r(errnum, buf, 256), buflen);       return strerrbuf;
                                               #else
 


(I realise this is not sufficient for a patch to correct the problem.)
-- 
Oliver Elphick                                          olly@lfix.co.uk
Isle of Wight                              http://www.lfix.co.uk/oliver
GPG: 1024D/A54310EA  92C8 39E7 280E 3631 3F0E  1EC0 5664 7A2F A543 10EA
========================================   "Thou will show me the path of life; in thy presence      is fullness of
joy;at thy right hand there are       pleasures for evermore."         Psalms 16:11 
 



Re: CVS tip problems

From
Tom Lane
Date:
Oliver Elphick <olly@lfix.co.uk> writes:
> -       strerror_r(errnum, strerrbuf, buflen);
> +       char buf[256];
> +       StrNCpy(strerrbuf, strerror_r(errnum, buf, 256), buflen);
>         return strerrbuf;

Easier and safer would be

> -       strerror_r(errnum, strerrbuf, buflen);
> -       return strerrbuf;
> +       return strerror_r(errnum, strerrbuf, buflen);

The real point here is that we need to code differently depending on
which flavor of strerror_r we have.
        regards, tom lane