B3, pear segfault, and msgs about deprecated

21 years ago by Pierre-Alain Joye — view source

unread

On Thu, 18 Dec 2003 12:43:03 +0100
Pierre-Alain Joye paj@pearfr.org wrote:

Hello,

I'm trying to see why $ make install-pear-packages segfaults with
HEAD. I hope I'll commit the fixes today. I suspect references
problem...

I got new messages (which where not displayed before):
: var: Deprecated. Please use the public/private/protected modifiers
in XXX.php on line XX

Have these warning/notices been introduced recently?

Or is it related to the e_pedantic thing?

Ok segfault is on line pear/PEAR/Registry.php line 345.

This is a call to unserialize. I do not know yet why it fails. Maybe
(again) the serialized data is corrupted. Anyway unserialize should
""never"" segfault.

hth

pierre

21 years ago by Derick Rethans — view source

unread

Ok segfault is on line pear/PEAR/Registry.php line 345.

This is a call to unserialize. I do not know yet why it fails. Maybe
(again) the serialized data is corrupted. Anyway unserialize should
""never"" segfault.

Can you put the datafile + serialize call in a test case?

Derick

21 years ago by Pierre-Alain Joye — view source

unread

On Thu, 18 Dec 2003 13:50:10 +0100 (CET)
Derick Rethans derick@php.net wrote:

Ok segfault is on line pear/PEAR/Registry.php line 345.

This is a call to unserialize. I do not know yet why it fails. Maybe
(again) the serialized data is corrupted. Anyway unserialize should
""never"" segfault.

Can you put the datafile + serialize call in a test case?

I tried (see links below), but a simple test case does not segfault
with the same file.

Please note a weird notice about an undefined $data variable. The
problem is that $data is a function argument and is used in the 1st line
of the functon... Any idea?

One of the serialized data which cause the segfault:
http://www.pearfr.org/~paj/pear/packageserialized

A patch against /php-src/pear/ to see where that happens and to
reproduce the weird notice:
http://www.pearfr.org/~paj/pear/pear-install-seg.patch

A useless test case:
http://www.pearfr.org/~paj/pear/test_seg.php

It works perfectly, but the same file and code segfaults in PEAR.

A bit lost right now, memory issue?

pierre

21 years ago by Derick Rethans — view source

unread

On Thu, 18 Dec 2003 13:50:10 +0100 (CET)
Derick Rethans derick@php.net wrote:

Ok segfault is on line pear/PEAR/Registry.php line 345.

This is a call to unserialize. I do not know yet why it fails. Maybe
(again) the serialized data is corrupted. Anyway unserialize should
""never"" segfault.

Can you put the datafile + serialize call in a test case?

I tried (see links below), but a simple test case does not segfault
with the same file.

Please note a weird notice about an undefined $data variable. The
problem is that $data is a function argument and is used in the 1st line
of the functon... Any idea?

One of the serialized data which cause the segfault:
http://www.pearfr.org/~paj/pear/packageserialized

A patch against /php-src/pear/ to see where that happens and to
reproduce the weird notice:
http://www.pearfr.org/~paj/pear/pear-install-seg.patch

A useless test case:
http://www.pearfr.org/~paj/pear/test_seg.php

It works perfectly, but the same file and code segfaults in PEAR.

A bit lost right now, memory issue?

Guesso... can you try to run it in valgrind, it should show all memory
overruns etc..

Derick

21 years ago by Pierre-Alain Joye — view source

unread

On Thu, 18 Dec 2003 14:44:29 +0100 (CET)
Derick Rethans derick@php.net wrote:

Guesso... can you try to run it in valgrind, it should show all memory
overruns etc..

Already done, no overuns, and obviously non freed mem after the crash ;)

Important note (dunno why I did not say that before :P ) It does not
crash in debug mode (bt ==> /dev/null :/), but:

/home/paj/cvs/builds/php5/Zend/zend_hash.c(504) : ht=0x40c09dc4 is being
destroyed

and valgrind still does see any problem.

pierre

21 years ago by Marcus Boerger — view source

unread

Hello Pierre-Alain,

Thursday, December 18, 2003, 12:43:03 PM, you wrote:

Hello,

I'm trying to see why $ make install-pear-packages segfaults with HEAD.
I hope I'll commit the fixes today. I suspect references problem...

I got new messages (which where not displayed before):
: var: Deprecated. Please use the public/private/protected modifiers in
XXX.php on line XX

Have these warning/notices been introduced recently?

Or is it related to the e_pedantic thing?

marcus@zaphod /usr/src/php5 $ php -derror_reporting=4095 -r 'class t {var $x;}'

