Implement correct incdec handling

php · nikic · Sep 20, 2019 · Sep 24, 2019 · Sep 25, 2019 · Oct 18, 2019
commit 7fc79823213de94ccc9ee963e19e07aabe8a37f9
diff --git a/Zend/tests/type_declarations/union_types/incdec_prop.phpt b/Zend/tests/type_declarations/union_types/incdec_prop.phpt
@@ -0,0 +1,132 @@
+--TEST--
+Increment/decrement a typed property with int|float type
+--FILE--
+<?php
+
+class Test {
+    public int|float $prop;
+    public int|bool $prop2;
+}
+
+/* Incrementing a int|float property past int min/max is legal */
+
+$test = new Test;
+$test->prop = PHP_INT_MAX;
+$x = $test->prop++;
+var_dump(is_double($test->prop));
+
+$test->prop = PHP_INT_MAX;
+$x = ++$test->prop;
+var_dump(is_double($test->prop));
+
+$test->prop = PHP_INT_MIN;
+$x = $test->prop--;
+var_dump(is_double($test->prop));
+
+$test->prop = PHP_INT_MIN;
+$x = --$test->prop;
+var_dump(is_double($test->prop));
+
+$test = new Test;
+$test->prop = PHP_INT_MAX;
+$r =& $test->prop;
+$x = $test->prop++;
+var_dump(is_double($test->prop));
+
+$test->prop = PHP_INT_MAX;
+$x = ++$test->prop;
+$r =& $test->prop;
+var_dump(is_double($test->prop));
+
+$test->prop = PHP_INT_MIN;
+$x = $test->prop--;
+$r =& $test->prop;
+var_dump(is_double($test->prop));
+
+$test->prop = PHP_INT_MIN;
+$x = --$test->prop;
+$r =& $test->prop;
+var_dump(is_double($test->prop));
+
+/* Incrementing a non-int|float property past int min/max is an error,
+ * even if the result of the overflow (a float) would technically be allowed
+ * under a type coercion. */
+
+try {
+    $test->prop2 = PHP_INT_MAX;
+    $x = $test->prop2++;
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    $test->prop2 = PHP_INT_MAX;
+    $x = ++$test->prop2;
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    $test->prop2 = PHP_INT_MIN;
+    $x = $test->prop2--;
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    $test->prop2 = PHP_INT_MIN;
+    $x = --$test->prop2;
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    $test->prop2 = PHP_INT_MAX;
+    $r =& $test->prop2;
+    $x = $test->prop2++;
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    $test->prop2 = PHP_INT_MAX;
+    $r =& $test->prop2;
+    $x = ++$test->prop2;
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    $test->prop2 = PHP_INT_MIN;
+    $r =& $test->prop2;
+    $x = $test->prop2--;
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    $test->prop2 = PHP_INT_MIN;
+    $r =& $test->prop2;
+    $x = --$test->prop2;
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+bool(true)
+bool(true)
+bool(true)
+bool(true)
+bool(true)
+bool(true)
+bool(true)
+bool(true)
+Cannot increment property Test::$prop2 of type int|bool past its maximal value
+Cannot increment property Test::$prop2 of type int|bool past its maximal value
+Cannot decrement property Test::$prop2 of type int|bool past its minimal value
+Cannot decrement property Test::$prop2 of type int|bool past its minimal value
+Cannot increment a reference held by property Test::$prop2 of type int|bool past its maximal value
+Cannot increment a reference held by property Test::$prop2 of type int|bool past its maximal value
+Cannot decrement a reference held by property Test::$prop2 of type int|bool past its minimal value
+Cannot decrement a reference held by property Test::$prop2 of type int|bool past its minimal value
@@ -1650,14 +1650,10 @@ static zend_property_info *zend_get_prop_not_accepting_double(zend_reference *re
 	return NULL;
 }
 
-static ZEND_COLD zend_long zend_throw_incdec_ref_error(zend_reference *ref OPLINE_DC)
+static ZEND_COLD zend_long zend_throw_incdec_ref_error(
+		zend_reference *ref, zend_property_info *error_prop OPLINE_DC)
 {
-	zend_property_info *error_prop = zend_get_prop_not_accepting_double(ref);
-	ZEND_ASSERT(error_prop);
 	zend_string *type_str = zend_type_to_string(error_prop->type);
-	// TODO!!! No longer true with union types
-	/* Currently there should be no way for a typed reference to accept both int and double.
-	 * Generalize this and the related property code once this becomes possible. */
 	if (ZEND_IS_INCREMENT(opline->opcode)) {
 		zend_type_error(
 			"Cannot increment a reference held by property %s::$%s of type %s past its maximal value",
@@ -1714,8 +1710,11 @@ static void zend_incdec_typed_ref(zend_reference *ref, zval *copy OPLINE_DC EXEC
 	}
 
 	if (UNEXPECTED(Z_TYPE_P(var_ptr) == IS_DOUBLE) && Z_TYPE_P(copy) == IS_LONG) {
-		zend_long val = zend_throw_incdec_ref_error(ref OPLINE_CC);
-		ZVAL_LONG(var_ptr, val);
+		zend_property_info *error_prop = zend_get_prop_not_accepting_double(ref);
+		if (UNEXPECTED(error_prop)) {
+			zend_long val = zend_throw_incdec_ref_error(ref, error_prop OPLINE_CC);
+			ZVAL_LONG(var_ptr, val);
+		}
 	} else if (UNEXPECTED(!zend_verify_ref_assignable_zval(ref, var_ptr, EX_USES_STRICT_TYPES()))) {
 		zval_ptr_dtor(var_ptr);
 		ZVAL_COPY_VALUE(var_ptr, copy);
@@ -1742,8 +1741,10 @@ static void zend_incdec_typed_prop(zend_property_info *prop_info, zval *var_ptr,
 	}
 
 	if (UNEXPECTED(Z_TYPE_P(var_ptr) == IS_DOUBLE) && Z_TYPE_P(copy) == IS_LONG) {
-		zend_long val = zend_throw_incdec_prop_error(prop_info OPLINE_CC);
-		ZVAL_LONG(var_ptr, val);
+		if (!(ZEND_TYPE_FULL_MASK(prop_info->type) & MAY_BE_DOUBLE)) {
+			zend_long val = zend_throw_incdec_prop_error(prop_info OPLINE_CC);
+			ZVAL_LONG(var_ptr, val);
+		}
 	} else if (UNEXPECTED(!zend_verify_property_type(prop_info, var_ptr, EX_USES_STRICT_TYPES()))) {
 		zval_ptr_dtor(var_ptr);
 		ZVAL_COPY_VALUE(var_ptr, copy);
@@ -1761,7 +1762,8 @@ static void zend_pre_incdec_property_zval(zval *prop, zend_property_info *prop_i
 		} else {
 			fast_long_decrement_function(prop);
 		}
-		if (UNEXPECTED(Z_TYPE_P(prop) != IS_LONG) && UNEXPECTED(prop_info)) {
+		if (UNEXPECTED(Z_TYPE_P(prop) != IS_LONG) && UNEXPECTED(prop_info)
+				&& !(ZEND_TYPE_FULL_MASK(prop_info->type) & MAY_BE_DOUBLE)) {
 			zend_long val = zend_throw_incdec_prop_error(prop_info OPLINE_CC);
 			ZVAL_LONG(prop, val);
 		}
@@ -1799,7 +1801,8 @@ static void zend_post_incdec_property_zval(zval *prop, zend_property_info *prop_
 		} else {
 			fast_long_decrement_function(prop);
 		}
-		if (UNEXPECTED(Z_TYPE_P(prop) != IS_LONG) && UNEXPECTED(prop_info)) {
+		if (UNEXPECTED(Z_TYPE_P(prop) != IS_LONG) && UNEXPECTED(prop_info)
+				&& !(ZEND_TYPE_FULL_MASK(prop_info->type) & MAY_BE_DOUBLE)) {
 			zend_long val = zend_throw_incdec_prop_error(prop_info OPLINE_CC);
 			ZVAL_LONG(prop, val);
 		}