snprintf causes regression tests to fail

Linux and Solaris 10 x86 pass regression tests fine when I force the use of new
snprintf(). The problem should be win32 - specific. I will
investigate it throughly
tonight. Can someone experienced in win32 what can possibly be the problem?

Nick

Nicolai Tufar wrote:

Linux and Solaris 10 x86 pass regression tests fine when I force the use of new
snprintf(). The problem should be win32 - specific. I will
investigate it throughly
tonight. Can someone experienced in win32 what can possibly be the problem?

Yea, I am confused too because my BSD uses the new snprintf.c code was
well. Magnus, what failures are you seeing on Win32?

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

Linux and Solaris 10 x86 pass regression tests fine when I force the use
of new
snprintf(). The problem should be win32 - specific. I will
investigate it throughly
tonight. Can someone experienced in win32 what can possibly be the
problem?

Do we have any idea about what format string causes the regression failure?
It may be that certain integer types are not promoted uniformly when
pushed on the stack.

After some extensive debugging with Magnus's
help we finally managed to a kind of isolate the
problem. We placed snprintf.c in a separate
file, added necessary #includes and wrote
a simple main() function:

main()
{
unsigned long long ull=4567890123456789ULL;
static char buf[1024];
mysnprintf(buf,1024,"%lld\n",ull);
printf(buf);
}

When compiled with -D HAVE_LONG_LONG_INT_64=1
which declares long_long and ulong_long like:

typedef long long long_long;
typedef unsigned long long ulong_long;

It compiles fine and produces desired result. If not,
it produces "-869367531" as in regression tests.

Amazingly enough HAVE_LONG_LONG_INT_64 is
defined when compilation comes to src/port/snprintf.c
but the result is still wrong. I looked into configure.in
but the check for HAVE_LONG_LONG_INT_64 is too
complicated for me to understand. Bruce, could you
take a look at this? I am 90% sure it is an issue with
some configure definitions.

Best regards,
Nicolai

pgsql@mohawksoft.com writes:

Do we have any idea about what format string causes the regression failure?

I'll bet the problem is that configure.in is doing things in the wrong
order: it computes INT64_FORMAT against the system printf before
deciding we should use our own printf.

regards, tom lane

Tom Lane wrote:

pgsql@mohawksoft.com writes:

Do we have any idea about what format string causes the regression failure?

I'll bet the problem is that configure.in is doing things in the wrong
order: it computes INT64_FORMAT against the system printf before
deciding we should use our own printf.

Ah, the problem was introduced here:

revision 1.401
date: 2005/02/24 02:12:15; author: tgl; state: Exp; lines: +13 -12
We aren't supposed to try to run test programs until after we've
verified that AC_TRY_RUN works.

The problem is that the PGAC_FUNC_PRINTF_ARG_CONTROL call was moved
below the printf 64-bit tests. This commited patch moves
PGAC_FUNC_PRINTF_ARG_CONTROL which is after we know AC_TRY_RUN works and
just before printf 64-bit args are tested.

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

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Ah, the problem was introduced here:

Right, it was my fault.

The problem is that the PGAC_FUNC_PRINTF_ARG_CONTROL call was moved
below the printf 64-bit tests. This commited patch moves
PGAC_FUNC_PRINTF_ARG_CONTROL which is after we know AC_TRY_RUN works and
just before printf 64-bit args are tested.

This patch breaks things in a different way: you should not have moved
the AC_LIBOBJ(snprintf) step. Also you randomly placed the arg-control
test between a chunk of code and the comment describing same.

Corrected version committed.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Ah, the problem was introduced here:

Right, it was my fault.

The problem is that the PGAC_FUNC_PRINTF_ARG_CONTROL call was moved
below the printf 64-bit tests. This commited patch moves
PGAC_FUNC_PRINTF_ARG_CONTROL which is after we know AC_TRY_RUN works and
just before printf 64-bit args are tested.

This patch breaks things in a different way: you should not have moved
the AC_LIBOBJ(snprintf) step. Also you randomly placed the arg-control
test between a chunk of code and the comment describing same.

Thanks.
-- 
  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, Pennsylvania 19073

Neither Bruce's nor subsequent Tom's patch did not fix
the issue. The command used is:

make maintainer-clean && ./configure && make && make install && make check

It should have be fine to recompile the source code
completely. I attach the resulting config.log. May be it
will give a clue. Regression test failure are in the
same places as previous ones.

Best regards,
Nicolai

On Mon, 28 Feb 2005 16:29:57 -0500 (EST), Bruce Momjian
<pgman@candle.pha.pa.us> wrote:

Nicolai Tufar wrote:

Neither Bruce's nor subsequent Tom's patch did not fix
the issue. The command used is:

make maintainer-clean && ./configure && make && make install && make check

It should have be fine to recompile the source code
completely. I attach the resulting config.log. May be it
will give a clue. Regression test failure are in the
same places as previous ones.

No log attached. Please email the log to me privately.

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

And while we are on it, I would like to submit minor
changes to make snprintf() vsnprintf() and printf()
functions in src/port/snprintf.c thread-safe.