Strict Standards: var: Deprecated. Please use the public/private/protected modifiers in Command line code on line 1

marcus@zaphod /usr/src/php5 $ php -derror_reporting=2047 -r 'class t {var $x;}'

marcus@zaphod /usr/src/php5 $ php -r 'var_dump(E_STRICT);'
int(2048)

regards
marcus

21 years ago by Daniel Convissor — view source

unread

Hi Pierre:

I got new messages (which where not displayed before):
: var: Deprecated. Please use the public/private/protected modifiers in
XXX.php on line XX

I noticed the same thing. The new E_STRICT reporting level produces such
output. But, E_STRICT is turned off by default. Didn't have time to
track down exactly what was going on, but I have a hunch that it is due to
some of the PEAR install scripts set error_reporting by using numbers
rather than constants.

--Dan

--
FREE scripts that make web and database programming easier
http://www.analysisandsolutions.com/software/
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
4015 7th Ave #4AJ, Brooklyn NY v: 718-854-0335 f: 718-854-0409

21 years ago by Derick Rethans — view source

unread

Hi Pierre:

I got new messages (which where not displayed before):
: var: Deprecated. Please use the public/private/protected modifiers in
XXX.php on line XX

I noticed the same thing. The new E_STRICT reporting level produces such
output. But, E_STRICT is turned off by default. Didn't have time to
track down exactly what was going on, but I have a hunch that it is due to
some of the PEAR install scripts set error_reporting by using numbers
rather than constants.

That shouldn't matter. As 2047 was the highest number before, this would
still exclude E_STRICT which is 2048. E_ALL also doesn't include it.

Derick

21 years ago by Greg Beaver — view source

unread

The only call to error_reporting() is in pearcmd.php:

error_reporting(E_ALL & ~E_NOTICE);

I added a var_dump(error_reporting()); and got 2039 out, as expected,
but I am still getting E_STRICT warnings galore. I wonder if E_STRICT
is on no matter what in head? I added & ~E_STRICT (which of course does
nothing) and STILL get the warnings.

Greg

Daniel Convissor wrote:

Hi Pierre:

I got new messages (which where not displayed before):
: var: Deprecated. Please use the public/private/protected modifiers in
XXX.php on line XX

I noticed the same thing. The new E_STRICT reporting level produces such
output. But, E_STRICT is turned off by default. Didn't have time to
track down exactly what was going on, but I have a hunch that it is due to
some of the PEAR install scripts set error_reporting by using numbers
rather than constants.

--Dan

21 years ago by Daniel Convissor — view source

unread

I wonder if E_STRICT is on no matter what in head?

It's not. Something in the pear install scripts is turning it on.

--Dan

--
FREE scripts that make web and database programming easier
http://www.analysisandsolutions.com/software/
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
4015 7th Ave #4AJ, Brooklyn NY v: 718-854-0335 f: 718-854-0409

21 years ago by Greg Beaver — view source

unread

Unless it is possible to turn on E_STRICT without using the
error_reporting() function, then this is not true - there is only 1
error_reporting() in all of the .php files that make up the PEAR core,
try grep if you don't believe me.

Greg

Daniel Convissor wrote:

I wonder if E_STRICT is on no matter what in head?

It's not. Something in the pear install scripts is turning it on.

--Dan

21 years ago by Daniel Convissor — view source

unread

Hi:

Unless it is possible to turn on E_STRICT without using the
error_reporting() function

It's the custom error handler in pearcmd.php. PHP doesn't pay attention
to any error_reporting() settings when a custom error handler is
established. Then, error_handler() doesn't account for the new warning
level.

So, the call to "error_reporting(E_ALL & ~E_NOTICE);" on line 36 can be
blown away because it's supurfluous and error_handler() needs tweaking.

--Dan

--
FREE scripts that make web and database programming easier
http://www.analysisandsolutions.com/software/
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
4015 7th Ave #4AJ, Brooklyn NY v: 718-854-0335 f: 718-854-0409

21 years ago by Christian Schneider — view source

unread

Daniel Convissor wrote:

It's the custom error handler in pearcmd.php. PHP doesn't pay attention
to any error_reporting() settings when a custom error handler is
established. Then, error_handler() doesn't account for the new warning
level.

Is it just me or is this undesired behaviour? I noticed this back when I
was playing around with error_handlers.

I think this should be treated as a bug and be changed for two reasons:

It seems illogical that an error_handler is called for an error level
which is disabled in php.ini
It's a performance issue since the custom error handler is called for
e.g. every access to an undefined array index even though I disabled
that warning.

Opinions?

Chris

