Merge branch 'PHP-8.4'

* PHP-8.4: ext/gmp: Fix segfault when null is encountered on an overloaded operator ext/gmp: Add behavioural tests for operator overloading
php · Nov 2, 2024 · 5d7fe13 · 5d7fe13
2 parents 9afc66f + 9e2367f
commit 5d7fe13
Show file tree

Hide file tree

Showing 12 changed files with 1,093 additions and 10 deletions.
diff --git a/ext/gmp/gmp.c b/ext/gmp/gmp.c
@@ -338,25 +338,89 @@ static zend_object *gmp_clone_obj(zend_object *obj) /* {{{ */
 }
 /* }}} */
 
-static void shift_operator_helper(gmp_binary_ui_op_t op, zval *return_value, zval *op1, zval *op2, uint8_t opcode) {
-	zend_long shift = zval_get_long(op2);
+static zend_result shift_operator_helper(gmp_binary_ui_op_t op, zval *return_value, zval *op1, zval *op2, uint8_t opcode) {
+	zend_long shift = 0;
+
+	if (UNEXPECTED(Z_TYPE_P(op2) != IS_LONG)) {
+		if (UNEXPECTED(!IS_GMP(op2))) {
+			// For PHP 8.3 and up use zend_try_get_long()
+			switch (Z_TYPE_P(op2)) {
+				case IS_DOUBLE:
+					shift = zval_get_long(op2);
+					if (UNEXPECTED(EG(exception))) {
+						return FAILURE;
+					}
+					break;
+				case IS_STRING:
+					if (is_numeric_str_function(Z_STR_P(op2), &shift, NULL) != IS_LONG) {
+						goto valueof_op_failure;
+					}
+					break;
+				default:
+					goto typeof_op_failure;
+			}
+		} else {
+			// TODO We shouldn't cast the GMP object to int here
+			shift = zval_get_long(op2);
+		}
+	} else {
+		shift = Z_LVAL_P(op2);
+	}
 
 	if (shift < 0) {
 		zend_throw_error(
 			zend_ce_value_error, "%s must be greater than or equal to 0",
 			opcode == ZEND_POW ? "Exponent" : "Shift"
 		);
 		ZVAL_UNDEF(return_value);
-		return;
+		return FAILURE;
 	} else {
 		mpz_ptr gmpnum_op, gmpnum_result;
 		gmp_temp_t temp;
 
-		FETCH_GMP_ZVAL(gmpnum_op, op1, temp, 1);
+		/* We do not use FETCH_GMP_ZVAL(...); here as we don't use convert_to_gmp()
+		 * as we want to handle the emitted exception ourself.  */
+		if (UNEXPECTED(!IS_GMP(op1))) {
+			if (UNEXPECTED(Z_TYPE_P(op1) != IS_LONG)) {
+				goto typeof_op_failure;
+			}
+			mpz_init(temp.num);
+			mpz_set_si(temp.num, Z_LVAL_P(op1));
+			temp.is_used = 1;
+			gmpnum_op = temp.num;
+		} else {
+			gmpnum_op = GET_GMP_FROM_ZVAL(op1);
+			temp.is_used = 0;
+		}
 		INIT_GMP_RETVAL(gmpnum_result);
 		op(gmpnum_result, gmpnum_op, (gmp_ulong) shift);
 		FREE_GMP_TEMP(temp);
+		return SUCCESS;
+	}
+
+typeof_op_failure: ;
+	/* Returning FAILURE without throwing an exception would emit the
+	 * Unsupported operand types: GMP OP TypeOfOp2
+	 * However, this leads to the engine trying to interpret the GMP object as an integer
+	 * and doing the operation that way, which is not something we want. */
+	const char *op_sigil;
+	switch (opcode) {
+		case ZEND_POW:
+			op_sigil = "**";
+			break;
+		case ZEND_SL:
+			op_sigil = "<<";
+			break;
+		case ZEND_SR:
+			op_sigil = ">>";
+			break;
+		EMPTY_SWITCH_DEFAULT_CASE();
 	}
+	zend_type_error("Unsupported operand types: %s %s %s", zend_zval_type_name(op1), op_sigil, zend_zval_type_name(op2));
+	return FAILURE;
+valueof_op_failure:
+	zend_value_error("Number is not an integer string");
+	return FAILURE;
 }
 
 #define DO_BINARY_UI_OP_EX(op, uop, check_b_zero) \
@@ -385,18 +449,15 @@ static zend_result gmp_do_operation_ex(uint8_t opcode, zval *result, zval *op1,
 	case ZEND_MUL:
 		DO_BINARY_UI_OP(mpz_mul);
 	case ZEND_POW:
-		shift_operator_helper(mpz_pow_ui, result, op1, op2, opcode);
-		return SUCCESS;
+		return shift_operator_helper(mpz_pow_ui, result, op1, op2, opcode);
 	case ZEND_DIV:
 		DO_BINARY_UI_OP_EX(mpz_tdiv_q, gmp_mpz_tdiv_q_ui, 1);
 	case ZEND_MOD:
 		DO_BINARY_UI_OP_EX(mpz_mod, gmp_mpz_mod_ui, 1);
 	case ZEND_SL:
-		shift_operator_helper(mpz_mul_2exp, result, op1, op2, opcode);
-		return SUCCESS;
+		return shift_operator_helper(mpz_mul_2exp, result, op1, op2, opcode);
 	case ZEND_SR:
-		shift_operator_helper(mpz_fdiv_q_2exp, result, op1, op2, opcode);
-		return SUCCESS;
+		return shift_operator_helper(mpz_fdiv_q_2exp, result, op1, op2, opcode);
 	case ZEND_BW_OR:
 		DO_BINARY_OP(mpz_ior);
 	case ZEND_BW_AND:
@@ -629,6 +690,13 @@ static zend_result convert_to_gmp(mpz_t gmpnumber, zval *val, zend_long base, ui
 	case IS_STRING: {
 		return convert_zstr_to_gmp(gmpnumber, Z_STR_P(val), base, arg_pos);
 	}
+	case IS_NULL:
+		/* Just reject null for operator overloading */
+		if (arg_pos == 0) {
+			zend_type_error("Number must be of type GMP|string|int, %s given", zend_zval_type_name(val));
+			return FAILURE;
+		}
+		ZEND_FALLTHROUGH;
 	default: {
 		zend_long lval;
 		if (!zend_parse_arg_long_slow(val, &lval, arg_pos)) {

diff --git a/ext/gmp/tests/overloading_cmp_op_with_null.phpt b/ext/gmp/tests/overloading_cmp_op_with_null.phpt
@@ -0,0 +1,53 @@
+--TEST--
+GMP comparison operator overloading supports null
+--EXTENSIONS--
+gmp
+--FILE--
+<?php
+
+$num = gmp_init(42);
+
+try {
+    var_dump($num < null);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num > null);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num <= null);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num >= null);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num == null);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num <=> null);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+?>
+--EXPECT--
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+int(1)
diff --git a/ext/gmp/tests/overloading_with_array.phpt b/ext/gmp/tests/overloading_with_array.phpt
@@ -0,0 +1,84 @@
+--TEST--
+GMP operator overloading does not support []
+--EXTENSIONS--
+gmp
+--FILE--
+<?php
+
+$num = gmp_init(42);
+
+try {
+    var_dump($num + []);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num - []);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num * []);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num / []);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num % []);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num ** []);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num | []);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($num & []);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($num ^ []);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($num << []);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($num >> []);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+?>
+--EXPECT--
+TypeError: Number must be of type GMP|string|int, array given
+TypeError: Number must be of type GMP|string|int, array given
+TypeError: Number must be of type GMP|string|int, array given
+TypeError: Number must be of type GMP|string|int, array given
+TypeError: Number must be of type GMP|string|int, array given
+TypeError: Unsupported operand types: GMP ** array
+TypeError: Number must be of type GMP|string|int, array given
+TypeError: Number must be of type GMP|string|int, array given
+TypeError: Number must be of type GMP|string|int, array given
+TypeError: Unsupported operand types: GMP << array
+TypeError: Unsupported operand types: GMP >> array
diff --git a/ext/gmp/tests/overloading_with_float.phpt b/ext/gmp/tests/overloading_with_float.phpt
@@ -0,0 +1,117 @@
+--TEST--
+GMP operator overloading does support float with no fractional
+--EXTENSIONS--
+gmp
+--FILE--
+<?php
+
+$num = gmp_init(42);
+
+try {
+    var_dump($num + 42.0);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num - 42.0);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num * 42.0);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num / 42.0);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num % 42.0);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num ** 42.0);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    var_dump($num | 42.0);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($num & 42.0);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($num ^ 42.0);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($num << 42.0);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($num >> 42.0);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+?>
+--EXPECT--
+object(GMP)#2 (1) {
+  ["num"]=>
+  string(2) "84"
+}
+object(GMP)#2 (1) {
+  ["num"]=>
+  string(1) "0"
+}
+object(GMP)#2 (1) {
+  ["num"]=>
+  string(4) "1764"
+}
+object(GMP)#2 (1) {
+  ["num"]=>
+  string(1) "1"
+}
+object(GMP)#2 (1) {
+  ["num"]=>
+  string(1) "0"
+}
+object(GMP)#2 (1) {
+  ["num"]=>
+  string(69) "150130937545296572356771972164254457814047970568738777235893533016064"
+}
+object(GMP)#2 (1) {
+  ["num"]=>
+  string(2) "42"
+}
+object(GMP)#2 (1) {
+  ["num"]=>
+  string(2) "42"
+}
+object(GMP)#2 (1) {
+  ["num"]=>
+  string(1) "0"
+}
+object(GMP)#2 (1) {
+  ["num"]=>
+  string(15) "184717953466368"
+}
+object(GMP)#2 (1) {
+  ["num"]=>
+  string(1) "0"
+}