Skip to content

Commit 967fa4e

Browse files
committed
Aggressively use actual func params in verror
PHP errors used to not show parameter info consistently. Make it so that it uses a backtrace to get function info, similar to how exceptions work. This makes the docref error functions' parameter argument mostly vestigal, being used only if allocation fails basically. Several tests will fail from the fact we include function params. One annoyance is that _build_trace_args truncates strings according to exception_string_param_max_len. See GH-12048
1 parent 0b70abc commit 967fa4e

3 files changed

Lines changed: 61 additions & 1 deletion

File tree

Zend/zend_exceptions.c

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,39 @@ static void _build_trace_string(smart_str *str, const HashTable *ht, uint32_t nu
613613
}
614614
/* }}} */
615615

616+
ZEND_API zend_string *zend_trace_function_args_to_string(HashTable *frame) {
617+
zval *tmp;
618+
smart_str str = {0};
619+
/* init because ASan will complain */
620+
smart_str_appends(&str, "");
621+
622+
tmp = zend_hash_find_known_hash(frame, ZSTR_KNOWN(ZEND_STR_ARGS));
623+
if (tmp) {
624+
if (Z_TYPE_P(tmp) == IS_ARRAY) {
625+
size_t last_len = ZSTR_LEN(str.s);
626+
zend_string *name;
627+
zval *arg;
628+
629+
ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(tmp), name, arg) {
630+
if (name) {
631+
smart_str_append(&str, name);
632+
smart_str_appends(&str, ": ");
633+
}
634+
_build_trace_args(arg, &str);
635+
} ZEND_HASH_FOREACH_END();
636+
637+
if (last_len != ZSTR_LEN(str.s)) {
638+
ZSTR_LEN(str.s) -= 2; /* remove last ', ' */
639+
}
640+
} else {
641+
smart_str_appends(&str, "<<invalid argument array>>");
642+
}
643+
}
644+
645+
smart_str_0(&str);
646+
return str.s ? str.s : ZSTR_EMPTY_ALLOC();
647+
}
648+
616649
ZEND_API zend_string *zend_trace_to_string(const HashTable *trace, bool include_main) {
617650
zend_ulong index;
618651
zval *frame;

Zend/zend_exceptions.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ ZEND_API zend_result zend_update_exception_properties(zend_execute_data *execute
6666
/* show an exception using zend_error(severity,...), severity should be E_ERROR */
6767
ZEND_API ZEND_COLD zend_result zend_exception_error(zend_object *exception, int severity);
6868
ZEND_NORETURN void zend_exception_uncaught_error(const char *prefix, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2);
69+
ZEND_API zend_string *zend_trace_function_args_to_string(HashTable *frame);
6970
ZEND_API zend_string *zend_trace_to_string(const HashTable *trace, bool include_main);
7071

7172
ZEND_API ZEND_COLD zend_object *zend_create_unwind_exit(void);

main/main.c

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
#include "win32/php_registry.h"
6565
#include "ext/standard/flock_compat.h"
6666
#endif
67+
#include "Zend/zend_builtin_functions.h"
6768
#include "Zend/zend_exceptions.h"
6869

6970
#if PHP_SIGCHILD
@@ -1052,6 +1053,26 @@ static zend_string *escape_html(const char *buffer, size_t buffer_len) {
10521053
return result;
10531054
}
10541055

1056+
static zend_string *build_dynamic_parameters(void) {
1057+
zend_string *dynamic_params = NULL;
1058+
/* get a backtrace to snarf function args */
1059+
zval backtrace, *first_frame;
1060+
zend_fetch_debug_backtrace(&backtrace, /* skip_last */ 0, /* options */ 0, /* limit */ 1);
1061+
/* can fail esp if low memory condition */
1062+
if (Z_TYPE(backtrace) != IS_ARRAY) {
1063+
return NULL; /* don't need to free */
1064+
}
1065+
first_frame = zend_hash_index_find(Z_ARRVAL(backtrace), 0);
1066+
if (!first_frame) {
1067+
goto free_backtrace;
1068+
}
1069+
dynamic_params = zend_trace_function_args_to_string(Z_ARRVAL_P(first_frame));
1070+
free_backtrace:
1071+
zval_ptr_dtor(&backtrace);
1072+
/* free the string after we use it */
1073+
return dynamic_params;
1074+
}
1075+
10551076
/* {{{ php_verror */
10561077
/* php_verror is called from php_error_docref<n> functions.
10571078
* Its purpose is to unify error messages and automatically generate clickable
@@ -1134,7 +1155,12 @@ PHPAPI ZEND_COLD void php_verror(const char *docref, const char *params, int typ
11341155

11351156
/* if we still have memory then format the origin */
11361157
if (is_function) {
1137-
origin_len = spprintf(&origin, 0, "%s%s%s(%s)", class_name, space, function, params);
1158+
zend_string *dynamic_params = NULL;
1159+
dynamic_params = build_dynamic_parameters();
1160+
origin_len = spprintf(&origin, 0, "%s%s%s(%s)", class_name, space, function, dynamic_params ? ZSTR_VAL(dynamic_params) : params);
1161+
if (dynamic_params) {
1162+
zend_string_release(dynamic_params);
1163+
}
11381164
} else {
11391165
origin_len = strlen(function);
11401166
origin = estrndup(function, origin_len);

0 commit comments

Comments
 (0)