gron                                     Mon, 15 Aug 2011 09:54:06 +0000

Revision: http://svn.php.net/viewvc?view=revision&revision=314933

Log:
Fixed Bug #55372 Incorrect handling of literals led to memory corruption.
# Dmitry you might want to review this patch, since I split up zend_add_literal
# and added a version for post-pass_two() usage.

Bug: https://bugs.php.net/55372 (Verified) Trait fails when method parameter 
has a default
      
Changed paths:
    A   php/php-src/branches/PHP_5_4/Zend/tests/traits/bug55372.phpt
    U   php/php-src/branches/PHP_5_4/Zend/zend_compile.c
    A   php/php-src/trunk/Zend/tests/traits/bug55372.phpt
    U   php/php-src/trunk/Zend/zend_compile.c

Added: php/php-src/branches/PHP_5_4/Zend/tests/traits/bug55372.phpt
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/tests/traits/bug55372.phpt	                        (rev 0)
+++ php/php-src/branches/PHP_5_4/Zend/tests/traits/bug55372.phpt	2011-08-15 09:54:06 UTC (rev 314933)
@@ -0,0 +1,28 @@
+--TEST--
+Bug #55372 (Literal handling in methods is inconsistent, causing memory corruption)
+--FILE--
+<?php
+
+trait testTrait {
+	public function testMethod() {
+		if (1) {
+			$letters1 = range('a', 'z', 1);
+			$letters2 = range('A', 'Z', 1);
+			$letters1 = 'foo';
+			$letters2 = 'baarr';
+			var_dump($letters1);
+			var_dump($letters2);
+		}
+	}
+}
+
+class foo {
+	use testTrait;
+}
+
+$x = new foo;
+$x->testMethod();
+?>
+--EXPECT--
+string(3) "foo"
+string(5) "baarr"

Modified: php/php-src/branches/PHP_5_4/Zend/zend_compile.c
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/zend_compile.c	2011-08-15 09:45:29 UTC (rev 314932)
+++ php/php-src/branches/PHP_5_4/Zend/zend_compile.c	2011-08-15 09:54:06 UTC (rev 314933)
@@ -339,25 +339,47 @@
 }
 /* }}} */

+/* Common part of zend_add_literal and zend_append_individual_literal */
+static inline void zend_insert_literal(zend_op_array *op_array, const zval *zv, int literal_position TSRMLS_DC) /* {{{ */
+{
+	if (Z_TYPE_P(zv) == IS_STRING || Z_TYPE_P(zv) == IS_CONSTANT) {
+		zval *z = (zval*)zv;
+		Z_STRVAL_P(z) = zend_new_interned_string(Z_STRVAL_P(zv), Z_STRLEN_P(zv) + 1, 1 TSRMLS_CC);
+	}
+	CONSTANT_EX(op_array, literal_position) = *zv;
+	Z_SET_REFCOUNT(CONSTANT_EX(op_array, literal_position), 2);
+	Z_SET_ISREF(CONSTANT_EX(op_array, literal_position));
+	op_array->literals[literal_position].hash_value = 0;
+	op_array->literals[literal_position].cache_slot = -1;
+}
+/* }}} */
+
+/* Is used while compiling a function, using the context to keep track
+   of an approximate size to avoid to relocate to often.
+   Literals are truncated to actual size in the second compiler pass (pass_two()). */
 int zend_add_literal(zend_op_array *op_array, const zval *zv TSRMLS_DC) /* {{{ */
 {
 	int i = op_array->last_literal;
 	op_array->last_literal++;
 	if (i >= CG(context).literals_size) {
-		CG(context).literals_size += 16; /* FIXME */
+		while (i >= CG(context).literals_size) {
+			CG(context).literals_size += 16; /* FIXME */
+		}
 		op_array->literals = (zend_literal*)erealloc(op_array->literals, CG(context).literals_size * sizeof(zend_literal));
 	}
-	if (Z_TYPE_P(zv) == IS_STRING || Z_TYPE_P(zv) == IS_CONSTANT) {
-		zval *z = (zval*)zv;
+	zend_insert_literal(op_array, zv, i TSRMLS_CC);
+	return i;
+}
+/* }}} */

-		Z_STRVAL_P(z) =
-			zend_new_interned_string(Z_STRVAL_P(zv), Z_STRLEN_P(zv) + 1, 1 TSRMLS_CC);
-	}
-	CONSTANT_EX(op_array, i) = *zv;
-	Z_SET_REFCOUNT(CONSTANT_EX(op_array, i), 2);
-	Z_SET_ISREF(CONSTANT_EX(op_array, i));
-	op_array->literals[i].hash_value = 0;
-	op_array->literals[i].cache_slot = -1;
+/* Is used after normal compilation to append an additional literal.
+   Allocation is done precisely here. */
+int zend_append_individual_literal(zend_op_array *op_array, const zval *zv TSRMLS_DC) /* {{{ */
+{
+	int i = op_array->last_literal;
+	op_array->last_literal++;
+	op_array->literals = (zend_literal*)erealloc(op_array->literals, (i + 1) * sizeof(zend_literal));
+	zend_insert_literal(op_array, zv, i TSRMLS_CC);
 	return i;
 }
 /* }}} */
