Everything

php

snprintf / ap_php_snprintf, %lld, gcc versions?

php.internals

Roman Neuhauser

21 years ago

Hello,

I'm having a puzzling problem with %lld conversion handling
in ap_php_snprintf, and it seems to be gcc version-dependent.

I found this problem through Statgrab, which has this macro:

#define PHP_SG_ADD_LLVAL(rtz, key, val)	{	\
char tmp[256];	\
int tmp_len = snprintf((char *)&tmp, sizeof(tmp) - 1, "%lld", val);	\
add_assoc_stringl_ex(rtz, key, sizeof(key), tmp, tmp_len, 1);	\
}

main/snprintf.[hc]:

#define snprintf ap_php_snprintf
PHPAPI int ap_php_snprintf(char *buf, size_t len, const char *format,...)

The above macro works fine on some machines, and produces "%ld" on others.
I have these configurations:

working: two FreeBSD-4 boxes (cc is 2.95.4), one of these has PHP 4.3.9
installed, the other one 5.0.3

broken: FreeBSD-5 (cc is 3.4.2 [FreeBSD] 20040728), PHP 4.3.10,
RHEL3 (cc is 3.2.3 20030502 (Red Hat Linux 3.2.3-42)), PHP 4.3.10

I don't see much in common between the broken installs except the major
versions of the compilers.

Has anyone else seen this?

--
How many Vietnam vets does it take to screw in a light bulb?
You don't know, man.  You don't KNOW.
Cause you weren't THERE.             http://bash.org/?255991

Derick Rethans

21 years ago

On Sun, 27 Mar 2005, Roman Neuhauser wrote:

> Hello,
>
> I'm having a puzzling problem with %lld conversion handling
> in ap_php_snprintf, and it seems to be gcc version-dependent.
>
> I found this problem through Statgrab, which has this macro:
>
> #define PHP_SG_ADD_LLVAL(rtz, key, val)	{	\
> 	char tmp[256];	\
> 	int tmp_len = snprintf((char *)&tmp, sizeof(tmp) - 1, "%lld", val);	\
> 	add_assoc_stringl_ex(rtz, key, sizeof(key), tmp, tmp_len, 1);	\
> }
>
> main/snprintf.[hc]:
>
> #define snprintf ap_php_snprintf
> PHPAPI int ap_php_snprintf(char *buf, size_t len, const char *format,...)

For some reason the header file is not included then - that should be
fixed as our own (ap_php_snprintf) function should always be used which
is not gcc dependent.

regards,
Derick

--
Derick Rethans
http://derickrethans.nl | http://ez.no | http://xdebug.org

Roman Neuhauser

21 years ago

# derick@php.net / 2005-03-27 07:49:31 +0200:

> On Sun, 27 Mar 2005, Roman Neuhauser wrote:
> > I'm having a puzzling problem with %lld conversion handling
> > in ap_php_snprintf, and it seems to be gcc version-dependent.
> >
> > I found this problem through Statgrab, which has this macro:
> >
> > #define PHP_SG_ADD_LLVAL(rtz, key, val)	{	\
> > 	char tmp[256];	\
> > 	int tmp_len = snprintf((char *)&tmp, sizeof(tmp) - 1, "%lld", val);	\
> > 	add_assoc_stringl_ex(rtz, key, sizeof(key), tmp, tmp_len, 1);	\
> > }
> >
> > main/snprintf.[hc]:
> >
> > #define snprintf ap_php_snprintf
> > PHPAPI int ap_php_snprintf(char *buf, size_t len, const char *format,...)
>
> For some reason the header file is not included then

That doesn't seem to be the case. The library is built with

cc -I. -I/usr/ports/sysutils/pecl-statgrab/work/Statgrab-0.3 -DPHP_ATOM_INC -I/usr/ports/sysutils/pecl-statgrab/work/Statgrab-0.3/include -I/usr/ports/sysutils/pecl-statgrab/work/Statgrab-0.3/main -I/usr/ports/sysutils/pecl-statgrab/work/Statgrab-0.3 -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/Zend -I/usr/local/include/php/TSRM -I/usr/local/include -DHAVE_CONFIG_H -O -pipe -prefer-pic -c /usr/ports/sysutils/pecl-statgrab/work/Statgrab-0.3/statgrab.c -o statgrab.o >/dev/null 2>&1

and when I replace "-o statgrab.o >/dev/null 2>&1" with "-E" I see:

# 65 "/usr/local/include/php/main/snprintf.h"
int ap_php_snprintf(char *, size_t, const char *, ...) __attribute__ ((format(printf, 3, 4)));

and the macro calls expanded to use ap_php_snprintf().

Plus, this program works on the machines where the Statgrab extension
produces "%ld":

#include <stdio.h>

int
main (int argc, char **argv)
{
char tmp[256];
int tmp_len = snprintf((char *)&tmp, sizeof(tmp) - 1, "%lld", 127LL);
printf("strlen (%s) == %d\n", tmp, tmp_len);
return 0;
}

--
How many Vietnam vets does it take to screw in a light bulb?
You don't know, man.  You don't KNOW.
Cause you weren't THERE.             http://bash.org/?255991

Roman Neuhauser

21 years ago

