Skip to content

Commit a22fa38

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 07cd468 commit a22fa38

File tree

3 files changed

+61
-1
lines changed

3 files changed

+61
-1
lines changed

Zend/zend_exceptions.c

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

593+
ZEND_API zend_string *zend_trace_function_args_to_string(HashTable *frame) {
594+
zval *tmp;
595+
smart_str str = {0};
596+
/* init because ASan will complain */
597+
smart_str_appends(&str, "");
598+
599+
tmp = zend_hash_find_known_hash(frame, ZSTR_KNOWN(ZEND_STR_ARGS));
600+
if (tmp) {
601+
if (Z_TYPE_P(tmp) == IS_ARRAY) {
602+
size_t last_len = ZSTR_LEN(str.s);
603+
zend_string *name;
604+
zval *arg;
605+
606+
ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(tmp), name, arg) {
607+
if (name) {
608+
smart_str_append(&str, name);
609+
smart_str_appends(&str, ": ");
610+
}
611+
_build_trace_args(arg, &str);
612+
} ZEND_HASH_FOREACH_END();
613+
614+
if (last_len != ZSTR_LEN(str.s)) {
615+
ZSTR_LEN(str.s) -= 2; /* remove last ', ' */
616+
}
617+
} else {
618+
smart_str_appends(&str, "<<invalid argument array>>");
619+
}
620+
}
621+
622+
smart_str_0(&str);
623+
return str.s ? str.s : ZSTR_EMPTY_ALLOC();
624+
}
625+
593626
ZEND_API zend_string *zend_trace_to_string(const HashTable *trace, bool include_main) {
594627
zend_ulong index;
595628
zval *frame;

Zend/zend_exceptions.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ extern ZEND_API void (*zend_throw_exception_hook)(zend_object *ex);
7272
/* show an exception using zend_error(severity,...), severity should be E_ERROR */
7373
ZEND_API ZEND_COLD zend_result zend_exception_error(zend_object *exception, int severity);
7474
ZEND_NORETURN void zend_exception_uncaught_error(const char *prefix, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2);
75+
ZEND_API zend_string *zend_trace_function_args_to_string(HashTable *frame);
7576
ZEND_API zend_string *zend_trace_to_string(const HashTable *trace, bool include_main);
7677

7778
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
@@ -59,6 +59,7 @@
5959
#include "ext/standard/flock_compat.h"
6060
#endif
6161
#include "php_syslog.h"
62+
#include "Zend/zend_builtin_functions.h"
6263
#include "Zend/zend_exceptions.h"
6364

6465
#if PHP_SIGCHILD
@@ -985,6 +986,26 @@ static zend_string *escape_html(const char *buffer, size_t buffer_len) {
985986
return result;
986987
}
987988

989+
static zend_string *build_dynamic_parameters(void) {
990+
zend_string *dynamic_params = NULL;
991+
/* get a backtrace to snarf function args */
992+
zval backtrace, *first_frame;
993+
zend_fetch_debug_backtrace(&backtrace, /* skip_last */ 0, /* options */ 0, /* limit */ 1);
994+
/* can fail esp if low memory condition */
995+
if (Z_TYPE(backtrace) != IS_ARRAY) {
996+
return NULL; /* don't need to free */
997+
}
998+
first_frame = zend_hash_index_find(Z_ARRVAL(backtrace), 0);
999+
if (!first_frame) {
1000+
goto free_backtrace;
1001+
}
1002+
dynamic_params = zend_trace_function_args_to_string(Z_ARRVAL_P(first_frame));
1003+
free_backtrace:
1004+
zval_ptr_dtor(&backtrace);
1005+
/* free the string after we use it */
1006+
return dynamic_params;
1007+
}
1008+
9881009
/* {{{ php_verror */
9891010
/* php_verror is called from php_error_docref<n> functions.
9901011
* Its purpose is to unify error messages and automatically generate clickable
@@ -1067,7 +1088,12 @@ PHPAPI ZEND_COLD void php_verror(const char *docref, const char *params, int typ
10671088

10681089
/* if we still have memory then format the origin */
10691090
if (is_function) {
1070-
origin_len = (int)spprintf(&origin, 0, "%s%s%s(%s)", class_name, space, function, params);
1091+
zend_string *dynamic_params = NULL;
1092+
dynamic_params = build_dynamic_parameters();
1093+
origin_len = (int)spprintf(&origin, 0, "%s%s%s(%s)", class_name, space, function, dynamic_params ? ZSTR_VAL(dynamic_params) : params);
1094+
if (dynamic_params) {
1095+
zend_string_release(dynamic_params);
1096+
}
10711097
} else {
10721098
origin_len = (int)spprintf(&origin, 0, "%s", function);
10731099
}

0 commit comments

Comments
 (0)