laruence Fri, 16 Dec 2011 19:02:52 +0000 Revision: http://svn.php.net/viewvc?view=revision&revision=321073
Log: Fixed bug #60536 (Traits Segfault) # this is a tough one, I think I should explain # Zend use zend_object->properties_table both as zval ** and zval *** # if a zend_object->properties is not initialized, the properties_table is zval ** # while in rebuild_object_properties, zend will store the zval ** to zend_object->properties # then stash the zval ***(ie, zobj->properties_table[0] is zval ** now) to zobj->properties_table[0] # so when a zend_object inherit form multi parent and these parent have a same property_info->offset # properties, will result in a repeat zval **->zval ** transform, which will lead to a segmentfault # *may be* this fix is not the best fix, we should not use this tricky way, and rewrite this mechanism. Bug: https://bugs.php.net/60536 (Analyzed) Traits Segfault Changed paths: U php/php-src/branches/PHP_5_4/NEWS A php/php-src/branches/PHP_5_4/Zend/tests/bug60536_001.phpt A php/php-src/branches/PHP_5_4/Zend/tests/bug60536_002.phpt A php/php-src/branches/PHP_5_4/Zend/tests/bug60536_003.phpt A php/php-src/branches/PHP_5_4/Zend/tests/bug60536_004.phpt A php/php-src/branches/PHP_5_4/Zend/tests/bug60536_005.phpt U php/php-src/branches/PHP_5_4/Zend/zend_object_handlers.c A php/php-src/trunk/Zend/tests/bug60536_001.phpt A php/php-src/trunk/Zend/tests/bug60536_002.phpt A php/php-src/trunk/Zend/tests/bug60536_003.phpt A php/php-src/trunk/Zend/tests/bug60536_004.phpt A php/php-src/trunk/Zend/tests/bug60536_005.phpt U php/php-src/trunk/Zend/zend_object_handlers.c
Modified: php/php-src/branches/PHP_5_4/NEWS =================================================================== --- php/php-src/branches/PHP_5_4/NEWS 2011-12-16 18:48:28 UTC (rev 321072) +++ php/php-src/branches/PHP_5_4/NEWS 2011-12-16 19:02:52 UTC (rev 321073) @@ -4,6 +4,7 @@ - Core: . Added max_input_vars directive to prevent attacks based on hash collisions (Dmitry). + . Fixed bug #60536 (Traits Segfault). (Laruence) - CLI SAPI: . Fixed bug #60477 (Segfault after two multipart/form-data POST requests, one 200 RQ and one 404). (Laruence) Added: php/php-src/branches/PHP_5_4/Zend/tests/bug60536_001.phpt =================================================================== --- php/php-src/branches/PHP_5_4/Zend/tests/bug60536_001.phpt (rev 0) +++ php/php-src/branches/PHP_5_4/Zend/tests/bug60536_001.phpt 2011-12-16 19:02:52 UTC (rev 321073) @@ -0,0 +1,26 @@ +--TEST-- +Bug #60536 (Traits Segfault) +--FILE-- +<?php +trait T { private $x = 0; } +class X { + use T; +} +class Y extends X { + use T; + function x() { + return ++$this->x; + } +} +class Z extends Y { + function z() { + return ++$this->x; + } +} +$a = new Z(); +$a->x(); +echo "DONE"; +?> +--EXPECTF-- +Strict Standards: X and T define the same property ($x) in the composition of Y. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %sbug60536_001.php on line %d +DONE Added: php/php-src/branches/PHP_5_4/Zend/tests/bug60536_002.phpt =================================================================== --- php/php-src/branches/PHP_5_4/Zend/tests/bug60536_002.phpt (rev 0) +++ php/php-src/branches/PHP_5_4/Zend/tests/bug60536_002.phpt 2011-12-16 19:02:52 UTC (rev 321073) @@ -0,0 +1,40 @@ +--TEST-- +The same rules are applied for properties that are defined in the class hierarchy. Thus, if the properties are compatible, a notice is issued, if not a fatal error occures. (relevant with #60536) +--FILE-- +<?php +error_reporting(E_ALL | E_STRICT); + +class Base { + private $hello; +} + +trait THello1 { + private $hello; +} + +echo "PRE-CLASS-GUARD\n"; +class Notice extends Base { + use THello1; + private $hello; +} +echo "POST-CLASS-GUARD\n"; + +// now we do the test for a fatal error + +class TraitsTest { + use THello1; + public $hello; +} + +echo "POST-CLASS-GUARD2\n"; + +$t = new TraitsTest; +$t->hello = "foo"; +?> +--EXPECTF-- +PRE-CLASS-GUARD + +Strict Standards: Notice and THello1 define the same property ($hello) in the composition of Notice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d +POST-CLASS-GUARD + +Fatal error: TraitsTest and THello1 define the same property ($hello) in the composition of TraitsTest. However, the definition differs and is considered incompatible. Class was composed in %s on line %d Added: php/php-src/branches/PHP_5_4/Zend/tests/bug60536_003.phpt =================================================================== --- php/php-src/branches/PHP_5_4/Zend/tests/bug60536_003.phpt (rev 0) +++ php/php-src/branches/PHP_5_4/Zend/tests/bug60536_003.phpt 2011-12-16 19:02:52 UTC (rev 321073) @@ -0,0 +1,49 @@ +--TEST-- +Private (relevant to #60536) +--FILE-- +<?php +error_reporting(E_ALL | E_STRICT); + +class BaseWithPropA { + private $hello = 0; +} + +trait AHelloProperty { + private $hello = 0; +} + +class BaseWithTPropB { + use AHelloProperty; +} + +class SubclassA extends BaseWithPropA { + use AHelloProperty; +} + +class SubclassB extends BaseWithTPropB { + use AHelloProperty; +} + +$a = new SubclassA; +var_dump($a); + +$b = new SubclassB; +var_dump($b); + +?> +--EXPECTF-- +Strict Standards: BaseWithPropA and AHelloProperty define the same property ($hello) in the composition of SubclassA. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %sbug60536_003.php on line %d + +Strict Standards: BaseWithTPropB and AHelloProperty define the same property ($hello) in the composition of SubclassB. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %sbug60536_003.php on line %d +object(SubclassA)#%d (2) { + ["hello":"SubclassA":private]=> + int(0) + ["hello":"BaseWithPropA":private]=> + int(0) +} +object(SubclassB)#%d (2) { + ["hello":"SubclassB":private]=> + int(0) + ["hello":"BaseWithTPropB":private]=> + int(0) +} Added: php/php-src/branches/PHP_5_4/Zend/tests/bug60536_004.phpt =================================================================== --- php/php-src/branches/PHP_5_4/Zend/tests/bug60536_004.phpt (rev 0) +++ php/php-src/branches/PHP_5_4/Zend/tests/bug60536_004.phpt 2011-12-16 19:02:52 UTC (rev 321073) @@ -0,0 +1,39 @@ +--TEST-- +Introducing new private variables of the same name in a subclass is ok, and does not lead to any output. That is consitent with normal inheritance handling. (relevant to #60536) +--FILE-- +<?php +error_reporting(E_ALL | E_STRICT); + +class Base { + private $hello; +} + +trait THello1 { + private $hello; +} + +// Now we use the trait, which happens to introduce another private variable +// but they are distinct, and not related to each other, so no warning. +echo "PRE-CLASS-GUARD\n"; +class SameNameInSubClassNoNotice extends Base { + use THello1; +} +echo "POST-CLASS-GUARD\n"; + +// now the same with a class that defines the property itself, +// that should give the expected strict warning. + +class Notice extends Base { + use THello1; + private $hello; +} +echo "POST-CLASS-GUARD2\n"; +?> +--EXPECTF-- +PRE-CLASS-GUARD + +Strict Standards: Base and THello1 define the same property ($hello) in the composition of SameNameInSubClassNoNotice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %sbug60536_004.php on line %d +POST-CLASS-GUARD + +Strict Standards: Notice and THello1 define the same property ($hello) in the composition of Notice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %sbug60536_004.php on line %d +POST-CLASS-GUARD2 Added: php/php-src/branches/PHP_5_4/Zend/tests/bug60536_005.phpt =================================================================== --- php/php-src/branches/PHP_5_4/Zend/tests/bug60536_005.phpt (rev 0) +++ php/php-src/branches/PHP_5_4/Zend/tests/bug60536_005.phpt 2011-12-16 19:02:52 UTC (rev 321073) @@ -0,0 +1,38 @@ +--TEST-- +Introducing new private variables of the same name in a subclass is ok, and does not lead to any output. That is consitent with normal inheritance handling. (relevant to #60536) +--FILE-- +<?php +error_reporting(E_ALL | E_STRICT); + +class Base { + protected $hello; +} + +trait THello1 { + protected $hello; +} + +// Protected and public are handle more strict with a warning then what is +// expected from normal inheritance since they can have easier coliding semantics +echo "PRE-CLASS-GUARD\n"; +class SameNameInSubClassProducesNotice extends Base { + use THello1; +} +echo "POST-CLASS-GUARD\n"; + +// now the same with a class that defines the property itself, too. + +class Notice extends Base { + use THello1; + protected $hello; +} +echo "POST-CLASS-GUARD2\n"; +?> +--EXPECTF-- +PRE-CLASS-GUARD + +Strict Standards: Base and THello1 define the same property ($hello) in the composition of SameNameInSubClassProducesNotice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d +POST-CLASS-GUARD + +Strict Standards: Notice and THello1 define the same property ($hello) in the composition of Notice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d +POST-CLASS-GUARD2 Modified: php/php-src/branches/PHP_5_4/Zend/zend_object_handlers.c =================================================================== --- php/php-src/branches/PHP_5_4/Zend/zend_object_handlers.c 2011-12-16 18:48:28 UTC (rev 321072) +++ php/php-src/branches/PHP_5_4/Zend/zend_object_handlers.c 2011-12-16 19:02:52 UTC (rev 321073) @@ -62,6 +62,7 @@ ALLOC_HASHTABLE(zobj->properties); zend_hash_init(zobj->properties, 0, NULL, ZVAL_PTR_DTOR, 0); if (ce->default_properties_count) { + int *flags = ecalloc(ce->default_properties_count, sizeof(int)); for (zend_hash_internal_pointer_reset_ex(&ce->properties_info, &pos); zend_hash_get_current_data_ex(&ce->properties_info, (void**)&prop_info, &pos) == SUCCESS; zend_hash_move_forward_ex(&ce->properties_info, &pos)) { @@ -70,6 +71,7 @@ prop_info->offset >= 0 && zobj->properties_table[prop_info->offset]) { zend_hash_quick_add(zobj->properties, prop_info->name, prop_info->name_length+1, prop_info->h, (void**)&zobj->properties_table[prop_info->offset], sizeof(zval*), (void**)&zobj->properties_table[prop_info->offset]); + flags[prop_info->offset] = 1; } } while (ce->parent && ce->parent->default_properties_count) { @@ -81,11 +83,16 @@ (prop_info->flags & ZEND_ACC_STATIC) == 0 && (prop_info->flags & ZEND_ACC_PRIVATE) != 0 && prop_info->offset >= 0 && - zobj->properties_table[prop_info->offset]) { - zend_hash_quick_add(zobj->properties, prop_info->name, prop_info->name_length+1, prop_info->h, (void**)&zobj->properties_table[prop_info->offset], sizeof(zval*), (void**)&zobj->properties_table[prop_info->offset]); + zobj->properties_table[prop_info->offset]) { + if (UNEXPECTED(flags[prop_info->offset])) { + zend_hash_quick_add(zobj->properties, prop_info->name, prop_info->name_length+1, prop_info->h, (void**)zobj->properties_table[prop_info->offset], sizeof(zval*), (void**)&zobj->properties_table[prop_info->offset]); + } else { + zend_hash_quick_add(zobj->properties, prop_info->name, prop_info->name_length+1, prop_info->h, (void**)&zobj->properties_table[prop_info->offset], sizeof(zval*), (void**)&zobj->properties_table[prop_info->offset]); + } } } } + efree(flags); } } } Added: php/php-src/trunk/Zend/tests/bug60536_001.phpt =================================================================== --- php/php-src/trunk/Zend/tests/bug60536_001.phpt (rev 0) +++ php/php-src/trunk/Zend/tests/bug60536_001.phpt 2011-12-16 19:02:52 UTC (rev 321073) @@ -0,0 +1,26 @@ +--TEST-- +Bug #60536 (Traits Segfault) +--FILE-- +<?php +trait T { private $x = 0; } +class X { + use T; +} +class Y extends X { + use T; + function x() { + return ++$this->x; + } +} +class Z extends Y { + function z() { + return ++$this->x; + } +} +$a = new Z(); +$a->x(); +echo "DONE"; +?> +--EXPECTF-- +Strict Standards: X and T define the same property ($x) in the composition of Y. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %sbug60536_001.php on line %d +DONE Added: php/php-src/trunk/Zend/tests/bug60536_002.phpt =================================================================== --- php/php-src/trunk/Zend/tests/bug60536_002.phpt (rev 0) +++ php/php-src/trunk/Zend/tests/bug60536_002.phpt 2011-12-16 19:02:52 UTC (rev 321073) @@ -0,0 +1,40 @@ +--TEST-- +The same rules are applied for properties that are defined in the class hierarchy. Thus, if the properties are compatible, a notice is issued, if not a fatal error occures. (relevant with #60536) +--FILE-- +<?php +error_reporting(E_ALL | E_STRICT); + +class Base { + private $hello; +} + +trait THello1 { + private $hello; +} + +echo "PRE-CLASS-GUARD\n"; +class Notice extends Base { + use THello1; + private $hello; +} +echo "POST-CLASS-GUARD\n"; + +// now we do the test for a fatal error + +class TraitsTest { + use THello1; + public $hello; +} + +echo "POST-CLASS-GUARD2\n"; + +$t = new TraitsTest; +$t->hello = "foo"; +?> +--EXPECTF-- +PRE-CLASS-GUARD + +Strict Standards: Notice and THello1 define the same property ($hello) in the composition of Notice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d +POST-CLASS-GUARD + +Fatal error: TraitsTest and THello1 define the same property ($hello) in the composition of TraitsTest. However, the definition differs and is considered incompatible. Class was composed in %s on line %d Added: php/php-src/trunk/Zend/tests/bug60536_003.phpt =================================================================== --- php/php-src/trunk/Zend/tests/bug60536_003.phpt (rev 0) +++ php/php-src/trunk/Zend/tests/bug60536_003.phpt 2011-12-16 19:02:52 UTC (rev 321073) @@ -0,0 +1,49 @@ +--TEST-- +Private (relevant to #60536) +--FILE-- +<?php +error_reporting(E_ALL | E_STRICT); + +class BaseWithPropA { + private $hello = 0; +} + +trait AHelloProperty { + private $hello = 0; +} + +class BaseWithTPropB { + use AHelloProperty; +} + +class SubclassA extends BaseWithPropA { + use AHelloProperty; +} + +class SubclassB extends BaseWithTPropB { + use AHelloProperty; +} + +$a = new SubclassA; +var_dump($a); + +$b = new SubclassB; +var_dump($b); + +?> +--EXPECTF-- +Strict Standards: BaseWithPropA and AHelloProperty define the same property ($hello) in the composition of SubclassA. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %sbug60536_003.php on line %d + +Strict Standards: BaseWithTPropB and AHelloProperty define the same property ($hello) in the composition of SubclassB. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %sbug60536_003.php on line %d +object(SubclassA)#%d (2) { + ["hello":"SubclassA":private]=> + int(0) + ["hello":"BaseWithPropA":private]=> + int(0) +} +object(SubclassB)#%d (2) { + ["hello":"SubclassB":private]=> + int(0) + ["hello":"BaseWithTPropB":private]=> + int(0) +} Added: php/php-src/trunk/Zend/tests/bug60536_004.phpt =================================================================== --- php/php-src/trunk/Zend/tests/bug60536_004.phpt (rev 0) +++ php/php-src/trunk/Zend/tests/bug60536_004.phpt 2011-12-16 19:02:52 UTC (rev 321073) @@ -0,0 +1,39 @@ +--TEST-- +Introducing new private variables of the same name in a subclass is ok, and does not lead to any output. That is consitent with normal inheritance handling. (relevant to #60536) +--FILE-- +<?php +error_reporting(E_ALL | E_STRICT); + +class Base { + private $hello; +} + +trait THello1 { + private $hello; +} + +// Now we use the trait, which happens to introduce another private variable +// but they are distinct, and not related to each other, so no warning. +echo "PRE-CLASS-GUARD\n"; +class SameNameInSubClassNoNotice extends Base { + use THello1; +} +echo "POST-CLASS-GUARD\n"; + +// now the same with a class that defines the property itself, +// that should give the expected strict warning. + +class Notice extends Base { + use THello1; + private $hello; +} +echo "POST-CLASS-GUARD2\n"; +?> +--EXPECTF-- +PRE-CLASS-GUARD + +Strict Standards: Base and THello1 define the same property ($hello) in the composition of SameNameInSubClassNoNotice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %sbug60536_004.php on line %d +POST-CLASS-GUARD + +Strict Standards: Notice and THello1 define the same property ($hello) in the composition of Notice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %sbug60536_004.php on line %d +POST-CLASS-GUARD2 Added: php/php-src/trunk/Zend/tests/bug60536_005.phpt =================================================================== --- php/php-src/trunk/Zend/tests/bug60536_005.phpt (rev 0) +++ php/php-src/trunk/Zend/tests/bug60536_005.phpt 2011-12-16 19:02:52 UTC (rev 321073) @@ -0,0 +1,38 @@ +--TEST-- +Introducing new private variables of the same name in a subclass is ok, and does not lead to any output. That is consitent with normal inheritance handling. (relevant to #60536) +--FILE-- +<?php +error_reporting(E_ALL | E_STRICT); + +class Base { + protected $hello; +} + +trait THello1 { + protected $hello; +} + +// Protected and public are handle more strict with a warning then what is +// expected from normal inheritance since they can have easier coliding semantics +echo "PRE-CLASS-GUARD\n"; +class SameNameInSubClassProducesNotice extends Base { + use THello1; +} +echo "POST-CLASS-GUARD\n"; + +// now the same with a class that defines the property itself, too. + +class Notice extends Base { + use THello1; + protected $hello; +} +echo "POST-CLASS-GUARD2\n"; +?> +--EXPECTF-- +PRE-CLASS-GUARD + +Strict Standards: Base and THello1 define the same property ($hello) in the composition of SameNameInSubClassProducesNotice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d +POST-CLASS-GUARD + +Strict Standards: Notice and THello1 define the same property ($hello) in the composition of Notice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d +POST-CLASS-GUARD2 Modified: php/php-src/trunk/Zend/zend_object_handlers.c =================================================================== --- php/php-src/trunk/Zend/zend_object_handlers.c 2011-12-16 18:48:28 UTC (rev 321072) +++ php/php-src/trunk/Zend/zend_object_handlers.c 2011-12-16 19:02:52 UTC (rev 321073) @@ -62,6 +62,7 @@ ALLOC_HASHTABLE(zobj->properties); zend_hash_init(zobj->properties, 0, NULL, ZVAL_PTR_DTOR, 0); if (ce->default_properties_count) { + int *flags = ecalloc(ce->default_properties_count, sizeof(int)); for (zend_hash_internal_pointer_reset_ex(&ce->properties_info, &pos); zend_hash_get_current_data_ex(&ce->properties_info, (void**)&prop_info, &pos) == SUCCESS; zend_hash_move_forward_ex(&ce->properties_info, &pos)) { @@ -70,6 +71,7 @@ prop_info->offset >= 0 && zobj->properties_table[prop_info->offset]) { zend_hash_quick_add(zobj->properties, prop_info->name, prop_info->name_length+1, prop_info->h, (void**)&zobj->properties_table[prop_info->offset], sizeof(zval*), (void**)&zobj->properties_table[prop_info->offset]); + flags[prop_info->offset] = 1; } } while (ce->parent && ce->parent->default_properties_count) { @@ -81,11 +83,16 @@ (prop_info->flags & ZEND_ACC_STATIC) == 0 && (prop_info->flags & ZEND_ACC_PRIVATE) != 0 && prop_info->offset >= 0 && - zobj->properties_table[prop_info->offset]) { - zend_hash_quick_add(zobj->properties, prop_info->name, prop_info->name_length+1, prop_info->h, (void**)&zobj->properties_table[prop_info->offset], sizeof(zval*), (void**)&zobj->properties_table[prop_info->offset]); + zobj->properties_table[prop_info->offset]) { + if (UNEXPECTED(flags[prop_info->offset])) { + zend_hash_quick_add(zobj->properties, prop_info->name, prop_info->name_length+1, prop_info->h, (void**)zobj->properties_table[prop_info->offset], sizeof(zval*), (void**)&zobj->properties_table[prop_info->offset]); + } else { + zend_hash_quick_add(zobj->properties, prop_info->name, prop_info->name_length+1, prop_info->h, (void**)&zobj->properties_table[prop_info->offset], sizeof(zval*), (void**)&zobj->properties_table[prop_info->offset]); + } } } } + efree(flags); } } }
-- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php