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.

diff --git a/Zend/Optimizer/sccp.c b/Zend/Optimizer/sccp.c
index 490fbdf16b58bf96be67577ea13d8f2c1fe88a6a..e4b7531f46502aab6b739e37b19db0a3eb7bc960 100644 (file)
--- a/Zend/Optimizer/sccp.c
+++ b/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;

diff --git a/Zend/zend.c b/Zend/zend.c
index 45183226227bef1a439602f737624bca16c7b767..f8bca2cd6ba882a1f1c25f845884d3ccefb45cb4 100644 (file)
--- a/Zend/zend.c
+++ b/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;

diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h
index 73dac228cceb85358372126c90b69b929862e952..825fad833c6352b0036a7c07a736fdd4db164204 100644 (file)
--- a/Zend/zend_globals.h
+++ b/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;

diff --git a/ext/opcache/tests/opt/sccp_033.phpt b/ext/opcache/tests/opt/sccp_033.phpt
new file mode 100644 (file)
index 0000000..f3fbbb9
--- /dev/null
+++ b/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