Skip to content

Commit d6b038d

Browse files
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 Co-authored-by: Tim Düsterhus <tim@bastelstu.be>
1 parent 69e023f commit d6b038d

3 files changed

Lines changed: 69 additions & 28 deletions

File tree

Zend/zend_exceptions.c

Lines changed: 60 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ ZEND_METHOD(ErrorException, getSeverity)
506506
} \
507507
} while (0)
508508

509-
static void _build_trace_args(zval *arg, smart_str *str) /* {{{ */
509+
static void build_trace_args(zval *arg, smart_str *str) /* {{{ */
510510
{
511511
/* the trivial way would be to do
512512
* convert_to_string(arg);
@@ -516,24 +516,21 @@ static void _build_trace_args(zval *arg, smart_str *str) /* {{{ */
516516

517517
ZVAL_DEREF(arg);
518518

519-
if (smart_str_append_zval(str, arg, EG(exception_string_param_max_len)) == SUCCESS) {
520-
smart_str_appends(str, ", ");
521-
} else {
519+
if (smart_str_append_zval(str, arg, EG(exception_string_param_max_len)) != SUCCESS) {
522520
switch (Z_TYPE_P(arg)) {
523521
case IS_RESOURCE:
524522
smart_str_appends(str, "Resource id #");
525523
smart_str_append_long(str, Z_RES_HANDLE_P(arg));
526-
smart_str_appends(str, ", ");
527524
break;
528525
case IS_ARRAY:
529-
smart_str_appends(str, "Array, ");
526+
smart_str_appends(str, "Array");
530527
break;
531528
case IS_OBJECT: {
532529
zend_string *class_name = Z_OBJ_HANDLER_P(arg, get_class_name)(Z_OBJ_P(arg));
533530
smart_str_appends(str, "Object(");
534531
/* cut off on NULL byte ... class@anonymous */
535532
smart_str_appends(str, ZSTR_VAL(class_name));
536-
smart_str_appends(str, "), ");
533+
smart_str_appends(str, ")");
537534
zend_string_release_ex(class_name, 0);
538535
break;
539536
}
@@ -542,7 +539,30 @@ static void _build_trace_args(zval *arg, smart_str *str) /* {{{ */
542539
}
543540
/* }}} */
544541

545-
static void _build_trace_string(smart_str *str, const HashTable *ht, uint32_t num) /* {{{ */
542+
static void build_trace_args_list(zval *tmp, smart_str *str) /* {{{ */
543+
{
544+
if (UNEXPECTED(Z_TYPE_P(tmp) != IS_ARRAY)) {
545+
/* only happens w/ reflection abuse (Zend/tests/bug63762.phpt) */
546+
zend_error(E_WARNING, "args element is not an array");
547+
return;
548+
}
549+
550+
bool first = true;
551+
ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(tmp), zend_string *name, zval *arg) {
552+
if (!first) {
553+
smart_str_appends(str, ", ");
554+
}
555+
first = false;
556+
if (name) {
557+
smart_str_append(str, name);
558+
smart_str_appends(str, ": ");
559+
}
560+
build_trace_args(arg, str);
561+
} ZEND_HASH_FOREACH_END();
562+
}
563+
/* }}} */
564+
565+
static void build_trace_string(smart_str *str, const HashTable *ht, uint32_t num) /* {{{ */
546566
{
547567
zval *file, *tmp;
548568

@@ -588,27 +608,40 @@ static void _build_trace_string(smart_str *str, const HashTable *ht, uint32_t nu
588608
smart_str_appendc(str, '(');
589609
tmp = zend_hash_find_known_hash(ht, ZSTR_KNOWN(ZEND_STR_ARGS));
590610
if (tmp) {
591-
if (EXPECTED(Z_TYPE_P(tmp) == IS_ARRAY)) {
592-
size_t last_len = ZSTR_LEN(str->s);
593-
zend_string *name;
594-
zval *arg;
595-
596-
ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(tmp), name, arg) {
597-
if (name) {
598-
smart_str_append(str, name);
599-
smart_str_appends(str, ": ");
600-
}
601-
_build_trace_args(arg, str);
602-
} ZEND_HASH_FOREACH_END();
611+
build_trace_args_list(tmp, str);
612+
}
613+
smart_str_appends(str, ")\n");
614+
}
615+
/* }}} */
603616

604-
if (last_len != ZSTR_LEN(str->s)) {
605-
ZSTR_LEN(str->s) -= 2; /* remove last ', ' */
606-
}
607-
} else {
608-
zend_error(E_WARNING, "args element is not an array");
617+
/* {{{ Gets the function arguments printed as a string from a backtrace frame. */
618+
ZEND_API zend_string *zend_trace_function_args_to_string(const HashTable *frame) {
619+
smart_str str = {0};
620+
621+
zval *tmp = zend_hash_find_known_hash(frame, ZSTR_KNOWN(ZEND_STR_ARGS));
622+
if (tmp) {
623+
build_trace_args_list(tmp, &str);
624+
}
625+
626+
return smart_str_extract(&str);
627+
}
628+
/* }}} */
629+
630+
/* {{{ Gets the currently executing function's arguments as a string. Used by php_verror. */
631+
ZEND_API zend_string *zend_trace_current_function_args_string(void) {
632+
zend_string *dynamic_params = NULL;
633+
/* get a backtrace to snarf function args */
634+
zval backtrace;
635+
zend_fetch_debug_backtrace(&backtrace, /* skip_last */ 0, /* options */ 0, /* limit */ 1);
636+
/* can fail esp if low memory condition */
637+
if (Z_TYPE(backtrace) == IS_ARRAY) {
638+
zval *first_frame = zend_hash_index_find(Z_ARRVAL(backtrace), 0);
639+
if (first_frame) {
640+
dynamic_params = zend_trace_function_args_to_string(Z_ARRVAL_P(first_frame));
609641
}
610642
}
611-
smart_str_appends(str, ")\n");
643+
zval_ptr_dtor(&backtrace);
644+
return dynamic_params;
612645
}
613646
/* }}} */
614647

@@ -624,7 +657,7 @@ ZEND_API zend_string *zend_trace_to_string(const HashTable *trace, bool include_
624657
continue;
625658
}
626659

627-
_build_trace_string(&str, Z_ARRVAL_P(frame), num++);
660+
build_trace_string(&str, Z_ARRVAL_P(frame), num++);
628661
} ZEND_HASH_FOREACH_END();
629662

630663
if (include_main) {

Zend/zend_exceptions.h

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

7072
ZEND_API ZEND_COLD zend_object *zend_create_unwind_exit(void);

main/main.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
#include "win32/php_registry.h"
6363
#include "ext/standard/flock_compat.h"
6464
#endif
65+
#include "Zend/zend_builtin_functions.h"
6566
#include "Zend/zend_exceptions.h"
6667

6768
#if PHP_SIGCHILD
@@ -1132,7 +1133,12 @@ PHPAPI ZEND_COLD void php_verror(const char *docref, const char *params, int typ
11321133

11331134
/* if we still have memory then format the origin */
11341135
if (is_function) {
1135-
origin_len = spprintf(&origin, 0, "%s%s%s(%s)", class_name, space, function, params);
1136+
zend_string *dynamic_params = NULL;
1137+
dynamic_params = zend_trace_current_function_args_string();
1138+
origin_len = spprintf(&origin, 0, "%s%s%s(%s)", class_name, space, function, dynamic_params ? ZSTR_VAL(dynamic_params) : params);
1139+
if (dynamic_params) {
1140+
zend_string_release(dynamic_params);
1141+
}
11361142
} else {
11371143
origin_len = strlen(function);
11381144
origin = estrndup(function, origin_len);

0 commit comments

Comments
 (0)