Handle warnings during sccp function evaluation

Some upcoming changes like https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg
will make it somewhat inconvenient to determine whether a given
function invocation will generate a diagnostic. Rather than trying
to exclude this in advance, call the function with diagnostics
suppressed, and check whether anything was thrown.

This adds a new EG flag that is kept specific to the SCCP use-case.
This does not use the error_cb hook as it is a (non-TLS) global,
and doesn't fully suppress error handling besides.

Test this by removing the in advance checks for implode and array_flip.

Author: nikic

Zend/Optimizer/sccp.c

@@ -788,6 +788,7 @@ static bool can_ct_eval_func_call(zend_string *name, uint32_t num_args, zval **a
 		|| zend_string_equals_literal(name, "array_diff")
 		|| zend_string_equals_literal(name, "array_diff_assoc")
 		|| zend_string_equals_literal(name, "array_diff_key")
+		|| zend_string_equals_literal(name, "array_flip")
 		|| zend_string_equals_literal(name, "array_is_list")
 		|| zend_string_equals_literal(name, "array_key_exists")
 		|| zend_string_equals_literal(name, "array_keys")
@@ -804,6 +805,7 @@ static bool can_ct_eval_func_call(zend_string *name, uint32_t num_args, zval **a
 #endif
 		|| zend_string_equals_literal(name, "imagetypes")
 		|| zend_string_equals_literal(name, "in_array")
+		|| zend_string_equals_literal(name, "implode")
 		|| zend_string_equals_literal(name, "ltrim")
 		|| zend_string_equals_literal(name, "php_sapi_name")
 		|| zend_string_equals_literal(name, "php_uname")
@@ -828,41 +830,6 @@ static bool can_ct_eval_func_call(zend_string *name, uint32_t num_args, zval **a
 		return true;
 	}
 
-	/* For the following functions we need to check arguments to prevent warnings during
-	 * evaluation. */
-	if (num_args == 1) {
-		if (zend_string_equals_literal(name, "array_flip")) {
-			zval *entry;
-
-			if (Z_TYPE_P(args[0]) != IS_ARRAY) {
-				return false;
-			}
-			ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(args[0]), entry) {
-				/* Throws warning for non int/string values. */
-				if (Z_TYPE_P(entry) != IS_LONG && Z_TYPE_P(entry) != IS_STRING) {
-					return false;
-				}
-			} ZEND_HASH_FOREACH_END();
-			return true;
-		}
-		if (zend_string_equals_literal(name, "implode")) {
-			zval *entry;
-
-			if (Z_TYPE_P(args[0]) != IS_ARRAY) {
-				return false;
-			}
-
-			ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(args[0]), entry) {
-				/* May throw warning during conversion to string. */
-				if (Z_TYPE_P(entry) > IS_STRING) {
-					return false;
-				}
-			} ZEND_HASH_FOREACH_END();
-			return true;
-		}
-		return false;
-	}
-
 	if (num_args == 2) {
 		if (zend_string_equals_literal(name, "str_repeat")) {
 			/* Avoid creating overly large strings at compile-time. */
@@ -871,29 +838,6 @@ static bool can_ct_eval_func_call(zend_string *name, uint32_t num_args, zval **a
 				&& Z_TYPE_P(args[1]) == IS_LONG
 				&& zend_safe_address(Z_STRLEN_P(args[0]), Z_LVAL_P(args[1]), 0, &overflow) < 64 * 1024
 				&& !overflow;
-		} else if (zend_string_equals_literal(name, "implode")) {
-			zval *entry;
-
-			if ((Z_TYPE_P(args[0]) != IS_STRING || Z_TYPE_P(args[1]) != IS_ARRAY)
-					&& (Z_TYPE_P(args[0]) != IS_ARRAY || Z_TYPE_P(args[1]) != IS_STRING)) {
-				return false;
-			}
-
-			/* May throw warning during conversion to string. */
-			if (Z_TYPE_P(args[0]) == IS_ARRAY) {
-				ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(args[0]), entry) {
-					if (Z_TYPE_P(entry) > IS_STRING) {
-						return false;
-					}
-				} ZEND_HASH_FOREACH_END();
-			} else {
-				ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(args[1]), entry) {
-					if (Z_TYPE_P(entry) > IS_STRING) {
-						return false;
-					}
-				} ZEND_HASH_FOREACH_END();
-			}
-			return true;
 		}
 		return false;
 	}
@@ -971,6 +915,10 @@ static inline int ct_eval_func_call(
 	execute_data->prev_execute_data = &dummy_frame;
 	EG(current_execute_data) = execute_data;
 
+	/* Enable suppression and counting of warnings. */
+	ZEND_ASSERT(EG(capture_warnings_during_sccp) == 0);
+	EG(capture_warnings_during_sccp) = 1;
+
 	EX(func) = func;
 	EX_NUM_ARGS() = num_args;
 	for (i = 0; i < num_args; i++) {
@@ -989,6 +937,12 @@ static inline int ct_eval_func_call(
 		retval = FAILURE;
 	}
 
+	if (EG(capture_warnings_during_sccp) > 1) {
+		zval_ptr_dtor(result);
+		retval = FAILURE;
+	}
+	EG(capture_warnings_during_sccp) = 0;
+
 	efree(execute_data);
 	EG(current_execute_data) = prev_execute_data;
 	return retval;

Zend/zend.c

@@ -714,7 +714,7 @@ static void executor_globals_ctor(zend_executor_globals *executor_globals) /* {{
 	zend_init_exception_op();
 	zend_init_call_trampoline_op();
 	memset(&executor_globals->trampoline, 0, sizeof(zend_op_array));
-	executor_globals->lambda_count = 0;
+	executor_globals->warnings_during_sccp = 0;
 	ZVAL_UNDEF(&executor_globals->user_error_handler);
 	ZVAL_UNDEF(&executor_globals->user_exception_handler);
 	executor_globals->in_autoload = NULL;
@@ -1299,6 +1299,14 @@ static ZEND_COLD void zend_error_impl(
 	zend_stack delayed_oplines_stack;
 	int type = orig_type & E_ALL;
 
+	/* If we're executing a function during SCCP, count any warnings that may be emitted,
+	 * but don't perform any other error handling. */
+	if (EG(capture_warnings_during_sccp)) {
+		ZEND_ASSERT(!(type & E_FATAL_ERRORS) && "Fatal error during SCCP");
+		EG(capture_warnings_during_sccp)++;
+		return;
+	}
+
 	/* Report about uncaught exception in case of fatal errors */
 	if (EG(exception)) {
 		zend_execute_data *ex;

Zend/zend_globals.h

@@ -207,7 +207,7 @@ struct _zend_executor_globals {
 	/* timeout support */
 	zend_long timeout_seconds;
 
-	int lambda_count;
+	int capture_warnings_during_sccp;
 
 	HashTable *ini_directives;
 	HashTable *modified_ini_directives;

ext/opcache/tests/opt/sccp_033.phpt

@@ -0,0 +1,27 @@
+--TEST--
+SCCP: Warning during evaluation of function
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+opcache.optimization_level=-1
+--SKIPIF--
+<?php require_once('skipif.inc'); ?>
+--FILE--
+<?php
+
+function test() {
+    $array = ["foo", []];
+    return implode(",", $array);
+}
+
+echo "Before\n";
+var_dump(test());
+echo "After\n";
+
+?>
+--EXPECTF--
+Before
+
+Warning: Array to string conversion in %s on line %d
+string(9) "foo,Array"
+After