21 years ago by Brad Fisher — view source

unread

Personally, I both like and dislike this behavior. I like it for debugging
purposes, since it allows me full control over whether I ignore
error_reporting level or perhaps just log disabled levels instead of
displaying them, etc. On the other hand, there is a minor performance issue,
but if you write your code to not emit E_NOTICES, it shouldn't really be a big
problem.

-Brad

Christian Schneider wrote:

Daniel Convissor wrote:

It's the custom error handler in pearcmd.php. PHP doesn't pay attention
to any error_reporting() settings when a custom error handler is
established. Then, error_handler() doesn't account for the new warning
level.

Is it just me or is this undesired behaviour? I noticed this back when I
was playing around with error_handlers.

I think this should be treated as a bug and be changed for two reasons:

It seems illogical that an error_handler is called for an error level
which is disabled in php.ini

It's a performance issue since the custom error handler is called for
e.g. every access to an undefined array index even though I disabled
that warning.

Opinions?

Chris

21 years ago by Derick Rethans — view source

unread

Daniel Convissor wrote:

It's the custom error handler in pearcmd.php. PHP doesn't pay attention
to any error_reporting() settings when a custom error handler is
established. Then, error_handler() doesn't account for the new warning
level.

Is it just me or is this undesired behaviour? I noticed this back when I
was playing around with error_handlers.

No, it's desired behavior.

I think this should be treated as a bug and be changed for two reasons:

It seems illogical that an error_handler is called for an error level
which is disabled in php.ini

It's a performance issue since the custom error handler is called for
e.g. every access to an undefined array index even though I disabled
that warning.

Performance and error handling don't go together. It's not a valid
argument and the current behavior IS the way it should be. (Please
search the archives for reasons, we discussed this before here).

Derick

21 years ago by Greg Beaver — view source

unread

At the least, this needs to be added to the thin-changes file, as it
will bork any script that uses a custom error handler and has PHP 4 objects.

Thanks for the catch Dan, that fixes it.

Greg

Christian Schneider wrote:

Daniel Convissor wrote:

It's the custom error handler in pearcmd.php. PHP doesn't pay attention
to any error_reporting() settings when a custom error handler is
established. Then, error_handler() doesn't account for the new warning
level.

Is it just me or is this undesired behaviour? I noticed this back when I
was playing around with error_handlers.

I think this should be treated as a bug and be changed for two reasons:

It seems illogical that an error_handler is called for an error level
which is disabled in php.ini

It's a performance issue since the custom error handler is called for
e.g. every access to an undefined array index even though I disabled
that warning.

Opinions?

Chris

21 years ago by Derick Rethans — view source

unread

At the least, this needs to be added to the thin-changes file, as it
will bork any script that uses a custom error handler and has PHP 4 objects.

It won't bork all scripts with a custom errorhandler if their default
action is to ignore core and other errors but only handle the ones they
should handle. I do agree that it should be mentioned in the changes
file though.

Derick

21 years ago by Christian Schneider — view source

unread

Derick Rethans wrote:

It won't bork all scripts with a custom errorhandler if their default
action is to ignore core and other errors but only handle the ones they
should handle. I do agree that it should be mentioned in the changes
file though.

On the other hand you have to be very careful (another way of saying
'avoid') using custom error_handlers anyway because

http://bugs.php.net/bug.php?id=25547

will break your application. Still there in PHP 5.0.0b3RC1

Chris

21 years ago by Daniel Convissor — view source

unread

Greg:

Thanks for the catch Dan, that fixes it.

But your fix uses the E_STRICT constant, which of course doesn't exist in
PHP 4, so an undefined constant notice gets produced.

I'm wondering why there's even a custom error handler in that file. It
doesn't seem to be performing any purpose that isn't met with PHP's own
error handling procedures.

--Dan

--
FREE scripts that make web and database programming easier
http://www.analysisandsolutions.com/software/
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
4015 7th Ave #4AJ, Brooklyn NY v: 718-854-0335 f: 718-854-0409

21 years ago by magnus@php.net — view source

unread

Daniel Convissor wrote:

It's the custom error handler in pearcmd.php. PHP doesn't pay attention
to any error_reporting() settings when a custom error handler is
established. Then, error_handler() doesn't account for the new warning
level.

Is it just me or is this undesired behaviour? I noticed this back when I
was playing around with error_handlers.

I think this should be treated as a bug and be changed for two reasons:

It seems illogical that an error_handler is called for an error level
which is disabled in php.ini

It's a performance issue since the custom error handler is called for
e.g. every access to an undefined array index even though I disabled
that warning.