@@ -3488,6 +3510,7 @@
 	zval class_name_zv;
 	int class_name_literal;
 	int i;
+	int number_of_literals;

 	if (fe->op_array.static_variables) {
 		HashTable *tmpHash;
@@ -3498,12 +3521,11 @@

 		fe->op_array.static_variables = tmpHash;
 	}
-
-	/* TODO: Verify that this size is a global thing, do not see why but is used
-	         like this elsewhere */
-	literals_copy = (zend_literal*)emalloc(CG(context).literals_size * sizeof(zend_literal));

-	for (i = 0; i < fe->op_array.last_literal; i++) {
+	number_of_literals = fe->op_array.last_literal;
+	literals_copy = (zend_literal*)emalloc(number_of_literals * sizeof(zend_literal));
+
+	for (i = 0; i < number_of_literals; i++) {
 		literals_copy[i] = fe->op_array.literals[i];
 		zval_copy_ctor(&literals_copy[i].constant);
 	}
@@ -3538,14 +3560,15 @@
 		} else {
 			/* if __CLASS__ i.e. T_CLASS_C was used, we need to fix it up here */
 			if (target_ce
-        /* REM: used a IS_NULL place holder with a special marker LVAL */
+				/* REM: used a IS_NULL place holder with a special marker LVAL */
 				&& Z_TYPE_P(opcode_copy[i].op1.zv) == IS_NULL
 				&& Z_LVAL_P(opcode_copy[i].op1.zv) == ZEND_ACC_TRAIT
-        /* Only on merge into an actual class */
-        &&  (ZEND_ACC_TRAIT != (target_ce->ce_flags & ZEND_ACC_TRAIT))) {
+				/* Only on merge into an actual class */
+				&&  (ZEND_ACC_TRAIT != (target_ce->ce_flags & ZEND_ACC_TRAIT)))
+			{
 				INIT_ZVAL(class_name_zv);
 				ZVAL_STRINGL(&class_name_zv, target_ce->name, target_ce->name_length, 1);
-				class_name_literal = zend_add_literal(&fe->op_array, &class_name_zv TSRMLS_CC);
+				class_name_literal = zend_append_individual_literal(&fe->op_array, &class_name_zv TSRMLS_CC);
 				opcode_copy[i].op1.zv = &fe->op_array.literals[class_name_literal].constant;
 			}
 		}
@@ -3558,14 +3581,15 @@
 		} else {
 			/* if __CLASS__ i.e. T_CLASS_C was used, we need to fix it up here */
 			if (target_ce
-        /* REM: used a IS_NULL place holder with a special marker LVAL */
+				/* REM: used a IS_NULL place holder with a special marker LVAL */
 				&& Z_TYPE_P(opcode_copy[i].op2.zv) == IS_NULL
 				&& Z_LVAL_P(opcode_copy[i].op2.zv) == ZEND_ACC_TRAIT
-        /* Only on merge into an actual class */
-        &&  (ZEND_ACC_TRAIT != (target_ce->ce_flags & ZEND_ACC_TRAIT))) {
+				/* Only on merge into an actual class */
+				&&  (ZEND_ACC_TRAIT != (target_ce->ce_flags & ZEND_ACC_TRAIT)))
+			{
 				INIT_ZVAL(class_name_zv);
 				ZVAL_STRINGL(&class_name_zv, target_ce->name, target_ce->name_length, 1);
-				class_name_literal = zend_add_literal(&fe->op_array, &class_name_zv TSRMLS_CC);
+				class_name_literal = zend_append_individual_literal(&fe->op_array, &class_name_zv TSRMLS_CC);
 				opcode_copy[i].op2.zv = &fe->op_array.literals[class_name_literal].constant;
 			}
 		}

