From b3dec4a81ca9195d12709d808e230465c97e0834 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 21 Apr 2025 15:40:44 +0200 Subject: [PATCH 1/2] Improve internal function argument parsing performance by reducing code bloat The goal of this PR is to reduce some of the code bloat induced by fast-ZPP. Reduced code bloat results in fewer cache misses (and better DSB coverage), and fewer instructions executed. If we take a look at a simple function: ```c PHP_FUNCTION(twice) { zend_long foo; ZEND_PARSE_PARAMETERS_START(1, 1) Z_PARAM_LONG(foo) ZEND_PARSE_PARAMETERS_END(); RETURN_LONG(foo * 2); } ``` We obtain the following assembly on x86-64 in the non-cold blocks: ```s <+0>: push %r12 <+2>: push %rbp <+3>: push %rbx <+4>: sub $0x10,%rsp <+8>: mov %fs:0x28,%r12 <+17>: mov %r12,0x8(%rsp) <+22>: mov 0x2c(%rdi),%r12d <+26>: cmp $0x1,%r12d <+30>: jne 0x22beaf <+36>: cmpb $0x4,0x58(%rdi) <+40>: mov %rsi,%rbp <+43>: jne 0x53c2f0 <+45>: mov 0x50(%rdi),%rax <+49>: add %rax,%rax <+52>: movl $0x4,0x8(%rbp) <+59>: mov %rax,0x0(%rbp) <+63>: mov 0x8(%rsp),%rax <+68>: sub %fs:0x28,%rax <+77>: jne 0x53c312 <+79>: add $0x10,%rsp <+83>: pop %rbx <+84>: pop %rbp <+85>: pop %r12 <+87>: ret <+88>: nopl 0x0(%rax,%rax,1) <+96>: lea 0x50(%rdi),%rbx <+100>: mov %rsp,%rsi <+103>: mov $0x1,%edx <+108>: mov %rbx,%rdi <+111>: call 0x620240 <+116>: test %al,%al <+118>: je 0x22be96 <+124>: mov (%rsp),%rax <+128>: jmp 0x53c2c1 <+130>: call 0x201050 <__stack_chk_fail@plt> ``` Notice how we get the stack protector overhead in this function and also have to reload the parsed value on the slow path. This happens because the parsed value is returned via a pointer. If instead we were to return struct with a value pair (similar to optional in C++ / Option in Rust), then the values are returned via registers. This means that we no longer have stack protector overhead and we also don't need to reload a value, resulting in better register usage. This is the resulting assembly for the sample function after this patch: ```s <+0>: push %r12 <+2>: push %rbp <+3>: push %rbx <+4>: mov 0x2c(%rdi),%r12d <+8>: cmp $0x1,%r12d <+12>: jne 0x22d482 <+18>: cmpb $0x4,0x58(%rdi) <+22>: mov %rsi,%rbp <+25>: jne 0x53be08 <+27>: mov 0x50(%rdi),%rax <+31>: add %rax,%rax <+34>: movl $0x4,0x8(%rbp) <+41>: mov %rax,0x0(%rbp) <+45>: pop %rbx <+46>: pop %rbp <+47>: pop %r12 <+49>: ret <+50>: nopw 0x0(%rax,%rax,1) <+56>: lea 0x50(%rdi),%rbx <+60>: mov $0x1,%esi <+65>: mov %rbx,%rdi <+68>: call 0x61e7b0 <+73>: test %dl,%dl <+75>: je 0x22d46a <+81>: jmp 0x53bdef ``` The following uses the default benchmark programs we use in CI. Each program is ran on php-cgi with the appropriate `-T` argument, then repeated 15 times. It shows a small performance improvement on Symfony both with and without JIT, and a small improvement on WordPress with JIT. For WordPress, the difference is small as my CPU is bottlenecked on some other stuff as well. | Test | Old Mean | Old Stddev | New Mean | New Stddev | |---------------------------------|----------|------------|----------|------------| | Symfony, no JIT (-T10,50) | 0.5324 | 0.0050 | 0.5272 | 0.0042 | | Symfony, tracing JIT (-T10,50) | 0.5301 | 0.0029 | 0.5264 | 0.0036 | | WordPress, no JIT (-T5,25) | 0.7408 | 0.0049 | 0.7404 | 0.0048 | | WordPress, tracing JIT (-T5,25) | 0.6814 | 0.0052 | 0.6770 | 0.0055 | I was not able to measure any meaningful difference for our micro benchmarks `Zend/bench.php` and `Zend/micro_bench.php`. The Valgrind instruction counts also show a decrease: -0.19% on Symfony without JIT, and -0.14% on WordPress without JIT (see CI). --- UPGRADING.INTERNALS | 2 + Zend/zend_API.c | 174 +++++++++++++++++---------------- Zend/zend_API.h | 111 ++++++++++++++++----- Zend/zend_execute.c | 49 +++++----- Zend/zend_frameless_function.h | 2 +- Zend/zend_vm_def.h | 8 +- Zend/zend_vm_execute.h | 12 ++- ext/bcmath/bcmath.c | 10 +- ext/gmp/gmp.c | 6 +- ext/mbstring/mbstring.c | 2 +- ext/openssl/openssl.c | 2 +- ext/pgsql/pgsql.c | 4 +- ext/session/session.c | 2 +- ext/standard/string.c | 4 +- 14 files changed, 237 insertions(+), 151 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 7c7f093e50cce..5cef5bb6f5c7e 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -27,6 +27,8 @@ PHP 8.5 INTERNALS UPGRADE NOTES runtime. . Removed the cache_slot argument of zend_check_user_type_slow() because now it only relies on the CE cache. + . Changed several zend_parse_arg_*_{weak,slow} functions to no longer have an + output pointer argument, but instead return a struct or a sentinel value. ======================== 2. Build system changes diff --git a/Zend/zend_API.c b/Zend/zend_API.c index e0006e7d7275f..7b1f06ba327d5 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -523,45 +523,44 @@ static ZEND_COLD bool zend_null_arg_deprecated(const char *fallback_type, uint32 return !EG(exception); } -ZEND_API bool ZEND_FASTCALL zend_parse_arg_bool_weak(const zval *arg, bool *dest, uint32_t arg_num) /* {{{ */ +ZEND_API zend_opt_bool ZEND_FASTCALL zend_parse_arg_bool_weak(const zval *arg, uint32_t arg_num) /* {{{ */ { if (EXPECTED(Z_TYPE_P(arg) <= IS_STRING)) { if (UNEXPECTED(Z_TYPE_P(arg) == IS_NULL) && !zend_null_arg_deprecated("bool", arg_num)) { - return 0; + return (zend_opt_bool){false, false}; } - *dest = zend_is_true(arg); + return (zend_opt_bool){zend_is_true(arg), true}; } else { - return 0; + return (zend_opt_bool){false, false}; } - return 1; } /* }}} */ -ZEND_API bool ZEND_FASTCALL zend_parse_arg_bool_slow(const zval *arg, bool *dest, uint32_t arg_num) /* {{{ */ +ZEND_API zend_opt_bool ZEND_FASTCALL zend_parse_arg_bool_slow(const zval *arg, uint32_t arg_num) /* {{{ */ { if (UNEXPECTED(ZEND_ARG_USES_STRICT_TYPES())) { - return 0; + return (zend_opt_bool){false, false}; } - return zend_parse_arg_bool_weak(arg, dest, arg_num); + return zend_parse_arg_bool_weak(arg, arg_num); } /* }}} */ -ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_bool_slow(const zval *arg, bool *dest, uint32_t arg_num) +ZEND_API zend_opt_bool ZEND_FASTCALL zend_flf_parse_arg_bool_slow(const zval *arg, uint32_t arg_num) { if (UNEXPECTED(ZEND_FLF_ARG_USES_STRICT_TYPES())) { - return 0; + return (zend_opt_bool){false, false}; } - return zend_parse_arg_bool_weak(arg, dest, arg_num); + return zend_parse_arg_bool_weak(arg, arg_num); } -ZEND_API bool ZEND_FASTCALL zend_parse_arg_long_weak(const zval *arg, zend_long *dest, uint32_t arg_num) /* {{{ */ +ZEND_API zend_opt_long ZEND_FASTCALL zend_parse_arg_long_weak(const zval *arg, uint32_t arg_num) /* {{{ */ { if (EXPECTED(Z_TYPE_P(arg) == IS_DOUBLE)) { if (UNEXPECTED(zend_isnan(Z_DVAL_P(arg)))) { - return 0; + goto fail; } if (UNEXPECTED(!ZEND_DOUBLE_FITS_LONG(Z_DVAL_P(arg)))) { - return 0; + goto fail; } else { zend_long lval = zend_dval_to_lval(Z_DVAL_P(arg)); if (UNEXPECTED(!zend_is_long_compatible(Z_DVAL_P(arg), lval))) { @@ -569,25 +568,25 @@ ZEND_API bool ZEND_FASTCALL zend_parse_arg_long_weak(const zval *arg, zend_long * zend_verify_weak_scalar_type_hint_no_sideeffect() */ if (arg_num != (uint32_t)-1) { zend_incompatible_double_to_long_error(Z_DVAL_P(arg)); - } - if (UNEXPECTED(EG(exception))) { - return 0; + if (UNEXPECTED(EG(exception))) { + goto fail; + } } } - *dest = lval; + return (zend_opt_long){lval, true}; } } else if (EXPECTED(Z_TYPE_P(arg) == IS_STRING)) { double d; uint8_t type; + zend_long lval; - if (UNEXPECTED((type = is_numeric_str_function(Z_STR_P(arg), dest, &d)) != IS_LONG)) { + if (UNEXPECTED((type = is_numeric_str_function(Z_STR_P(arg), &lval, &d)) != IS_LONG)) { if (EXPECTED(type != 0)) { - zend_long lval; if (UNEXPECTED(zend_isnan(d))) { - return 0; + goto fail; } if (UNEXPECTED(!ZEND_DOUBLE_FITS_LONG(d))) { - return 0; + goto fail; } lval = zend_dval_to_lval(d); @@ -597,95 +596,101 @@ ZEND_API bool ZEND_FASTCALL zend_parse_arg_long_weak(const zval *arg, zend_long * zend_verify_weak_scalar_type_hint_no_sideeffect() */ if (arg_num != (uint32_t)-1) { zend_incompatible_string_to_long_error(Z_STR_P(arg)); - } - if (UNEXPECTED(EG(exception))) { - return 0; + if (UNEXPECTED(EG(exception))) { + goto fail; + } } } - *dest = lval; } else { - return 0; + goto fail; } } - if (UNEXPECTED(EG(exception))) { - return 0; - } + return (zend_opt_long){lval, true}; } else if (EXPECTED(Z_TYPE_P(arg) < IS_TRUE)) { if (UNEXPECTED(Z_TYPE_P(arg) == IS_NULL) && !zend_null_arg_deprecated("int", arg_num)) { - return 0; + goto fail; } - *dest = 0; + return (zend_opt_long){0, true}; } else if (EXPECTED(Z_TYPE_P(arg) == IS_TRUE)) { - *dest = 1; - } else { - return 0; + return (zend_opt_long){1, true}; } - return 1; + +fail:; + zend_opt_long result; + result.has_value = false; + return result; } /* }}} */ -ZEND_API bool ZEND_FASTCALL zend_parse_arg_long_slow(const zval *arg, zend_long *dest, uint32_t arg_num) /* {{{ */ +ZEND_API zend_opt_long ZEND_FASTCALL zend_parse_arg_long_slow(const zval *arg, uint32_t arg_num) /* {{{ */ { if (UNEXPECTED(ZEND_ARG_USES_STRICT_TYPES())) { - return 0; + zend_opt_long result; + result.has_value = false; + return result; } - return zend_parse_arg_long_weak(arg, dest, arg_num); + return zend_parse_arg_long_weak(arg, arg_num); } /* }}} */ -ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_long_slow(const zval *arg, zend_long *dest, uint32_t arg_num) +ZEND_API zend_opt_long ZEND_FASTCALL zend_flf_parse_arg_long_slow(const zval *arg, uint32_t arg_num) { if (UNEXPECTED(ZEND_FLF_ARG_USES_STRICT_TYPES())) { - return 0; + zend_opt_long result; + result.has_value = false; + return result; } - return zend_parse_arg_long_weak(arg, dest, arg_num); + return zend_parse_arg_long_weak(arg, arg_num); } -ZEND_API bool ZEND_FASTCALL zend_parse_arg_double_weak(const zval *arg, double *dest, uint32_t arg_num) /* {{{ */ +ZEND_API zend_opt_double ZEND_FASTCALL zend_parse_arg_double_weak(const zval *arg, uint32_t arg_num) /* {{{ */ { if (EXPECTED(Z_TYPE_P(arg) == IS_LONG)) { - *dest = (double)Z_LVAL_P(arg); + return (zend_opt_double){(double)Z_LVAL_P(arg), true}; } else if (EXPECTED(Z_TYPE_P(arg) == IS_STRING)) { zend_long l; uint8_t type; + double d; - if (UNEXPECTED((type = is_numeric_str_function(Z_STR_P(arg), &l, dest)) != IS_DOUBLE)) { + if (UNEXPECTED((type = is_numeric_str_function(Z_STR_P(arg), &l, &d)) != IS_DOUBLE)) { if (EXPECTED(type != 0)) { - *dest = (double)(l); - } else { - return 0; + return (zend_opt_double){(double)l, true}; } - } - if (UNEXPECTED(EG(exception))) { - return 0; + goto fail; + } else { + return (zend_opt_double){d, true}; } } else if (EXPECTED(Z_TYPE_P(arg) < IS_TRUE)) { if (UNEXPECTED(Z_TYPE_P(arg) == IS_NULL) && !zend_null_arg_deprecated("float", arg_num)) { - return 0; + goto fail; } - *dest = 0.0; + return (zend_opt_double){0.0, true}; } else if (EXPECTED(Z_TYPE_P(arg) == IS_TRUE)) { - *dest = 1.0; - } else { - return 0; + return (zend_opt_double){1.0, true}; } - return 1; + +fail:; + zend_opt_double result; + result.has_value = false; + return result; } /* }}} */ -ZEND_API bool ZEND_FASTCALL zend_parse_arg_double_slow(const zval *arg, double *dest, uint32_t arg_num) /* {{{ */ +ZEND_API zend_opt_double ZEND_FASTCALL zend_parse_arg_double_slow(const zval *arg, uint32_t arg_num) /* {{{ */ { if (EXPECTED(Z_TYPE_P(arg) == IS_LONG)) { /* SSTH Exception: IS_LONG may be accepted instead as IS_DOUBLE */ - *dest = (double)Z_LVAL_P(arg); + return (zend_opt_double){(double)Z_LVAL_P(arg), true}; } else if (UNEXPECTED(ZEND_ARG_USES_STRICT_TYPES())) { - return 0; + zend_opt_double result; + result.has_value = false; + return result; } - return zend_parse_arg_double_weak(arg, dest, arg_num); + return zend_parse_arg_double_weak(arg, arg_num); } /* }}} */ -ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_slow(zval *arg, zval **dest, uint32_t arg_num) /* {{{ */ +ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_slow(zval *arg, uint32_t arg_num) /* {{{ */ { if (UNEXPECTED(ZEND_ARG_USES_STRICT_TYPES())) { return 0; @@ -713,13 +718,12 @@ ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_slow(zval *arg, zval **dest, u } else { return 0; } - *dest = arg; return 1; } /* }}} */ -ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_or_str_slow(zval *arg, zval **dest, uint32_t arg_num) /* {{{ */ +ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_or_str_slow(zval *arg, uint32_t arg_num) /* {{{ */ { if (UNEXPECTED(ZEND_ARG_USES_STRICT_TYPES())) { return false; @@ -737,57 +741,53 @@ ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_or_str_slow(zval *arg, zval ** if (zobj->handlers->cast_object(zobj, &obj, IS_STRING) == SUCCESS) { OBJ_RELEASE(zobj); ZVAL_COPY_VALUE(arg, &obj); - *dest = arg; return true; } return false; } else { return false; } - *dest = arg; return true; } -ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_weak(zval *arg, zend_string **dest, uint32_t arg_num) /* {{{ */ +ZEND_API zend_string * ZEND_FASTCALL zend_parse_arg_str_weak(zval *arg, uint32_t arg_num) /* {{{ */ { if (EXPECTED(Z_TYPE_P(arg) < IS_STRING)) { if (UNEXPECTED(Z_TYPE_P(arg) == IS_NULL) && !zend_null_arg_deprecated("string", arg_num)) { - return 0; + return NULL; } convert_to_string(arg); - *dest = Z_STR_P(arg); + return Z_STR_P(arg); } else if (UNEXPECTED(Z_TYPE_P(arg) == IS_OBJECT)) { zend_object *zobj = Z_OBJ_P(arg); zval obj; if (zobj->handlers->cast_object(zobj, &obj, IS_STRING) == SUCCESS) { OBJ_RELEASE(zobj); ZVAL_COPY_VALUE(arg, &obj); - *dest = Z_STR_P(arg); - return 1; + return Z_STR_P(arg); } - return 0; + return NULL; } else { - return 0; + return NULL; } - return 1; } /* }}} */ -ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_slow(zval *arg, zend_string **dest, uint32_t arg_num) /* {{{ */ +ZEND_API zend_string * ZEND_FASTCALL zend_parse_arg_str_slow(zval *arg, uint32_t arg_num) /* {{{ */ { if (UNEXPECTED(ZEND_ARG_USES_STRICT_TYPES())) { - return 0; + return NULL; } - return zend_parse_arg_str_weak(arg, dest, arg_num); + return zend_parse_arg_str_weak(arg, arg_num); } /* }}} */ -ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_str_slow(zval *arg, zend_string **dest, uint32_t arg_num) +ZEND_API zend_string * ZEND_FASTCALL zend_flf_parse_arg_str_slow(zval *arg, uint32_t arg_num) { if (UNEXPECTED(ZEND_FLF_ARG_USES_STRICT_TYPES())) { return 0; } - return zend_parse_arg_str_weak(arg, dest, arg_num); + return zend_parse_arg_str_weak(arg, arg_num); } ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_or_long_slow(zval *arg, zend_string **dest_str, zend_long *dest_long, uint32_t arg_num) /* {{{ */ @@ -795,14 +795,20 @@ ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_or_long_slow(zval *arg, zend_stri if (UNEXPECTED(ZEND_ARG_USES_STRICT_TYPES())) { return 0; } - if (zend_parse_arg_long_weak(arg, dest_long, arg_num)) { + zend_opt_long result = zend_parse_arg_long_weak(arg, arg_num); + if (result.has_value) { + *dest_long = result.value; *dest_str = NULL; return 1; - } else if (zend_parse_arg_str_weak(arg, dest_str, arg_num)) { - *dest_long = 0; - return 1; } else { - return 0; + zend_string *str = zend_parse_arg_str_weak(arg, arg_num); + if (str) { + *dest_long = 0; + *dest_str = str; + return 1; + } else { + return 0; + } } } /* }}} */ diff --git a/Zend/zend_API.h b/Zend/zend_API.h index a644de8e15134..95455d855ed7f 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -2174,22 +2174,37 @@ ZEND_API ZEND_COLD void zend_class_redeclaration_error_ex(int type, zend_string /* Inlined implementations shared by new and old parameter parsing APIs */ +typedef struct { + zend_long value; + bool has_value; +} zend_opt_long; + +typedef struct { + bool value; + bool has_value; +} zend_opt_bool; + +typedef struct { + double value; + bool has_value; +} zend_opt_double; + ZEND_API bool ZEND_FASTCALL zend_parse_arg_class(zval *arg, zend_class_entry **pce, uint32_t num, bool check_null); -ZEND_API bool ZEND_FASTCALL zend_parse_arg_bool_slow(const zval *arg, bool *dest, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_parse_arg_bool_weak(const zval *arg, bool *dest, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_parse_arg_long_slow(const zval *arg, zend_long *dest, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_parse_arg_long_weak(const zval *arg, zend_long *dest, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_parse_arg_double_slow(const zval *arg, double *dest, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_parse_arg_double_weak(const zval *arg, double *dest, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_slow(zval *arg, zend_string **dest, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_weak(zval *arg, zend_string **dest, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_slow(zval *arg, zval **dest, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_or_str_slow(zval *arg, zval **dest, uint32_t arg_num); +ZEND_API zend_opt_bool ZEND_FASTCALL zend_parse_arg_bool_slow(const zval *arg, uint32_t arg_num); +ZEND_API zend_opt_bool ZEND_FASTCALL zend_parse_arg_bool_weak(const zval *arg, uint32_t arg_num); +ZEND_API zend_opt_long ZEND_FASTCALL zend_parse_arg_long_slow(const zval *arg, uint32_t arg_num); +ZEND_API zend_opt_long ZEND_FASTCALL zend_parse_arg_long_weak(const zval *arg, uint32_t arg_num); +ZEND_API zend_opt_double ZEND_FASTCALL zend_parse_arg_double_slow(const zval *arg, uint32_t arg_num); +ZEND_API zend_opt_double ZEND_FASTCALL zend_parse_arg_double_weak(const zval *arg, uint32_t arg_num); +ZEND_API zend_string * ZEND_FASTCALL zend_parse_arg_str_slow(zval *arg, uint32_t arg_num); +ZEND_API zend_string * ZEND_FASTCALL zend_parse_arg_str_weak(zval *arg, uint32_t arg_num); +ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_slow(zval *arg, uint32_t arg_num); +ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_or_str_slow(zval *arg, uint32_t arg_num); ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_or_long_slow(zval *arg, zend_string **dest_str, zend_long *dest_long, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_bool_slow(const zval *arg, bool *dest, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_str_slow(zval *arg, zend_string **dest, uint32_t arg_num); -ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_long_slow(const zval *arg, zend_long *dest, uint32_t arg_num); +ZEND_API zend_opt_bool ZEND_FASTCALL zend_flf_parse_arg_bool_slow(const zval *arg, uint32_t arg_num); +ZEND_API zend_string * ZEND_FASTCALL zend_flf_parse_arg_str_slow(zval *arg, uint32_t arg_num); +ZEND_API zend_opt_long ZEND_FASTCALL zend_flf_parse_arg_long_slow(const zval *arg, uint32_t arg_num); static zend_always_inline bool zend_parse_arg_bool_ex(const zval *arg, bool *dest, bool *is_null, bool check_null, uint32_t arg_num, bool frameless) { @@ -2204,10 +2219,16 @@ static zend_always_inline bool zend_parse_arg_bool_ex(const zval *arg, bool *des *is_null = 1; *dest = 0; } else { + zend_opt_bool result; if (frameless) { - return zend_flf_parse_arg_bool_slow(arg, dest, arg_num); + result = zend_flf_parse_arg_bool_slow(arg, arg_num); } else { - return zend_parse_arg_bool_slow(arg, dest, arg_num); + result = zend_parse_arg_bool_slow(arg, arg_num); + } + if (result.has_value) { + *dest = result.value; + } else { + return 0; } } return 1; @@ -2229,10 +2250,16 @@ static zend_always_inline bool zend_parse_arg_long_ex(zval *arg, zend_long *dest *is_null = 1; *dest = 0; } else { + zend_opt_long result; if (frameless) { - return zend_flf_parse_arg_long_slow(arg, dest, arg_num); + result = zend_flf_parse_arg_long_slow(arg, arg_num); } else { - return zend_parse_arg_long_slow(arg, dest, arg_num); + result = zend_parse_arg_long_slow(arg, arg_num); + } + if (result.has_value) { + *dest = result.value; + } else { + return 0; } } return 1; @@ -2254,7 +2281,12 @@ static zend_always_inline bool zend_parse_arg_double(const zval *arg, double *de *is_null = 1; *dest = 0.0; } else { - return zend_parse_arg_double_slow(arg, dest, arg_num); + zend_opt_double result = zend_parse_arg_double_slow(arg, arg_num); + if (result.has_value) { + *dest = result.value; + } else { + return 0; + } } return 1; } @@ -2266,7 +2298,11 @@ static zend_always_inline bool zend_parse_arg_number(zval *arg, zval **dest, boo } else if (check_null && EXPECTED(Z_TYPE_P(arg) == IS_NULL)) { *dest = NULL; } else { - return zend_parse_arg_number_slow(arg, dest, arg_num); + if (zend_parse_arg_number_slow(arg, arg_num)) { + *dest = arg; + } else { + return 0; + } } return 1; } @@ -2278,7 +2314,11 @@ static zend_always_inline bool zend_parse_arg_number_or_str(zval *arg, zval **de } else if (check_null && EXPECTED(Z_TYPE_P(arg) == IS_NULL)) { *dest = NULL; } else { - return zend_parse_arg_number_or_str_slow(arg, dest, arg_num); + if (zend_parse_arg_number_or_str_slow(arg, arg_num)) { + *dest = arg; + } else { + return false; + } } return true; } @@ -2290,10 +2330,16 @@ static zend_always_inline bool zend_parse_arg_str_ex(zval *arg, zend_string **de } else if (check_null && Z_TYPE_P(arg) == IS_NULL) { *dest = NULL; } else { + zend_string *str; if (frameless) { - return zend_flf_parse_arg_str_slow(arg, dest, arg_num); + str = zend_flf_parse_arg_str_slow(arg, arg_num); } else { - return zend_parse_arg_str_slow(arg, dest, arg_num); + str = zend_parse_arg_str_slow(arg, arg_num); + } + if (str) { + *dest = str; + } else { + return 0; } } return 1; @@ -2415,7 +2461,12 @@ static zend_always_inline bool zend_parse_arg_array_ht_or_long( *is_null = 1; } else { *dest_ht = NULL; - return zend_parse_arg_long_slow(arg, dest_long, arg_num); + zend_opt_long result = zend_parse_arg_long_slow(arg, arg_num); + if (result.has_value) { + *dest_long = result.value; + } else { + return 0; + } } return 1; @@ -2464,7 +2515,12 @@ static zend_always_inline bool zend_parse_arg_obj_or_long( *is_null = 1; } else { *dest_obj = NULL; - return zend_parse_arg_long_slow(arg, dest_long, arg_num); + zend_opt_long result = zend_parse_arg_long_slow(arg, arg_num); + if (result.has_value) { + *dest_long = result.value; + } else { + return 0; + } } return 1; @@ -2527,7 +2583,12 @@ static zend_always_inline bool zend_parse_arg_array_ht_or_str( *dest_str = NULL; } else { *dest_ht = NULL; - return zend_parse_arg_str_slow(arg, dest_str, arg_num); + zend_string *str = zend_parse_arg_str_slow(arg, arg_num); + if (str) { + *dest_str = str; + } else { + return 0; + } } return 1; } diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 89fbcf2cbd781..c80bd7324411a 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -725,8 +725,6 @@ static bool zend_verify_weak_scalar_type_hint(uint32_t type_mask, zval *arg) { zend_long lval; double dval; - zend_string *str; - bool bval; /* Type preference order: int -> float -> string -> bool */ if (type_mask & MAY_BE_LONG) { @@ -744,27 +742,36 @@ static bool zend_verify_weak_scalar_type_hint(uint32_t type_mask, zval *arg) ZVAL_DOUBLE(arg, dval); return 1; } - } else if (zend_parse_arg_long_weak(arg, &lval, 0)) { + } else { + zend_opt_long result = zend_parse_arg_long_weak(arg, 0); + if (result.has_value) { + zval_ptr_dtor(arg); + ZVAL_LONG(arg, result.value); + return 1; + } else if (UNEXPECTED(EG(exception))) { + return 0; + } + } + } + if ((type_mask & MAY_BE_DOUBLE)) { + zend_opt_double result = zend_parse_arg_double_weak(arg, 0); + if (result.has_value) { zval_ptr_dtor(arg); - ZVAL_LONG(arg, lval); + ZVAL_DOUBLE(arg, result.value); return 1; - } else if (UNEXPECTED(EG(exception))) { - return 0; } } - if ((type_mask & MAY_BE_DOUBLE) && zend_parse_arg_double_weak(arg, &dval, 0)) { - zval_ptr_dtor(arg); - ZVAL_DOUBLE(arg, dval); - return 1; - } - if ((type_mask & MAY_BE_STRING) && zend_parse_arg_str_weak(arg, &str, 0)) { + if ((type_mask & MAY_BE_STRING) && zend_parse_arg_str_weak(arg, 0)) { /* on success "arg" is converted to IS_STRING */ return 1; } - if ((type_mask & MAY_BE_BOOL) == MAY_BE_BOOL && zend_parse_arg_bool_weak(arg, &bval, 0)) { - zval_ptr_dtor(arg); - ZVAL_BOOL(arg, bval); - return 1; + if ((type_mask & MAY_BE_BOOL) == MAY_BE_BOOL) { + zend_opt_bool result = zend_parse_arg_bool_weak(arg, 0); + if (result.has_value) { + zval_ptr_dtor(arg); + ZVAL_BOOL(arg, result.value); + return 1; + } } return 0; } @@ -784,22 +791,18 @@ static bool can_convert_to_string(const zval *zv) { /* Used to sanity-check internal arginfo types without performing any actual type conversions. */ static bool zend_verify_weak_scalar_type_hint_no_sideeffect(uint32_t type_mask, const zval *arg) { - zend_long lval; - double dval; - bool bval; - /* Pass (uint32_t)-1 as arg_num to indicate to ZPP not to emit any deprecation notice, * this is needed because the version with side effects also uses 0 (e.g. for typed properties) */ - if ((type_mask & MAY_BE_LONG) && zend_parse_arg_long_weak(arg, &lval, (uint32_t)-1)) { + if ((type_mask & MAY_BE_LONG) && zend_parse_arg_long_weak(arg, (uint32_t)-1).has_value) { return 1; } - if ((type_mask & MAY_BE_DOUBLE) && zend_parse_arg_double_weak(arg, &dval, (uint32_t)-1)) { + if ((type_mask & MAY_BE_DOUBLE) && zend_parse_arg_double_weak(arg, (uint32_t)-1).has_value) { return 1; } if ((type_mask & MAY_BE_STRING) && can_convert_to_string(arg)) { return 1; } - if ((type_mask & MAY_BE_BOOL) == MAY_BE_BOOL && zend_parse_arg_bool_weak(arg, &bval, (uint32_t)-1)) { + if ((type_mask & MAY_BE_BOOL) == MAY_BE_BOOL && zend_parse_arg_bool_weak(arg, (uint32_t)-1).has_value) { return 1; } return 0; diff --git a/Zend/zend_frameless_function.h b/Zend/zend_frameless_function.h index d64ca7ee15e2f..6f2a987e88195 100644 --- a/Zend/zend_frameless_function.h +++ b/Zend/zend_frameless_function.h @@ -65,7 +65,7 @@ dest_ht = NULL; \ ZVAL_COPY(&str_tmp, arg ## arg_num); \ arg ## arg_num = &str_tmp; \ - if (!zend_flf_parse_arg_str_slow(arg ## arg_num, &dest_str, arg_num)) { \ + if (!(dest_str = zend_flf_parse_arg_str_slow(arg ## arg_num, arg_num))) { \ zend_wrong_parameter_type_error(arg_num, Z_EXPECTED_ARRAY_OR_STRING, arg ## arg_num); \ goto flf_clean; \ } \ diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 7b861688c9661..c05b00e3884e6 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -5501,18 +5501,22 @@ ZEND_VM_C_LABEL(send_array): uint32_t skip = opline->extended_value; uint32_t count = zend_hash_num_elements(ht); zend_long len; + zend_opt_long result; if (EXPECTED(Z_TYPE_P(op2) == IS_LONG)) { len = Z_LVAL_P(op2); } else if (Z_TYPE_P(op2) == IS_NULL) { len = count - skip; } else if (EX_USES_STRICT_TYPES() - || !zend_parse_arg_long_weak(op2, &len, /* arg_num */ 3)) { + || !(result = zend_parse_arg_long_weak(op2, /* arg_num */ 3)).has_value) { zend_type_error( "array_slice(): Argument #3 ($length) must be of type ?int, %s given", zend_zval_value_name(op2)); FREE_OP2(); FREE_OP1(); HANDLE_EXCEPTION(); + } else { + ZEND_ASSERT(result.has_value); + len = result.value; } if (len < 0) { @@ -8718,7 +8722,7 @@ ZEND_VM_COLD_CONST_HANDLER(121, ZEND_STRLEN, CONST|TMPVAR|CV, ANY) } ZVAL_COPY(&tmp, value); - if (zend_parse_arg_str_weak(&tmp, &str, 1)) { + if ((str = zend_parse_arg_str_weak(&tmp, 1))) { ZVAL_LONG(EX_VAR(opline->result.var), ZSTR_LEN(str)); zval_ptr_dtor(&tmp); break; diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 2211f6753485d..2e05fa7ebe20d 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -2627,18 +2627,22 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_SEND_ARRAY_SPEC_HANDLER(ZEND_O uint32_t skip = opline->extended_value; uint32_t count = zend_hash_num_elements(ht); zend_long len; + zend_opt_long result; if (EXPECTED(Z_TYPE_P(op2) == IS_LONG)) { len = Z_LVAL_P(op2); } else if (Z_TYPE_P(op2) == IS_NULL) { len = count - skip; } else if (EX_USES_STRICT_TYPES() - || !zend_parse_arg_long_weak(op2, &len, /* arg_num */ 3)) { + || !(result = zend_parse_arg_long_weak(op2, /* arg_num */ 3)).has_value) { zend_type_error( "array_slice(): Argument #3 ($length) must be of type ?int, %s given", zend_zval_value_name(op2)); FREE_OP(opline->op2_type, opline->op2.var); FREE_OP(opline->op1_type, opline->op1.var); HANDLE_EXCEPTION(); + } else { + ZEND_ASSERT(result.has_value); + len = result.value; } if (len < 0) { @@ -5967,7 +5971,7 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_STRLEN_SPEC_CONST } ZVAL_COPY(&tmp, value); - if (zend_parse_arg_str_weak(&tmp, &str, 1)) { + if ((str = zend_parse_arg_str_weak(&tmp, 1))) { ZVAL_LONG(EX_VAR(opline->result.var), ZSTR_LEN(str)); zval_ptr_dtor(&tmp); break; @@ -15576,7 +15580,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_STRLEN_SPEC_TMPVAR_HANDLER(ZEN } ZVAL_COPY(&tmp, value); - if (zend_parse_arg_str_weak(&tmp, &str, 1)) { + if ((str = zend_parse_arg_str_weak(&tmp, 1))) { ZVAL_LONG(EX_VAR(opline->result.var), ZSTR_LEN(str)); zval_ptr_dtor(&tmp); break; @@ -41544,7 +41548,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_STRLEN_SPEC_CV_HANDLER(ZEND_OP } ZVAL_COPY(&tmp, value); - if (zend_parse_arg_str_weak(&tmp, &str, 1)) { + if ((str = zend_parse_arg_str_weak(&tmp, 1))) { ZVAL_LONG(EX_VAR(opline->result.var), ZSTR_LEN(str)); zval_ptr_dtor(&tmp); break; diff --git a/ext/bcmath/bcmath.c b/ext/bcmath/bcmath.c index 1e0e6d14d81eb..b0a8b481e7bd7 100644 --- a/ext/bcmath/bcmath.c +++ b/ext/bcmath/bcmath.c @@ -1183,8 +1183,14 @@ static zend_result bcmath_number_parse_num(const zval *zv, zend_object **obj, ze *lval = 0; return FAILURE; - default: - return zend_parse_arg_long_slow(zv, lval, 1 /* dummy */) ? SUCCESS : FAILURE; + default: { + zend_opt_long result = zend_parse_arg_long_slow(zv, 1 /* dummy */); + if (result.has_value) { + *lval = result.value; + return SUCCESS; + } + return FAILURE; + } } } } diff --git a/ext/gmp/gmp.c b/ext/gmp/gmp.c index 8cf20c90fc7a2..098583e5283b1 100644 --- a/ext/gmp/gmp.c +++ b/ext/gmp/gmp.c @@ -137,15 +137,15 @@ static bool gmp_zend_parse_arg_into_mpz_ex( * but operator overloading with objects should behave as if a * method was called, thus strict types should apply. */ if (!ZEND_ARG_USES_STRICT_TYPES()) { - zend_long lval = 0; if (is_operator && Z_TYPE_P(arg) == IS_NULL) { return false; } - if (!zend_parse_arg_long_weak(arg, &lval, arg_num)) { + zend_opt_long result = zend_parse_arg_long_weak(arg, arg_num); + if (!result.has_value) { return false; } - mpz_set_si(*destination_mpz_ptr, lval); + mpz_set_si(*destination_mpz_ptr, result.value); return true; } diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c index 7f0cbaeb346ee..7be8dd09f8779 100644 --- a/ext/mbstring/mbstring.c +++ b/ext/mbstring/mbstring.c @@ -2317,7 +2317,7 @@ PHP_FUNCTION(mb_substr_count) PHP_FUNCTION(mb_substr) { zend_string *str, *encoding = NULL; - zend_long from, len; + zend_long from, len = 0; size_t real_from, real_len; bool len_is_null = true; diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index b17159d46aabc..4d1a426ccc265 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -2790,7 +2790,7 @@ PHP_FUNCTION(openssl_pkcs7_sign) X509 *cert = NULL; zend_object *cert_obj; zend_string *cert_str; - zval *zprivkey, * zheaders; + zval *zprivkey, * zheaders = NULL; zval * hval; EVP_PKEY * privkey = NULL; zend_long flags = PKCS7_DETACHED; diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index b86909d3aa3ee..9495136e56866 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -1900,7 +1900,7 @@ PHP_FUNCTION(pg_fetch_result) { zval *result; zend_string *field_name; - zend_long row, field_offset = 0; + zend_long row = 0, field_offset = 0; bool row_is_null = false; PGresult *pgsql_result; pgsql_result_handle *pg_result; @@ -2236,7 +2236,7 @@ static void php_pgsql_data_info(INTERNAL_FUNCTION_PARAMETERS, int entry_type, bo { zval *result; zend_string *field_name; - zend_long row, field_offset = 0; + zend_long row = 0, field_offset = 0; bool row_is_null = false; PGresult *pgsql_result; pgsql_result_handle *pg_result; diff --git a/ext/session/session.c b/ext/session/session.c index fd877483c0a6f..ef3c5d850245c 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -1727,7 +1727,7 @@ PHPAPI void session_adapt_url(const char *url, size_t url_len, char **new_url, s PHP_FUNCTION(session_set_cookie_params) { HashTable *options_ht; - zend_long lifetime_long; + zend_long lifetime_long = 0; zend_string *lifetime = NULL, *path = NULL, *domain = NULL, *samesite = NULL; bool secure = 0, secure_null = 1; bool httponly = 0, httponly_null = 1; diff --git a/ext/standard/string.c b/ext/standard/string.c index 1e20791eb61ce..b91b5e0ba03b5 100644 --- a/ext/standard/string.c +++ b/ext/standard/string.c @@ -2313,9 +2313,9 @@ PHP_FUNCTION(substr_replace) zend_string *str, *repl_str; HashTable *str_ht, *repl_ht; HashTable *from_ht; - zend_long from_long; + zend_long from_long = 0; HashTable *len_ht = NULL; - zend_long len_long; + zend_long len_long = 0; bool len_is_null = 1; zend_long l = 0; zend_long f; From a1c97a8b458ffff3f39df5a25d265cedde74cd05 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 27 Apr 2025 11:40:46 +0200 Subject: [PATCH 2/2] Fix false positive warning --- ext/date/php_date.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/date/php_date.c b/ext/date/php_date.c index f1ea3d3a01d12..7a6899fb42f84 100644 --- a/ext/date/php_date.c +++ b/ext/date/php_date.c @@ -5438,7 +5438,7 @@ PHP_FUNCTION(date_default_timezone_get) */ static void php_do_date_sunrise_sunset(INTERNAL_FUNCTION_PARAMETERS, bool calc_sunset) { - double latitude, longitude, zenith, gmt_offset, altitude; + double latitude, longitude, zenith, gmt_offset = 0, altitude; bool latitude_is_null = 1, longitude_is_null = 1, zenith_is_null = 1, gmt_offset_is_null = 1; double h_rise, h_set, N; timelib_sll rise, set, transit;