Best regards,
Nicolai Tufar

Patch applied. Thanks.

---------------------------------------------------------------------------

Nicolai Tufar wrote:

And while we are on it, I would like to submit minor
changes to make snprintf() vsnprintf() and printf()
functions in src/port/snprintf.c thread-safe.

Best regards,
Nicolai Tufar

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

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

Nicolai Tufar wrote:

Regression test diff is attached.
It fails on the following tests:
int8
subselect
union
sequence

It fails to display correctly number "4567890123456789".
In output is shows "-869367531". Apparent overflow or
interpreting int8 as int4.

while rewriting snprintf() I did not touch the actual functions
that convert number to ASCII digit string. In truth, if you
force PostgreSQL to use snprintf.c without my patch applied
it produces the same errors.

What can be wrong? GCC bug? The one I use is:
gcc.exe (GCC) 3.3.1 (mingw special 20030804-1)

I can confirm your failure in current sources on Win32:

template1=# create table test(x int8);
CREATE TABLE
template1=# insert into test values ('4567890123456789');
INSERT 17235 1
template1=# select * from test;
x
------------
-869367531
(1 row)

and it knows it is a large number:

template1=# select * from test where x > 1000::int8;
x
------------
-869367531
(1 row)
template1=# select * from test where x < 1000::int8;
x
---
(0 rows)

I am going to add some debugs to see what is being passed to *printf().

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

Bruce Momjian wrote:

I can confirm your failure in current sources on Win32:

template1=# create table test(x int8);
CREATE TABLE
template1=# insert into test values ('4567890123456789');
INSERT 17235 1
template1=# select * from test;
x
------------
-869367531
(1 row)

and it knows it is a large number:

template1=# select * from test where x > 1000::int8;
x
------------
-869367531
(1 row)
template1=# select * from test where x < 1000::int8;
x
---
(0 rows)

I am going to add some debugs to see what is being passed to *printf().

I have not found the solution yet but am heading to bed. My next guess
is that Win32 isn't handling va_arg(..., long long int) properly.

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

On Tue, 1 Mar 2005 00:55:20 -0500 (EST), Bruce Momjian

My next guess
is that Win32 isn't handling va_arg(..., long long int) properly.

I am trying various combination of number and types
of parameters in my test program and everything prints fine.
When it comes to pg, it fails :(

template1=# select * from test where x > 1000::int8;
x
------------
-869367531
(1 row)

I am not too fluent in source code, could someone
point me to there actual call to snprintf() is being done
when a query like this is executed. I could not find it myslef
:(

Regards,
Nick

Nicolai Tufar wrote:

On Tue, 1 Mar 2005 00:55:20 -0500 (EST), Bruce Momjian

My next guess
is that Win32 isn't handling va_arg(..., long long int) properly.

I am trying various combination of number and types
of parameters in my test program and everything prints fine.
When it comes to pg, it fails :(

template1=# select * from test where x > 1000::int8;
x
------------
-869367531
(1 row)

I am not too fluent in source code, could someone
point me to there actual call to snprintf() is being done
when a query like this is executed. I could not find it myslef

Sure, in src/backend/utils/adt/int8.c, there is a call in int8out():

if ((len = snprintf(buf, MAXINT8LEN, INT64_FORMAT, val)) < 0)

and that calls port/snprintf.c.

I have added a puts() in snprintf.c to make sure it is getting the
long/long specifier.

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

I spent all day debugging it. Still have absolutely
no idea what could possibly go wrong. Does
anyone have a slightest clue what can it be and
why it manifests itself only on win32?

On Tue, 1 Mar 2005 09:29:07 -0500 (EST), Bruce Momjian
<pgman@candle.pha.pa.us> wrote:

Nicolai Tufar wrote:

On Tue, 1 Mar 2005 00:55:20 -0500 (EST), Bruce Momjian

My next guess
is that Win32 isn't handling va_arg(..., long long int) properly.

I am trying various combination of number and types
of parameters in my test program and everything prints fine.
When it comes to pg, it fails :(

template1=# select * from test where x > 1000::int8;
x
------------
-869367531
(1 row)

I am not too fluent in source code, could someone
point me to there actual call to snprintf() is being done
when a query like this is executed. I could not find it myslef

Sure, in src/backend/utils/adt/int8.c, there is a call in int8out():

if ((len = snprintf(buf, MAXINT8LEN, INT64_FORMAT, val)) < 0)

and that calls port/snprintf.c.

I have added a puts() in snprintf.c to make sure it is getting the
long/long specifier.

Just a question, and don't mind me if I am being rude, isn't this the
WRONG PLACE for a "printf" function? Wouldn't an "itoa" function be more
efficient and be less problematic?

pgsql@mohawksoft.com writes:

Just a question, and don't mind me if I am being rude, isn't this the
WRONG PLACE for a "printf" function? Wouldn't an "itoa" function be more
efficient and be less problematic?

This particular call could be so replaced, but it wouldn't solve the
general problem. snprintf has to work.

regards, tom lane