Added: php/php-src/trunk/Zend/tests/traits/bug55372.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/bug55372.phpt	                        (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/bug55372.phpt	2011-08-15 09:54:06 UTC (rev 314933)
@@ -0,0 +1,28 @@
+--TEST--
+Bug #55372 (Literal handling in methods is inconsistent, causing memory corruption)
+--FILE--
+<?php
+
+trait testTrait {
+	public function testMethod() {
+		if (1) {
+			$letters1 = range('a', 'z', 1);
+			$letters2 = range('A', 'Z', 1);
+			$letters1 = 'foo';
+			$letters2 = 'baarr';
+			var_dump($letters1);
+			var_dump($letters2);
+		}
+	}
+}
+
+class foo {
+	use testTrait;
+}
+
+$x = new foo;
+$x->testMethod();
+?>
+--EXPECT--
+string(3) "foo"
+string(5) "baarr"

Modified: php/php-src/trunk/Zend/zend_compile.c
===================================================================
--- php/php-src/trunk/Zend/zend_compile.c	2011-08-15 09:45:29 UTC (rev 314932)
+++ php/php-src/trunk/Zend/zend_compile.c	2011-08-15 09:54:06 UTC (rev 314933)
@@ -339,25 +339,47 @@
 }
 /* }}} */

+/* Common part of zend_add_literal and zend_append_individual_literal */
+static inline void zend_insert_literal(zend_op_array *op_array, const zval *zv, int literal_position TSRMLS_DC) /* {{{ */
+{
+	if (Z_TYPE_P(zv) == IS_STRING || Z_TYPE_P(zv) == IS_CONSTANT) {
+		zval *z = (zval*)zv;
+		Z_STRVAL_P(z) = zend_new_interned_string(Z_STRVAL_P(zv), Z_STRLEN_P(zv) + 1, 1 TSRMLS_CC);
+	}
+	CONSTANT_EX(op_array, literal_position) = *zv;
+	Z_SET_REFCOUNT(CONSTANT_EX(op_array, literal_position), 2);
+	Z_SET_ISREF(CONSTANT_EX(op_array, literal_position));
+	op_array->literals[literal_position].hash_value = 0;
+	op_array->literals[literal_position].cache_slot = -1;
+}
+/* }}} */
+
+/* Is used while compiling a function, using the context to keep track
+   of an approximate size to avoid to relocate to often.
+   Literals are truncated to actual size in the second compiler pass (pass_two()). */
 int zend_add_literal(zend_op_array *op_array, const zval *zv TSRMLS_DC) /* {{{ */
 {
 	int i = op_array->last_literal;
 	op_array->last_literal++;
 	if (i >= CG(context).literals_size) {
-		CG(context).literals_size += 16; /* FIXME */
+		while (i >= CG(context).literals_size) {
+			CG(context).literals_size += 16; /* FIXME */
+		}
 		op_array->literals = (zend_literal*)erealloc(op_array->literals, CG(context).literals_size * sizeof(zend_literal));
 	}
-	if (Z_TYPE_P(zv) == IS_STRING || Z_TYPE_P(zv) == IS_CONSTANT) {
-		zval *z = (zval*)zv;
+	zend_insert_literal(op_array, zv, i TSRMLS_CC);
+	return i;
+}
+/* }}} */

-		Z_STRVAL_P(z) =
-			zend_new_interned_string(Z_STRVAL_P(zv), Z_STRLEN_P(zv) + 1, 1 TSRMLS_CC);
-	}
-	CONSTANT_EX(op_array, i) = *zv;
-	Z_SET_REFCOUNT(CONSTANT_EX(op_array, i), 2);
-	Z_SET_ISREF(CONSTANT_EX(op_array, i));
-	op_array->literals[i].hash_value = 0;
-	op_array->literals[i].cache_slot = -1;
+/* Is used after normal compilation to append an additional literal.
+   Allocation is done precisely here. */
+int zend_append_individual_literal(zend_op_array *op_array, const zval *zv TSRMLS_DC) /* {{{ */
+{
+	int i = op_array->last_literal;
+	op_array->last_literal++;
+	op_array->literals = (zend_literal*)erealloc(op_array->literals, (i + 1) * sizeof(zend_literal));
+	zend_insert_literal(op_array, zv, i TSRMLS_CC);
 	return i;
 }
 /* }}} */
