Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Don't check for throwing calls in sccp function evaluation
We only need to reject functions that could warn (or have runtime dependent behavior). If a function can throw in some cases, just let it and discard the result.
- Loading branch information
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -781,253 +781,174 @@ static inline int ct_eval_array_key_exists(zval *result, zval *op1, zval *op2) { | |
| return SUCCESS; | ||
| } | ||
|
|
||
| static zend_bool can_ct_eval_func_call(zend_string *name, uint32_t num_args, zval **args) { | ||
| /* Functions that can be evaluated independently of what the arguments are. | ||
| * It's okay if these functions throw on invalid arguments, but they should not warn. */ | ||
| if (false | ||
| || 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_key_exists") | ||
| || zend_string_equals_literal(name, "array_keys") | ||
| || zend_string_equals_literal(name, "array_merge") | ||
| || zend_string_equals_literal(name, "array_merge_recursive") | ||
| || zend_string_equals_literal(name, "array_replace") | ||
| || zend_string_equals_literal(name, "array_replace_recursive") | ||
| || zend_string_equals_literal(name, "array_values") | ||
| || zend_string_equals_literal(name, "base64_decode") | ||
| || zend_string_equals_literal(name, "base64_encode") | ||
| || zend_string_equals_literal(name, "imagetypes") | ||
| || zend_string_equals_literal(name, "in_array") | ||
| || zend_string_equals_literal(name, "ltrim") | ||
| || zend_string_equals_literal(name, "php_sapi_name") | ||
| || zend_string_equals_literal(name, "php_uname") | ||
| || zend_string_equals_literal(name, "phpversion") | ||
| || zend_string_equals_literal(name, "pow") | ||
| || zend_string_equals_literal(name, "preg_quote") | ||
| || zend_string_equals_literal(name, "rawurldecode") | ||
| || zend_string_equals_literal(name, "rawurlencode") | ||
| || zend_string_equals_literal(name, "rtrim") | ||
| || zend_string_equals_literal(name, "serialize") | ||
| || zend_string_equals_literal(name, "str_contains") | ||
| || zend_string_equals_literal(name, "str_ends_with") | ||
| || zend_string_equals_literal(name, "str_split") | ||
| || zend_string_equals_literal(name, "str_split") | ||
| || zend_string_equals_literal(name, "str_starts_with") | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong. 
nikic
Author
Member
|
||
| || zend_string_equals_literal(name, "strpos") | ||
| || zend_string_equals_literal(name, "substr") | ||
| || zend_string_equals_literal(name, "trim") | ||
| || zend_string_equals_literal(name, "urldecode") | ||
| || zend_string_equals_literal(name, "urlencode") | ||
| || zend_string_equals_literal(name, "version_compare") | ||
| ) { | ||
| 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. */ | ||
| bool overflow; | ||
| return Z_TYPE_P(args[0]) == IS_STRING | ||
| && 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; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /* The functions chosen here are simple to implement and either likely to affect a branch, | ||
| * or just happened to be commonly used with constant operands in WP (need to test other | ||
| * applications as well, of course). */ | ||
| static inline int ct_eval_func_call( | ||
| zval *result, zend_string *name, uint32_t num_args, zval **args) { | ||
| uint32_t i; | ||
| zend_execute_data *execute_data, *prev_execute_data; | ||
| zend_function *func = zend_hash_find_ptr(CG(function_table), name); | ||
| if (!func || func->type != ZEND_INTERNAL_FUNCTION) { | ||
| return FAILURE; | ||
| } | ||
|
|
||
| if (num_args == 1) { | ||
| /* Handle a few functions for which we manually implement evaluation here. */ | ||
| if (zend_string_equals_literal(name, "chr")) { | ||
| zend_long c; | ||
| if (Z_TYPE_P(args[0]) != IS_LONG) { | ||
| return FAILURE; | ||
| } | ||
|
|
||
| c = Z_LVAL_P(args[0]) & 0xff; | ||
| ZVAL_CHAR(result, c); | ||
| return SUCCESS; | ||
| } else if (zend_string_equals_literal(name, "count")) { | ||
| if (Z_TYPE_P(args[0]) != IS_ARRAY) { | ||
| return FAILURE; | ||
| } | ||
|
|
||
| ZVAL_LONG(result, zend_hash_num_elements(Z_ARRVAL_P(args[0]))); | ||
| return SUCCESS; | ||
| } else if (zend_string_equals_literal(name, "ini_get")) { | ||
| zend_ini_entry *ini_entry; | ||
|
|
||
| if (Z_TYPE_P(args[0]) != IS_STRING) { | ||
| return FAILURE; | ||
| } | ||
|
|
||
| ini_entry = zend_hash_find_ptr(EG(ini_directives), Z_STR_P(args[0])); | ||
| if (!ini_entry) { | ||
| ZVAL_FALSE(result); | ||
| } else if (ini_entry->modifiable != ZEND_INI_SYSTEM) { | ||
| return FAILURE; | ||
| } else if (ini_entry->value) { | ||
| ZVAL_STR_COPY(result, ini_entry->value); | ||
| } else { | ||
| ZVAL_EMPTY_STRING(result); | ||
| } | ||
| return SUCCESS; | ||
| } | ||
| } | ||
|
|
||
| if (!can_ct_eval_func_call(name, num_args, args)) { | ||
| return FAILURE; | ||
| } | ||
|
|
||
|
|
||
@nikic - This changes the behavior of calls to internal functions from files with
declare(strict_types=1). Functions always behave like they're declared withstrict_types=0when they're called from php's C codefunc->internal_function.handler(execute_data, result);- I looked into that when working on another proposal - one way to set strict_types=1 would be to insert a dummy stack frame that has the same strict_types setting as the opcode being evaluated then pop it (internals of function handlers always check the frame to determine whether the call is strict)I also discovered a different pre-existing issue of different results with serialize() and opcache, but this may already be known and low importance - not optimizing values containing a float anywhere would fix that