Thread: snprintf causes regression tests to fail
Hi! The new snpritnf code appears not to deal with 64-bit ints. I'm getting failures on win32 for int8 as well as all the time related tests (win32 uses int8 for tinmestamps). Removing the snprintf code and falling back to the OS code makes everything pass again. I would guess this affects int8 etc on other platforms as well (assuming they use our snprintf and not the libc one), but I haven't checked it. //Magnus
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, Pennsylvania19073
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 On Sun, 27 Feb 2005 19:07:16 +0100, Magnus Hagander <mha@sollentuna.net> wrote: > Hi! > > The new snpritnf code appears not to deal with 64-bit ints. I'm getting > failures on win32 for int8 as well as all the time related tests (win32 > uses int8 for tinmestamps). Removing the snprintf code and falling back > to the OS code makes everything pass again. > > I would guess this affects int8 etc on other platforms as well (assuming > they use our snprintf and not the libc one), but I haven't checked it. > > //Magnus > >
> 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. > > Nick > > On Sun, 27 Feb 2005 19:07:16 +0100, Magnus Hagander <mha@sollentuna.net> > wrote: >> Hi! >> >> The new snpritnf code appears not to deal with 64-bit ints. I'm getting >> failures on win32 for int8 as well as all the time related tests (win32 >> uses int8 for tinmestamps). Removing the snprintf code and falling back >> to the OS code makes everything pass again. >> >> I would guess this affects int8 etc on other platforms as well (assuming >> they use our snprintf and not the libc one), but I haven't checked it. >> >> //Magnus >> >> > > ---------------------------(end of broadcast)--------------------------- > TIP 9: the planner will ignore your desire to choose an index scan if your > joining column's datatypes do not match >
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 On Mon, 28 Feb 2005 19:58:15 +0200, Nicolai Tufar <ntufar@gmail.com> 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) > > Any thoughts? > > > On Mon, 28 Feb 2005 09:17:20 -0500 (EST), Bruce Momjian > <pgman@candle.pha.pa.us> wrote: > > 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 > > > > >
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
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
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. > > -- > 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: >> 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
> 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? It may be that the CLIB has badly broken support for 64bit integers on 32 bit platforms. Does anyone know of any Cygwin/Ming issues? Is this only with the new snprintf code in Win32? Is this a problem with snprintf as implemented in src/port? Is there a reason why we don't use the snprintf that comes with the various C compilers? > > > 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. >> >> -- >> 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 >> > > ---------------------------(end of broadcast)--------------------------- > TIP 8: explain analyze is your friend >
On Tue, 1 Mar 2005 15:38:58 -0500 (EST), pgsql@mohawksoft.com <pgsql@mohawksoft.com> wrote: > Is there a reason why we don't use the snprintf that comes with the > various C compilers? snprintf() is usually buried in OS libraries. We implement our own snprintf to make things like this: snprintf(buf,"%2$s %1$s","world","Hello"); which is not supported on some platforms work. We do it for national language translation of messages. In some languages you may need to change order of parameters to make a meaningful sentence. Another question is why we are using it for printing values from database. I am not too good on function overriding in standard C but maybe there is a way to usage of standard snprintf() in a particular place.
> On Tue, 1 Mar 2005 15:38:58 -0500 (EST), pgsql@mohawksoft.com > <pgsql@mohawksoft.com> wrote: >> Is there a reason why we don't use the snprintf that comes with the >> various C compilers? > > snprintf() is usually buried in OS libraries. We implement > our own snprintf to make things like this: > snprintf(buf,"%2$s %1$s","world","Hello"); > which is not supported on some platforms work. > > We do it for national language translation of > messages. In some languages you may need > to change order of parameters to make a meaningful > sentence. > > Another question is why we are using it for printing > values from database. I am not too good on function > overriding in standard C but maybe there is a way > to usage of standard snprintf() in a particular place. > Well, here is a stupid question, do we even know which snprintf we are using on Win32? May it be possible that we are using the MingW version which may be broken?
pgsql@mohawksoft.com writes: > Well, here is a stupid question, do we even know which snprintf we are > using on Win32? May it be possible that we are using the MingW version > which may be broken? The regression tests were not failing until we started using the port/ version ... regards, tom lane
On Tue, 1 Mar 2005 16:20:50 -0500 (EST), pgsql@mohawksoft.com <pgsql@mohawksoft.com> wrote: > > Well, here is a stupid question, do we even know which snprintf we are > using on Win32? May it be possible that we are using the MingW version > which may be broken? Defenitely not. I checked it by placing debugging print statements in code.
Nicolai Tufar <ntufar@gmail.com> writes: > 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. Just out of curiosity, do either HAVE_INT64 or HAVE_UINT64 get set in pg_config.h? The observed symptoms would be explained if typedef int64 were ending up as "long" rather than "long long". Looking at the #ifdef nest in include/c.h, there are a couple of ways that could happen, including importing a definition from system header files. If this were happening, it would presumably break all int8 math not only snprintf, so I'm not sure it's the story. As far as I've seen, no one has actually posted the regression diffs seen in this failure, so most of us are in the dark about the details of the problem. regards, tom lane
On Tue, 01 Mar 2005 17:45:31 -0500, Tom Lane > Just out of curiosity, do either HAVE_INT64 or HAVE_UINT64 get set > in pg_config.h? pg_config.h is attached. What drew my attention is the following declaration: /* Define to 1 if `long long int' works and is 64 bits. */ #define HAVE_LONG_LONG_INT_64 is it normal? should it not be like this: #define HAVE_LONG_LONG_INT_64 1
Attachment
I have some new information. If I add the attached patch to snprintf.c, I should see see snprintf() being called printing "0", vsnprintf() printing "1" and dopr(), "2". However, I only see "0" printing in the server logs. I think this means it is finding our /port/snprintf(), but when it calls vsnprintf, it must be using some other version, probably the operating system version that doesn't support %lld. I am also attaching the 'nm' output from libpgport_srv.a which does show vsnprintf as being defined. Win32 doesn't like multiply defined symbols so we use -Wl,--allow-multiple-definition to allow multiple symbols. I bet if I define LONG_LONG_INT_FORMAT as '%I64d' it would pass the regression tests. (I will test now.) Our config/c-library.m4 file confirms that format for MinGW: # MinGW uses '%I64d', though gcc throws an warning with -Wall, The big question is why our own vsnprintf() is not being called from snprintf() in our port file. -- 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 *** snprintf.c Tue Mar 1 19:02:13 2005 --- /laptop/tmp/snprintf.c Tue Mar 1 20:27:21 2005 *************** *** 96,101 **** --- 96,102 ---- int len; va_list args; + puts("0"); va_start(args, fmt); len = vsnprintf(str, count, fmt, args); va_end(args); *************** *** 109,114 **** --- 110,116 ---- char *end; str[0] = '\0'; end = str + count - 1; + puts("1"); dopr(str, fmt, args, end); if (count > 0) end[0] = '\0'; *************** *** 178,183 **** --- 180,186 ---- int realpos; } fmtpar[NL_ARGMAX+1], *fmtparptr[NL_ARGMAX+1]; + puts("2"); format_save = format; output = buffer; crypt.o: 00000000 b .bss 00000000 d .data 00000000 t .text 00000060 b _a64toi 00002ce0 b _CF6464 000003c0 t _CIFP 000034e0 b _constdatablock 00000441 T _crypt 000034f0 b _cryptresult 00000760 t _des_cipher 00000000 d _des_ready 000006c2 t _des_setkey 000000c0 t _ExpandTr 000018e0 b _IE3264 00000a84 t _init_des 00000f0f t _init_perm 00000080 t _IP 00000400 t _itoa64 00003510 b _KS 000003a0 t _P32Tr 00000100 t _PC1 000000e0 b _PC1ROT 00000160 t _PC2 000008e0 b _PC2ROT 00000000 b _perm.0 00000000 t _permute 00000138 t _Rotates 000001a0 t _S 00001ce0 b _SPE 00000040 b _tmp32.1 fseeko.o: 00000000 b .bss 00000000 d .data 00000000 t .text getrusage.o: 00000000 b .bss 00000000 d .data 00000000 t .text U ___udivdi3 U ___umoddi3 U __dosmaperr U __errno U _GetCurrentProcess@0 U _GetLastError@0 U _GetProcessTimes@20 00000000 T _getrusage inet_aton.o: 00000000 b .bss 00000000 d .data 00000000 t .text U __imp____mb_cur_max U __imp___pctype U __isctype U _htonl@4 00000000 T _inet_aton random.o: 00000000 b .bss 00000000 d .data 00000000 t .text U _lrand48 00000000 T _random srandom.o: 00000000 b .bss 00000000 d .data 00000000 t .text U _srand48 00000000 T _srandom unsetenv.o: 00000000 b .bss 00000000 d .data 00000000 t .text U _getenv U _malloc U _putenv U _sprintf 00000004 T _unsetenv getaddrinfo_srv.o: 00000000 b .bss 00000000 d .data 00000000 t .text U _atoi U _free U _gethostbyname@4 U _htonl@4 U _htons@4 U _inet_aton U _inet_ntoa@4 U _malloc U _memcpy U _ntohs@4 0000025a T _pg_freeaddrinfo 000002ca T _pg_gai_strerror 00000000 T _pg_getaddrinfo 000002f6 T _pg_getnameinfo U _snprintf U _WSAGetLastError@0 copydir.o: 00000000 b .bss 00000000 d .data 00000000 t .text 00000000 t ___func__.0 U _AllocateDir 000000a5 T _copydir U _CopyFileA@12 U _errcode_for_file_access U _errfinish U _errmsg U _errstart U _FreeDir U _mkdir U _readdir U _snprintf gettimeofday.o: 00000000 b .bss 00000000 d .data 00000000 t .text U ___udivdi3 00000000 t _epoch U _GetSystemTime@4 00000008 T _gettimeofday U _SystemTimeToFileTime@8 kill.o: 00000000 b .bss 00000000 d .data 00000000 t .text U __errno U _CallNamedPipeA@28 U _GetLastError@0 00000015 T _pgkill U _wsprintfA open.o: 00000000 b .bss 00000000 d .data 00000000 t .text U __assert U __errno U __open_osfhandle U __setmode U _CloseHandle@4 U _CreateFileA@28 U _GetLastError@0 00000000 t _openFlagsToCreateFileFlags 00000160 T _win32_open rand.o: 00000000 b .bss 00000000 d .data 00000000 t .text 00000000 t __dorand48 0000000c D __rand48_add 00000006 D __rand48_mult 00000000 D __rand48_seed 000000bd T _lrand48 000000ed T _srand48 snprintf.o: 00000000 b .bss 00000000 d .data 00000000 t .text U ___udivdi3 U ___umoddi3 U __alloca U __vsnprintf 000000dd t _dopr 00000c73 t _dopr_outch 00000c09 t _dostr 00000b29 t _fmtfloat 00000986 t _fmtnum 00000000 b _fmtpar.0 00050050 b _fmtparptr.1 000008cc t _fmtstr 00000000 T _printf U _putchar U _puts 00000055 T _snprintf U _sprintf 00000083 T _vsnprintf dirmod_srv.o: 00000000 b .bss 00000000 d .data 00000000 t .text 00000000 t ___func__.0 00000137 t ___func__.1 00000230 t ___func__.2 000005ec t ___func__.3 U __errno U _closedir U _CloseHandle@4 U _CreateDirectoryA@8 U _CreateFileA@28 U _DeviceIoControl@32 U _elog_finish U _elog_start U _errcode_for_file_access U _errfinish U _errmsg U _errstart 000004c2 t _fnames 000005b8 t _fnames_cleanup U _FormatMessageA@28 U _GetLastError@0 U _LocalFree@4 U _MoveFileExA@12 U _MultiByteToWideChar@24 U _opendir U _pfree U _pgport_palloc U _pgport_pstrdup 00000081 T _pgrename 00000282 T _pgsymlink 00000182 T _pgunlink U _pgwin32_backend_usleep U _readdir U _RemoveDirectoryA@4 U _repalloc U _rmdir 0000062c T _rmtree U _snprintf U _sprintf U _stat U _strchr U _strcpy U _unlink exec_srv.o: 00000000 b .bss 00000000 d .data 00000000 t .text 000000d2 t ___func__.0 000008a5 t ___func__.1 U __errno U __imp____mb_cur_max U __imp___pctype U __isctype U __pclose U _canonicalize_path U _CloseHandle@4 U _CreatePipe@16 U _CreateProcessA@40 U _DuplicateHandle@28 U _elog_finish U _elog_start 00000181 T _find_my_exec 00000514 T _find_other_exec U _first_dir_separator U _first_path_separator U _GetCurrentProcess@0 U _getcwd U _getenv U _join_path_components U _last_dir_separator U _memset 00000971 T _pclose_check U _perror U _pg_strcasecmp 00000610 t _pipe_read_line U _ReadFile@20 000004f3 t _resolve_symlinks U _snprintf U _stat U _strcat U _strchr U _strcmp U _strcpy U _strerror U _strncpy 00000005 t _validate_exec U _WaitForSingleObject@8 noblock.o: 00000000 b .bss 00000000 d .data 00000000 t .text U _ioctlsocket@12 00000000 T _set_noblock path.o: 00000000 b .bss 00000000 d .data 00000000 t .text U __imp____mb_cur_max U __imp___iob U __imp___pctype U __isctype 00000210 T _canonicalize_path U _exit U _find_my_exec 0000008f T _first_dir_separator 000000bc T _first_path_separator U _fprintf 000004d3 T _get_etc_path 00000620 T _get_home_path 0000050b T _get_include_path 00000580 T _get_includeserver_path 000005b4 T _get_lib_path 000005f3 T _get_locale_path 00000684 T _get_parent_directory 0000052a T _get_pkginclude_path 000005d3 T _get_pkglib_path 00000323 T _get_progname 0000049f T _get_share_path U _getenv 0000012d T _join_path_components 000000d9 T _last_dir_separator 0000010e T _make_native_path 000003d8 t _make_relative_path U _pg_strcasecmp U _putenv 000006bc T _set_pglocale_pgservice U _setlocale U _SHGetFolderPathA@20 00000000 t _skip_drive U _snprintf U _strdup U _strncmp U _strncpy 00000772 t _trim_directory 00000805 t _trim_trailing_separator pipe.o: 00000000 b .bss 00000000 d .data 00000000 t .text 00000000 t ___func__.0 U _accept@12 U _bind@12 U _closesocket@4 U _connect@12 U _errfinish U _errmsg_internal U _errstart U _getsockname@12 U _htonl@4 U _htons@4 U _listen@8 00000164 T _pgpipe 0000042e T _piperead U _recv@16 U _socket@12 U _WSAGetLastError@0 pgsleep.o: 00000000 b .bss 00000000 d .data 00000000 t .text 00000000 T _pg_usleep U _SleepEx@8 pgstrcasecmp.o: 00000000 b .bss 00000000 d .data 00000000 t .text U __imp____mb_cur_max U __imp___pctype U __isctype 00000000 T _pg_strcasecmp 0000010e T _pg_strncasecmp 000002aa T _pg_tolower 00000234 T _pg_toupper U _tolower U _toupper sprompt.o: 00000000 b .bss 00000000 d .data 00000000 t .text U __imp___iob U _fclose U _fflush U _fgets U _fopen U _fputc U _fputs U _free U _GetConsoleMode@8 U _GetStdHandle@4 U _malloc 00000000 D _prompt_state U _SetConsoleMode@8 0000000d T _simple_prompt thread_srv.o: 00000000 b .bss 00000000 d .data 00000000 t .text U _gethostbyname@4 00000039 T _pqGethostbyname 00000000 T _pqStrerror U _strerror U _strncpy U _WSAGetLastError@0
Bruce Momjian <pgman@candle.pha.pa.us> writes: > I think this means it is finding our /port/snprintf(), but when it calls > vsnprintf, it must be using some other version, probably the operating > system version that doesn't support %lld. Ya know, I was wondering about that but dismissed it because the routines were all declared in the same file. Windows' linker must behave very oddly to do this. Does it help if you flip the order of the snprintf and vsnprintf functions in snprintf.c? regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > I think this means it is finding our /port/snprintf(), but when it calls > > vsnprintf, it must be using some other version, probably the operating > > system version that doesn't support %lld. I can confirm that using "%I64d" for the printf format allows the regression tests to pass for int8. > Ya know, I was wondering about that but dismissed it because the > routines were all declared in the same file. Windows' linker must > behave very oddly to do this. Yep, quite unusual. > Does it help if you flip the order of the snprintf and vsnprintf > functions in snprintf.c? Testing now. -- 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
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > I think this means it is finding our /port/snprintf(), but when it calls > > vsnprintf, it must be using some other version, probably the operating > > system version that doesn't support %lld. > > Ya know, I was wondering about that but dismissed it because the > routines were all declared in the same file. Windows' linker must > behave very oddly to do this. > > Does it help if you flip the order of the snprintf and vsnprintf > functions in snprintf.c? Yes, it fixes the problem and I have applied the reordering with a comment. I will start working on fixing the large fmtpar allocations now. -- 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: > Tom Lane wrote: >> Does it help if you flip the order of the snprintf and vsnprintf >> functions in snprintf.c? > Yes, it fixes the problem and I have applied the reordering with a > comment. Fascinating. > I will start working on fixing the large fmtpar allocations now. Quite honestly, this code is not worth micro-optimizing because it is fundamentally broken. See my other comments in this thread. Can we find a BSD-license implementation that follows the spec? regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> Does it help if you flip the order of the snprintf and vsnprintf > >> functions in snprintf.c? > > > Yes, it fixes the problem and I have applied the reordering with a > > comment. > > Fascinating. > > > I will start working on fixing the large fmtpar allocations now. > > Quite honestly, this code is not worth micro-optimizing because it > is fundamentally broken. See my other comments in this thread. I am working on something that just counts the '%' characters in the format string and allocates an array that size. > Can we find a BSD-license implementation that follows the spec? I would think NetBSD would be our best bet. I will download it and take a look. -- 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: > Tom Lane wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > Tom Lane wrote: > > >> Does it help if you flip the order of the snprintf and vsnprintf > > >> functions in snprintf.c? > > > > > Yes, it fixes the problem and I have applied the reordering with a > > > comment. > > > > Fascinating. > > > > > I will start working on fixing the large fmtpar allocations now. > > > > Quite honestly, this code is not worth micro-optimizing because it > > is fundamentally broken. See my other comments in this thread. > > I am working on something that just counts the '%' characters in the > format string and allocates an array that size. > > > Can we find a BSD-license implementation that follows the spec? > > I would think NetBSD would be our best bet. I will download it and take > a look. Oops, I remember now that NetBSD doesn't support it. I think FreeBSD does. -- 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
> > The big question is why our own vsnprintf() is not being called from > snprintf() in our port file. > I have seen this "problem" before, well, it isn't really a problem I guess. I'm not sure of the gcc compiler options, but.... On the Microsoft compiler if you specify the option "/Gy" it separates the functions within the object file, that way you don't load all the functions from the object if they are not needed. If, however, you create a function with the same name as another function, and one is declared in an object compiled with the "/Gy" option, and the other's object file is not, then if you also use a different function or reference variable in the object file compiled without the "/Gy" option, then the conflicting function will probably be used. Make sense? I would suggest using macro to redefine snprintf and vnsprintf to avoid the issue: #define snprintf pg_snprintf #define vnsprintf pg_vnsprintf