# derick@php.net / 2005-03-27 07:49:31 +0200:

> On Sun, 27 Mar 2005, Roman Neuhauser wrote:
> > I'm having a puzzling problem with %lld conversion handling
> > in ap_php_snprintf, and it seems to be gcc version-dependent.
> >
> > I found this problem through Statgrab, which has this macro:
> >
> > #define PHP_SG_ADD_LLVAL(rtz, key, val)	{	\
> > 	char tmp[256];	\
> > 	int tmp_len = snprintf((char *)&tmp, sizeof(tmp) - 1, "%lld", val);	\
> > 	add_assoc_stringl_ex(rtz, key, sizeof(key), tmp, tmp_len, 1);	\
> > }
> >
> > main/snprintf.[hc]:
> >
> > #define snprintf ap_php_snprintf
> > PHPAPI int ap_php_snprintf(char *buf, size_t len, const char *format,...)
>
> For some reason the header file is not included then - that should be
> fixed as our own (ap_php_snprintf) function should always be used which
> is not gcc dependent.

Turns out you are right, but probably not the way you expected:

%lld is broken on the boxes where snprintf expands to
%ap_php_snprintf(). The difference is in the installed
include/php/main/snprintf.h file (present vs. missing
HAVE_SNPRINTF check), at least between a pair of hosts I'm looking
at right now.

So now the question is: what is the best (least intrusive) way to
make sure the system snprintf is used? #if 0 the snprintf macro?

BTW, perhaps the unconditional ap_php_snprintf use could be backed
out until the function is mature enough to actually be an
improvement? See also
http://marc.theaimsgroup.com/?l=php-dev&m=110746124520191&w=2

--
How many Vietnam vets does it take to screw in a light bulb?
You don't know, man.  You don't KNOW.
Cause you weren't THERE.             http://bash.org/?255991

Derick Rethans

21 years ago

On Tue, 29 Mar 2005, Roman Neuhauser wrote:

> > > #define snprintf ap_php_snprintf
> > > PHPAPI int ap_php_snprintf(char *buf, size_t len, const char *format,...)
> >
> > For some reason the header file is not included then - that should be
> > fixed as our own (ap_php_snprintf) function should always be used which
> > is not gcc dependent.
>
>     Turns out you are right, but probably not the way you expected:
>
>     %lld is broken on the boxes where snprintf expands to
>     %ap_php_snprintf(). The difference is in the installed
>     include/php/main/snprintf.h file (present vs. missing
>     HAVE_SNPRINTF check), at least between a pair of hosts I'm looking
>     at right now.
>
>     So now the question is: what is the best (least intrusive) way to
>     make sure the system snprintf is used? #if 0 the snprintf macro?

Don't do that :)

>     BTW, perhaps the unconditional ap_php_snprintf use could be backed
>     out until the function is mature enough to actually be an
>     improvement? See also
>     http://marc.theaimsgroup.com/?l=php-dev&m=110746124520191&w=2

No, we should not back it out, but fix our ap_php_snprintf function, but
it seems you already filed a bug about this which is assigned to Marcus.

regards,
Derick

--
Derick Rethans
http://derickrethans.nl | http://ez.no | http://xdebug.org

Roman Neuhauser

21 years ago

# derick@php.net / 2005-03-30 09:55:57 -0500:

> On Tue, 29 Mar 2005, Roman Neuhauser wrote:
>
> > > > #define snprintf ap_php_snprintf
> > > > PHPAPI int ap_php_snprintf(char *buf, size_t len, const char *format,...)
> > >
> > > For some reason the header file is not included then - that should be
> > > fixed as our own (ap_php_snprintf) function should always be used which
> > > is not gcc dependent.
> >
> >     Turns out you are right, but probably not the way you expected:
> >
> >     %lld is broken on the boxes where snprintf expands to
> >     %ap_php_snprintf(). The difference is in the installed
> >     include/php/main/snprintf.h file (present vs. missing
> >     HAVE_SNPRINTF check), at least between a pair of hosts I'm looking
> >     at right now.
> >
> >     So now the question is: what is the best (least intrusive) way to
> >     make sure the system snprintf is used? #if 0 the snprintf macro?
>
> Don't do that :)

To be honest, at this point I don't trust ap_php_s*printf with my
data.

If ap_php_snprintf is meant as a drop-in replacement for snprintf,
then I don't see what would break if I did exactly that (before
recompiling all of PHP + extensions).

I wound up putting #undef snprintf in the extencsion's header file,
and that fixed it, but to be honest, I'm shitting bricks from fear
that other parts of the installation are broken as well, and will
break my code in totally unexpected places, and damage the data in
new ways.

> >     BTW, perhaps the unconditional ap_php_snprintf use could be backed
> >     out until the function is mature enough to actually be an
> >     improvement? See also
> >     http://marc.theaimsgroup.com/?l=php-dev&m=110746124520191&w=2
>
> No, we should not back it out, but fix our ap_php_snprintf function, but
> it seems you already filed a bug about this which is assigned to Marcus.

Yup, I did.

--
How many Vietnam vets does it take to screw in a light bulb?
You don't know, man.  You don't KNOW.
Cause you weren't THERE.             http://bash.org/?255991