Skip to content

Minor refactorings to zend_exceptions() #16684

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

Merged
merged 2 commits into from
Nov 10, 2024
Merged

Minor refactorings to zend_exceptions() #16684

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
148f98d
Minor refactorings to zend_exceptions()
Girgias Nov 2, 2024
90a806a
Address review comments
Girgias Nov 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions Zend/zend_exceptions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ static void _build_trace_args(zval *arg, smart_str *str) /* {{{ */
}
/* }}} */

static void _build_trace_string(smart_str *str, HashTable *ht, uint32_t num) /* {{{ */
static void _build_trace_string(smart_str *str, const HashTable *ht, uint32_t num) /* {{{ */
{
zval *file, *tmp;

Expand All @@ -539,16 +539,18 @@ static void _build_trace_string(smart_str *str, HashTable *ht, uint32_t num) /*

file = zend_hash_find_known_hash(ht, ZSTR_KNOWN(ZEND_STR_FILE));
if (file) {
if (Z_TYPE_P(file) != IS_STRING) {
if (UNEXPECTED(Z_TYPE_P(file) != IS_STRING)) {
/* This is a typed property and can only happen if modified via ArrayObject */
Copy link
Member

Choose a reason for hiding this comment

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

This is not true, see bug63762.phpt in Zend

zend_error(E_WARNING, "File name is not a string");
smart_str_appends(str, "[unknown file]: ");
} else{
zend_long line = 0;
tmp = zend_hash_find_known_hash(ht, ZSTR_KNOWN(ZEND_STR_LINE));
if (tmp) {
if (Z_TYPE_P(tmp) == IS_LONG) {
if (EXPECTED(Z_TYPE_P(tmp) == IS_LONG)) {
line = Z_LVAL_P(tmp);
} else {
/* This is a typed property and can only happen if modified via ArrayObject */
Copy link
Member

Choose a reason for hiding this comment

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

Likely not true as well due to similar reasons as bug63762.phpt in Zend

zend_error(E_WARNING, "Line is not an int");
}
}
Expand All @@ -566,7 +568,7 @@ static void _build_trace_string(smart_str *str, HashTable *ht, uint32_t num) /*
smart_str_appendc(str, '(');
tmp = zend_hash_find_known_hash(ht, ZSTR_KNOWN(ZEND_STR_ARGS));
if (tmp) {
if (Z_TYPE_P(tmp) == IS_ARRAY) {
if (EXPECTED(Z_TYPE_P(tmp) == IS_ARRAY)) {
size_t last_len = ZSTR_LEN(str->s);
zend_string *name;
zval *arg;
Expand All @@ -583,21 +585,22 @@ static void _build_trace_string(smart_str *str, HashTable *ht, uint32_t num) /*
ZSTR_LEN(str->s) -= 2; /* remove last ', ' */
}
} else {
/* The trace property is typed and private */
Copy link
Member

Choose a reason for hiding this comment

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

Check with bug63762.phpt in Zend

zend_error(E_WARNING, "args element is not an array");
}
}
smart_str_appends(str, ")\n");
}
/* }}} */

ZEND_API zend_string *zend_trace_to_string(HashTable *trace, bool include_main) {
ZEND_API zend_string *zend_trace_to_string(const HashTable *trace, bool include_main) {
zend_ulong index;
zval *frame;
uint32_t num = 0;
smart_str str = {0};

ZEND_HASH_FOREACH_NUM_KEY_VAL(trace, index, frame) {
if (Z_TYPE_P(frame) != IS_ARRAY) {
if (UNEXPECTED(Z_TYPE_P(frame) != IS_ARRAY)) {
zend_error(E_WARNING, "Expected array for frame " ZEND_ULONG_FMT, index);
continue;
}
Expand All @@ -624,7 +627,7 @@ ZEND_METHOD(Exception, getTraceAsString)
zval *object = ZEND_THIS;
zend_class_entry *base_ce = i_get_exception_base(Z_OBJ_P(object));
zval rv;
zval *trace = zend_read_property_ex(base_ce, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_TRACE), 1, &rv);
const zval *trace = zend_read_property_ex(base_ce, Z_OBJ_P(object), ZSTR_KNOWN(ZEND_STR_TRACE), 1, &rv);
if (EG(exception)) {
RETURN_THROWS();
}
Expand Down Expand Up @@ -859,14 +862,13 @@ ZEND_API ZEND_COLD zend_object *zend_throw_exception(zend_class_entry *exception
ZEND_API ZEND_COLD zend_object *zend_throw_exception_ex(zend_class_entry *exception_ce, zend_long code, const char *format, ...) /* {{{ */
{
va_list arg;
char *message;
zend_object *obj;

va_start(arg, format);
zend_vspprintf(&message, 0, format, arg);
zend_string *msg_str = zend_vstrpprintf(0, format, arg);
va_end(arg);
obj = zend_throw_exception(exception_ce, message, code);
efree(message);
obj = zend_throw_exception_zstr(exception_ce, msg_str, code);
zend_string_release(msg_str);
return obj;
}
/* }}} */
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_exceptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ extern ZEND_API void (*zend_throw_exception_hook)(zend_object *ex);
/* show an exception using zend_error(severity,...), severity should be E_ERROR */
ZEND_API ZEND_COLD zend_result zend_exception_error(zend_object *exception, int severity);
ZEND_NORETURN void zend_exception_uncaught_error(const char *prefix, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2);
ZEND_API zend_string *zend_trace_to_string(HashTable *trace, bool include_main);
ZEND_API zend_string *zend_trace_to_string(const HashTable *trace, bool include_main);

ZEND_API ZEND_COLD zend_object *zend_create_unwind_exit(void);
ZEND_API ZEND_COLD zend_object *zend_create_graceful_exit(void);
Expand Down