Skip to content

Use the same zend_arg_info struct for internal and user functions #19022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
+213 −229
Open

Use the same zend_arg_info struct for internal and user functions #19022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0731ca6
Use the same zend_arg_info struct for internal and user functions
arnaud-lb Jun 18, 2025
c79d568
Apply suggestions from code review
arnaud-lb Jul 3, 2025
a42cfc9
Remove unnecessary memcpy
arnaud-lb Jul 3, 2025
0dd97dc
Formatting
arnaud-lb Jul 3, 2025
31cd429
COPY
arnaud-lb Jul 3, 2025
9fba6b3
Fix signature
arnaud-lb Jul 3, 2025
9031567
1 -> true
arnaud-lb Jul 3, 2025
3e0ec86
Cleanup
arnaud-lb Jul 3, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Zend/zend.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
#include "zend_call_stack.h"
#include "zend_max_execution_timer.h"
#include "zend_hrtime.h"
#include "zend_enum.h"
#include "zend_closures.h"
#include "Optimizer/zend_optimizer.h"
#include "php.h"
#include "php_globals.h"
Expand Down Expand Up @@ -1077,6 +1079,9 @@ void zend_startup(zend_utility_functions *utility_functions) /* {{{ */
tsrm_set_new_thread_end_handler(zend_new_thread_end_handler);
tsrm_set_shutdown_handler(zend_interned_strings_dtor);
#endif

zend_enum_startup();
zend_closure_startup();
}
/* }}} */

Expand Down
194 changes: 104 additions & 90 deletions Zend/zend_API.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*/

#include "zend.h"
#include "zend_compile.h"
#include "zend_execute.h"
#include "zend_API.h"
#include "zend_hash.h"
Expand Down Expand Up @@ -2933,6 +2934,80 @@ static zend_always_inline void zend_normalize_internal_type(zend_type *type) {
} ZEND_TYPE_FOREACH_END();
}

void zend_convert_internal_arg_info_type(zend_type *type)
{
if (ZEND_TYPE_HAS_LITERAL_NAME(*type)) {
// gen_stubs.php does not support codegen for DNF types in arg infos.
// As a temporary workaround, we split the type name on `|` characters,
// converting it to an union type if necessary.
const char *class_name = ZEND_TYPE_LITERAL_NAME(*type);
type->type_mask &= ~_ZEND_TYPE_LITERAL_NAME_BIT;

size_t num_types = 1;
const char *p = class_name;
while ((p = strchr(p, '|'))) {
num_types++;
p++;
}

if (num_types == 1) {
/* Simple class type */
zend_string *str = zend_string_init_interned(class_name, strlen(class_name), true);
zend_alloc_ce_cache(str);
ZEND_TYPE_SET_PTR(*type, str);
type->type_mask |= _ZEND_TYPE_NAME_BIT;
} else {
/* Union type */
zend_type_list *list = malloc(ZEND_TYPE_LIST_SIZE(num_types));
list->num_types = num_types;
ZEND_TYPE_SET_LIST(*type, list);
ZEND_TYPE_FULL_MASK(*type) |= _ZEND_TYPE_UNION_BIT;

const char *start = class_name;
uint32_t j = 0;
while (true) {
const char *end = strchr(start, '|');
zend_string *str = zend_string_init_interned(start, end ? end - start : strlen(start), true);
zend_alloc_ce_cache(str);
list->types[j] = (zend_type) ZEND_TYPE_INIT_CLASS(str, 0, 0);
if (!end) {
break;
}
start = end + 1;
j++;
}
}
}
if (ZEND_TYPE_IS_ITERABLE_FALLBACK(*type)) {
/* Warning generated an extension load warning which is emitted for every test
zend_error(E_CORE_WARNING, "iterable type is now a compile time alias for array|Traversable,"
" regenerate the argument info via the php-src gen_stub build script");
*/
zend_type legacy_iterable = ZEND_TYPE_INIT_CLASS_MASK(
ZSTR_KNOWN(ZEND_STR_TRAVERSABLE),
(type->type_mask | MAY_BE_ARRAY)
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest reducing indentation to match the indentation of the start

Suggested change
);
);
*type = legacy_iterable;
}
}

