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

Reply via email to