@@ -3488,6 +3510,7 @@
 	zval class_name_zv;
 	int class_name_literal;
 	int i;
+	int number_of_literals;

 	if (fe->op_array.static_variables) {
 		HashTable *tmpHash;
@@ -3498,12 +3521,11 @@

 		fe->op_array.static_variables = tmpHash;
 	}
-
-	/* TODO: Verify that this size is a global thing, do not see why but is used
-	         like this elsewhere */
-	literals_copy = (zend_literal*)emalloc(CG(context).literals_size * sizeof(zend_literal));

-	for (i = 0; i < fe->op_array.last_literal; i++) {
+	number_of_literals = fe->op_array.last_literal;
+	literals_copy = (zend_literal*)emalloc(number_of_literals * sizeof(zend_literal));
+
+	for (i = 0; i < number_of_literals; i++) {
 		literals_copy[i] = fe->op_array.literals[i];
 		zval_copy_ctor(&literals_copy[i].constant);
 	}
@@ -3538,14 +3560,15 @@
 		} else {
 			/* if __CLASS__ i.e. T_CLASS_C was used, we need to fix it up here */
 			if (target_ce
-        /* REM: used a IS_NULL place holder with a special marker LVAL */
+				/* REM: used a IS_NULL place holder with a special marker LVAL */
 				&& Z_TYPE_P(opcode_copy[i].op1.zv) == IS_NULL
 				&& Z_LVAL_P(opcode_copy[i].op1.zv) == ZEND_ACC_TRAIT
-        /* Only on merge into an actual class */
-        &&  (ZEND_ACC_TRAIT != (target_ce->ce_flags & ZEND_ACC_TRAIT))) {
+				/* Only on merge into an actual class */
+				&&  (ZEND_ACC_TRAIT != (target_ce->ce_flags & ZEND_ACC_TRAIT)))
+			{
 				INIT_ZVAL(class_name_zv);
 				ZVAL_STRINGL(&class_name_zv, target_ce->name, target_ce->name_length, 1);
-				class_name_literal = zend_add_literal(&fe->op_array, &class_name_zv TSRMLS_CC);
+				class_name_literal = zend_append_individual_literal(&fe->op_array, &class_name_zv TSRMLS_CC);
 				opcode_copy[i].op1.zv = &fe->op_array.literals[class_name_literal].constant;
 			}
 		}
@@ -3558,14 +3581,15 @@
 		} else {
 			/* if __CLASS__ i.e. T_CLASS_C was used, we need to fix it up here */
 			if (target_ce
-        /* REM: used a IS_NULL place holder with a special marker LVAL */
+				/* REM: used a IS_NULL place holder with a special marker LVAL */
 				&& Z_TYPE_P(opcode_copy[i].op2.zv) == IS_NULL
 				&& Z_LVAL_P(opcode_copy[i].op2.zv) == ZEND_ACC_TRAIT
-        /* Only on merge into an actual class */
-        &&  (ZEND_ACC_TRAIT != (target_ce->ce_flags & ZEND_ACC_TRAIT))) {
+				/* Only on merge into an actual class */
+				&&  (ZEND_ACC_TRAIT != (target_ce->ce_flags & ZEND_ACC_TRAIT)))
+			{
 				INIT_ZVAL(class_name_zv);
 				ZVAL_STRINGL(&class_name_zv, target_ce->name, target_ce->name_length, 1);
-				class_name_literal = zend_add_literal(&fe->op_array, &class_name_zv TSRMLS_CC);
+				class_name_literal = zend_append_individual_literal(&fe->op_array, &class_name_zv TSRMLS_CC);
 				opcode_copy[i].op2.zv = &fe->op_array.literals[class_name_literal].constant;
 			}
 		}
-- 
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to