void zend_convert_internal_arg_info(zend_arg_info *new_arg_info, const zend_internal_arg_info *arg_info, bool is_return_info)
{
if (!is_return_info) {
new_arg_info->name = zend_string_init_interned(arg_info->name, strlen(arg_info->name), true);
if (arg_info->default_value) {
new_arg_info->default_value = zend_string_init_interned(arg_info->default_value, strlen(arg_info->default_value), true);
} else {
new_arg_info->default_value = NULL;
}
} else {
new_arg_info->name = NULL;
new_arg_info->default_value = NULL;
}
new_arg_info->type = arg_info->type;
zend_convert_internal_arg_info_type(&new_arg_info->type);
}

/* registers all functions in *library_functions in the function hash */
ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend_function_entry *functions, HashTable *function_table, int type) /* {{{ */
{
Expand All @@ -2944,6 +3019,7 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend
int error_type;
zend_string *lowercase_name;
size_t fname_len;
const zend_internal_arg_info *internal_arg_info;

if (type==MODULE_PERSISTENT) {
error_type = E_CORE_WARNING;
Expand Down Expand Up @@ -3000,7 +3076,7 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend

if (ptr->arg_info) {
zend_internal_function_info *info = (zend_internal_function_info*)ptr->arg_info;
internal_function->arg_info = (zend_internal_arg_info*)ptr->arg_info+1;
internal_arg_info = ptr->arg_info+1;
internal_function->num_args = ptr->num_args;
/* Currently you cannot denote that the function can accept less arguments than num_args */
if (info->required_num_args == (uintptr_t)-1) {
Expand Down Expand Up @@ -3030,7 +3106,7 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend
zend_error(E_CORE_WARNING, "Missing arginfo for %s%s%s()",
scope ? ZSTR_VAL(scope->name) : "", scope ? "::" : "", ptr->fname);

internal_function->arg_info = NULL;
internal_arg_info = NULL;
internal_function->num_args = 0;
internal_function->required_num_args = 0;
}
Expand All @@ -3041,13 +3117,11 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend
!(internal_function->fn_flags & ZEND_ACC_HAS_RETURN_TYPE)) {
zend_error(E_CORE_WARNING, "%s::__toString() implemented without string return type",
ZSTR_VAL(scope->name));
internal_function->arg_info = (zend_internal_arg_info *) arg_info_toString + 1;
internal_arg_info = (zend_internal_arg_info*) arg_info_toString + 1;
internal_function->fn_flags |= ZEND_ACC_HAS_RETURN_TYPE;
internal_function->num_args = internal_function->required_num_args = 0;
}


zend_set_function_arg_flags((zend_function*)internal_function);
if (ptr->flags & ZEND_ACC_ABSTRACT) {
if (scope) {
/* This is a class that must be abstract itself. Here we set the check info. */
Expand Down Expand Up @@ -3112,17 +3186,17 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend
}

/* If types of arguments have to be checked */
if (reg_function->arg_info && num_args) {
if (internal_arg_info && num_args) {
uint32_t i;
for (i = 0; i < num_args; i++) {
zend_internal_arg_info *arg_info = &reg_function->arg_info[i];
const zend_internal_arg_info *arg_info = &internal_arg_info[i];
ZEND_ASSERT(arg_info->name && "Parameter must have a name");
if (ZEND_TYPE_IS_SET(arg_info->type)) {
reg_function->fn_flags |= ZEND_ACC_HAS_TYPE_HINTS;
}
#if ZEND_DEBUG
for (uint32_t j = 0; j < i; j++) {
if (!strcmp(arg_info->name, reg_function->arg_info[j].name)) {
if (!strcmp(arg_info->name, internal_arg_info[j].name)) {
zend_error_noreturn(E_CORE_ERROR,
"Duplicate parameter name $%s for function %s%s%s()", arg_info->name,
scope ? ZSTR_VAL(scope->name) : "", scope ? "::" : "", ptr->fname);
Expand All @@ -3132,78 +3206,23 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend
}
}

/* Rebuild arginfos if parameter/property types and/or a return type are used */
if (reg_function->arg_info &&
(reg_function->fn_flags & (ZEND_ACC_HAS_RETURN_TYPE|ZEND_ACC_HAS_TYPE_HINTS))) {
/* convert "const char*" class type names into "zend_string*" */
/* Convert zend_internal_arg_info to zend_arg_info */
if (internal_arg_info) {
uint32_t i;
zend_internal_arg_info *arg_info = reg_function->arg_info - 1;
zend_internal_arg_info *new_arg_info;
const zend_internal_arg_info *arg_info = internal_arg_info - 1;
zend_arg_info *new_arg_info;

/* Treat return type as an extra argument */
num_args++;
new_arg_info = malloc(sizeof(zend_internal_arg_info) * num_args);
memcpy(new_arg_info, arg_info, sizeof(zend_internal_arg_info) * num_args);
new_arg_info = malloc(sizeof(zend_arg_info) * num_args);
reg_function->arg_info = new_arg_info + 1;
for (i = 0; i < num_args; i++) {
if (ZEND_TYPE_HAS_LITERAL_NAME(new_arg_info[i].type)) {
// gen_stubs.php does not support codegen for DNF types in arg infos.
// As a temporary workaround, we split the type name on `|` characters,
// converting it to an union type if necessary.
const char *class_name = ZEND_TYPE_LITERAL_NAME(new_arg_info[i].type);
new_arg_info[i].type.type_mask &= ~_ZEND_TYPE_LITERAL_NAME_BIT;

size_t num_types = 1;
const char *p = class_name;
while ((p = strchr(p, '|'))) {
num_types++;
p++;
}

if (num_types == 1) {
/* Simple class type */
zend_string *str = zend_string_init_interned(class_name, strlen(class_name), 1);
zend_alloc_ce_cache(str);
ZEND_TYPE_SET_PTR(new_arg_info[i].type, str);
new_arg_info[i].type.type_mask |= _ZEND_TYPE_NAME_BIT;
} else {
/* Union type */
zend_type_list *list = malloc(ZEND_TYPE_LIST_SIZE(num_types));
list->num_types = num_types;
ZEND_TYPE_SET_LIST(new_arg_info[i].type, list);
ZEND_TYPE_FULL_MASK(new_arg_info[i].type) |= _ZEND_TYPE_UNION_BIT;

const char *start = class_name;
uint32_t j = 0;
while (true) {
const char *end = strchr(start, '|');
zend_string *str = zend_string_init_interned(start, end ? end - start : strlen(start), 1);
zend_alloc_ce_cache(str);
list->types[j] = (zend_type) ZEND_TYPE_INIT_CLASS(str, 0, 0);
if (!end) {
break;
}
start = end + 1;
j++;
}
}
}
if (ZEND_TYPE_IS_ITERABLE_FALLBACK(new_arg_info[i].type)) {
/* Warning generated an extension load warning which is emitted for every test
zend_error(E_CORE_WARNING, "iterable type is now a compile time alias for array|Traversable,"
" regenerate the argument info via the php-src gen_stub build script");
*/
zend_type legacy_iterable = ZEND_TYPE_INIT_CLASS_MASK(
ZSTR_KNOWN(ZEND_STR_TRAVERSABLE),
(new_arg_info[i].type.type_mask | MAY_BE_ARRAY)
);
new_arg_info[i].type = legacy_iterable;
}

zend_normalize_internal_type(&new_arg_info[i].type);
zend_convert_internal_arg_info(&new_arg_info[i], &arg_info[i], i == 0);
}
}

zend_set_function_arg_flags((zend_function*)reg_function);

if (scope) {
zend_check_magic_method_implementation(
scope, (zend_function *)reg_function, lowercase_name, E_CORE_ERROR);
Expand Down Expand Up @@ -5371,49 +5390,44 @@ static zend_string *try_parse_string(const char *str, size_t len, char quote) {
return zend_string_init(str, len, 0);
}

ZEND_API zend_result zend_get_default_from_internal_arg_info(
zval *default_value_zval, zend_internal_arg_info *arg_info)
ZEND_API zend_result zend_get_default_from_arg_info(
zval *default_value_zval, const zend_arg_info *arg_info)
{
const char *default_value = arg_info->default_value;
zend_string *default_value = arg_info->default_value;
if (!default_value) {
return FAILURE;
}

/* Avoid going through the full AST machinery for some simple and common cases. */
size_t default_value_len = strlen(default_value);
zend_ulong lval;
if (default_value_len == sizeof("null")-1
&& !memcmp(default_value, "null", sizeof("null")-1)) {
if (zend_string_equals_literal(default_value, "null")) {
ZVAL_NULL(default_value_zval);
return SUCCESS;
} else if (default_value_len == sizeof("true")-1
&& !memcmp(default_value, "true", sizeof("true")-1)) {
} else if (zend_string_equals_literal(default_value, "true")) {
ZVAL_TRUE(default_value_zval);
return SUCCESS;
} else if (default_value_len == sizeof("false")-1
&& !memcmp(default_value, "false", sizeof("false")-1)) {
} else if (zend_string_equals_literal(default_value, "false")) {
ZVAL_FALSE(default_value_zval);
return SUCCESS;
} else if (default_value_len >= 2
&& (default_value[0] == '\'' || default_value[0] == '"')
&& default_value[default_value_len - 1] == default_value[0]) {
} else if (ZSTR_LEN(default_value) >= 2
&& (ZSTR_VAL(default_value)[0] == '\'' || ZSTR_VAL(default_value)[0] == '"')
&& ZSTR_VAL(default_value)[ZSTR_LEN(default_value) - 1] == ZSTR_VAL(default_value)[0]) {
zend_string *str = try_parse_string(
default_value + 1, default_value_len - 2, default_value[0]);
ZSTR_VAL(default_value) + 1, ZSTR_LEN(default_value) - 2, ZSTR_VAL(default_value)[0]);
if (str) {
ZVAL_STR(default_value_zval, str);
return SUCCESS;
}
} else if (default_value_len == sizeof("[]")-1
&& !memcmp(default_value, "[]", sizeof("[]")-1)) {
} else if (zend_string_equals_literal(default_value, "[]")) {
ZVAL_EMPTY_ARRAY(default_value_zval);
return SUCCESS;
} else if (ZEND_HANDLE_NUMERIC_STR(default_value, default_value_len, lval)) {
} else if (ZEND_HANDLE_NUMERIC(default_value, lval)) {
ZVAL_LONG(default_value_zval, lval);
return SUCCESS;
}

#if 0
fprintf(stderr, "Evaluating %s via AST\n", default_value);
fprintf(stderr, "Evaluating %s via AST\n", ZSTR_VAL(default_value));
#endif
return get_default_via_ast(default_value_zval, default_value);
return get_default_via_ast(default_value_zval, ZSTR_VAL(default_value));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow-up, the get_default_via_ast() function should be changed to accept a zend_string to get rid of an unnecessary strlen() computation :)
}
7 changes: 5 additions & 2 deletions Zend/zend_API.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,8 +929,11 @@ ZEND_API bool zend_is_iterable(const zval *iterable);

ZEND_API bool zend_is_countable(const zval *countable);

ZEND_API zend_result zend_get_default_from_internal_arg_info(
zval *default_value_zval, zend_internal_arg_info *arg_info);
void zend_convert_internal_arg_info(zend_arg_info *new_arg_info,
const zend_internal_arg_info *arg_info, bool is_return_info);

ZEND_API zend_result zend_get_default_from_arg_info(
zval *default_value_zval, const zend_arg_info *arg_info);

END_EXTERN_C()

Expand Down
25 changes: 13 additions & 12 deletions Zend/zend_closures.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,6 @@ static HashTable *zend_closure_get_debug_info(zend_object *object, int *is_temp)
zval val;
struct _zend_arg_info *arg_info = closure->func.common.arg_info;
HashTable *debug_info;
bool zstr_args = (closure->func.type == ZEND_USER_FUNCTION) || (closure->func.common.fn_flags & ZEND_ACC_USER_ARG_INFO);

*is_temp = 1;

Expand Down Expand Up @@ -664,15 +663,9 @@ static HashTable *zend_closure_get_debug_info(zend_object *object, int *is_temp)
zend_string *name;
zval info;
ZEND_ASSERT(arg_info->name && "Argument should have name");
if (zstr_args) {
name = zend_strpprintf(0, "%s$%s",
ZEND_ARG_SEND_MODE(arg_info) ? "&" : "",
ZSTR_VAL(arg_info->name));
} else {
name = zend_strpprintf(0, "%s$%s",
ZEND_ARG_SEND_MODE(arg_info) ? "&" : "",
((zend_internal_arg_info*)arg_info)->name);
}
name = zend_strpprintf(0, "%s$%s",
ZEND_ARG_SEND_MODE(arg_info) ? "&" : "",
ZSTR_VAL(arg_info->name));
ZVAL_NEW_STR(&info, zend_strpprintf(0, "%s", i >= required ? "<optional>" : "<required>"));
zend_hash_update(Z_ARRVAL(val), name, &info);
zend_string_release_ex(name, 0);
Expand Down Expand Up @@ -852,8 +845,7 @@ ZEND_API void zend_create_fake_closure(zval *res, zend_function *func, zend_clas
}
/* }}} */

/* __call and __callStatic name the arguments "$arguments" in the docs. */
static zend_internal_arg_info trampoline_arg_info[] = {ZEND_ARG_VARIADIC_TYPE_INFO(false, arguments, IS_MIXED, false)};
static zend_arg_info trampoline_arg_info[1];

void zend_closure_from_frame(zval *return_value, zend_execute_data *call) { /* {{{ */
zval instance;
Expand Down Expand Up @@ -917,4 +909,13 @@ void zend_closure_bind_var_ex(zval *closure_zv, uint32_t offset, zval *val) /* {
zval_ptr_dtor(var);
ZVAL_COPY_VALUE(var, val);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated newline addition
/* }}} */

void zend_closure_startup(void)
{
/* __call and __callStatic name the arguments "$arguments" in the docs. */
trampoline_arg_info[0].name = zend_string_init_interned("arguments", strlen("arguments"), true);
trampoline_arg_info[0].type = (zend_type)ZEND_TYPE_INIT_CODE(IS_MIXED, false, _ZEND_ARG_INFO_FLAGS(false, 1, 0));
trampoline_arg_info[0].default_value = NULL;
}
1 change: 1 addition & 0 deletions Zend/zend_closures.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ BEGIN_EXTERN_C()
#define ZEND_CLOSURE_OBJECT(op_array) \
((zend_object*)((char*)(op_array) - sizeof(zend_object)))

void zend_closure_startup(void);
void zend_register_closure_ce(void);
void zend_closure_bind_var(zval *closure_zv, zend_string *var_name, zval *var);
void zend_closure_bind_var_ex(zval *closure_zv, uint32_t offset, zval *val);
Expand Down
Loading