I find this very useful and don't think it should be changed, maybe
it's just me.

/Magnus

--
Marriage is a lot like the army, everyone complains, but you'd be
surprised at the large number that re-enlist.
-- James Garner

21 years ago by Zeev Suraski — view source

unread

Any chance you can dissect it to the smallest piece of code that still
produces the crash?

At 14:46 18/12/2003, Pierre-Alain Joye wrote:

On Thu, 18 Dec 2003 12:43:03 +0100
Pierre-Alain Joye paj@pearfr.org wrote:

Hello,

I'm trying to see why $ make install-pear-packages segfaults with
HEAD. I hope I'll commit the fixes today. I suspect references
problem...

I got new messages (which where not displayed before):
: var: Deprecated. Please use the public/private/protected modifiers
in XXX.php on line XX

Have these warning/notices been introduced recently?

Or is it related to the e_pedantic thing?

Ok segfault is on line pear/PEAR/Registry.php line 345.

This is a call to unserialize. I do not know yet why it fails. Maybe
(again) the serialized data is corrupted. Anyway unserialize should
""never"" segfault.

hth

pierre

21 years ago by Johannes Schlueter — view source

unread

Hi,

as I wrote in my initial post: Even changing the paramters for
pear/install-pear.php "helps" and I am not able to create a simpler
script reproducing the crash. But meanwhile I've found the time when it
broke:
zend_execute.c,v 1.565 2003/12/14 16:09:07 still works but
zend_execute.c,v 1.566 2003/12/15 07:22:09 breaks. So this seems to be
the causing commit:
http://lists.php.net/article.php?group=php.zend-engine.cvs&article=2118

maybe it helps...

Zeev Suraski wrote:

Any chance you can dissect it to the smallest piece of code that still
produces the crash?

At 14:46 18/12/2003, Pierre-Alain Joye wrote:

On Thu, 18 Dec 2003 12:43:03 +0100
Pierre-Alain Joye paj@pearfr.org wrote:

Hello,

I'm trying to see why $ make install-pear-packages segfaults with
HEAD. I hope I'll commit the fixes today. I suspect references
problem...

I got new messages (which where not displayed before):
: var: Deprecated. Please use the public/private/protected modifiers
in XXX.php on line XX

Have these warning/notices been introduced recently?

Or is it related to the e_pedantic thing?

Ok segfault is on line pear/PEAR/Registry.php line 345.

This is a call to unserialize. I do not know yet why it fails. Maybe
(again) the serialized data is corrupted. Anyway unserialize should
""never"" segfault.

hth

pierre

21 years ago by Rob Richards — view source

unread

Did the problem end up being due to zend_execute.c 1.566 patch?

Within the zend_assign_to_object function, the temp variable needs to be
free'd, otherwise it needs to be handled within the extensions themselves.
Not sure about the other functions in that patch as I only ran into issues
with this function so far in some of the extensions.

ex.

$dom=new domDocument();
$dom->preserveWhiteSpace=false; <- leaks

$array = simplexml_load_string('<array><data>sample data</data></array>');
$array->data = false; <- leaks and is valid syntax as it gets converted to
string

Within dom (pre 1.566 patch) , I had been handling this in my property write
function (removed the other day) via:
if (! PZVAL_IS_REF(value) && value->refcount == 0) {
value->refcount++;
zval_ptr_dtor(&value);
}

Does this need to go back in or is the code from the 1.566 patch going to be
revisited and re-implemented?

Rob

21 years ago by Andi Gutmans — view source

unread

At 06:44 AM 12/19/2003 -0500, Rob Richards wrote:

Did the problem end up being due to zend_execute.c 1.566 patch?

Within the zend_assign_to_object function, the temp variable needs to be
free'd, otherwise it needs to be handled within the extensions themselves.
Not sure about the other functions in that patch as I only ran into issues
with this function so far in some of the extensions.

ex.

$dom=new domDocument();
$dom->preserveWhiteSpace=false; <- leaks

$array = simplexml_load_string('<array><data>sample data</data></array>');
$array->data = false; <- leaks and is valid syntax as it gets converted to
string

Within dom (pre 1.566 patch) , I had been handling this in my property write
function (removed the other day) via:
if (! PZVAL_IS_REF(value) && value->refcount == 0) {
value->refcount++;
zval_ptr_dtor(&value);
}

Does this need to go back in or is the code from the 1.566 patch going to be
revisited and re-implemented?

We're going to have to revisit this problem for RC1. It definitely needs
solving. Right now I suggest you don't put the hack back into DOM and we'll
try and fix and well define the rules of how to return values and with what
reference count.

Andi