Skip to content

Commit 975acfe

Browse files
nikicnikic
committed
Pass zend_string message to zend_error_cb
This makes the zend_error_cb API simpler, and avoid formatting the same message in multiple places. It should be noted that the passed zend_string is always non-persistent, so if you want to store it persistently somewhere, you may still need to duplicate it. The last_error_message is cleared a bit more aggressive, to make sure it doesn't hang around across allocator life-cycles. Closes GH-5639.
1 parent e949f30 commit 975acfe

File tree

10 files changed

+119
-162
lines changed

10 files changed

+119
-162
lines changed

‎Zend/zend.c‎

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ ZEND_API FILE *(*zend_fopen)(const char *filename, zend_string **opened_path);
7474
ZEND_API int (*zend_stream_open_function)(const char *filename, zend_file_handle *handle);
7575
ZEND_API void (*zend_ticks_function)(int ticks);
7676
ZEND_API void (*zend_interrupt_function)(zend_execute_data *execute_data);
77-
ZEND_API void (*zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args);
77+
ZEND_API void (*zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, zend_string *message);
7878
void (*zend_printf_to_smart_string)(smart_string *buf, const char *format, va_list ap);
7979
void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap);
8080
ZEND_API char *(*zend_getenv)(char *name, size_t name_len);
@@ -1259,11 +1259,9 @@ ZEND_API zval *zend_get_configuration_directive(zend_string *name) /* {{{ */
12591259
} \
12601260
} while (0)
12611261

1262-
static ZEND_COLD void zend_error_va_list(
1263-
int orig_type, const char *error_filename, uint32_t error_lineno,
1264-
const char *format, va_list args)
1262+
static ZEND_COLD void zend_error_impl(
1263+
int orig_type, const char *error_filename, uint32_t error_lineno, zend_string *message)
12651264
{
1266-
va_list usr_copy;
12671265
zval params[4];
12681266
zval retval;
12691267
zval orig_user_error_handler;
@@ -1308,18 +1306,15 @@ static ZEND_COLD void zend_error_va_list(
13081306

13091307
#ifdef HAVE_DTRACE
13101308
if (DTRACE_ERROR_ENABLED()) {
1311-
char *dtrace_error_buffer;
1312-
zend_vspprintf(&dtrace_error_buffer, 0, format, args);
1313-
DTRACE_ERROR(dtrace_error_buffer, (char *)error_filename, error_lineno);
1314-
efree(dtrace_error_buffer);
1309+
DTRACE_ERROR(ZSTR_VAL(message), (char *)error_filename, error_lineno);
13151310
}
13161311
#endif /* HAVE_DTRACE */
13171312

13181313
/* if we don't have a user defined error handler */
13191314
if (Z_TYPE(EG(user_error_handler)) == IS_UNDEF
13201315
|| !(EG(user_error_handler_error_reporting) & type)
13211316
|| EG(error_handling) != EH_NORMAL) {
1322-
zend_error_cb(orig_type, error_filename, error_lineno, format, args);
1317+
zend_error_cb(orig_type, error_filename, error_lineno, message);
13231318
} else switch (type) {
13241319
case E_ERROR:
13251320
case E_PARSE:
@@ -1328,14 +1323,11 @@ static ZEND_COLD void zend_error_va_list(
13281323
case E_COMPILE_ERROR:
13291324
case E_COMPILE_WARNING:
13301325
/* The error may not be safe to handle in user-space */
1331-
zend_error_cb(orig_type, error_filename, error_lineno, format, args);
1326+
zend_error_cb(orig_type, error_filename, error_lineno, message);
13321327
break;
13331328
default:
13341329
/* Handle the error in user space */
1335-
va_copy(usr_copy, args);
1336-
ZVAL_STR(&params[1], zend_vstrpprintf(0, format, usr_copy));
1337-
va_end(usr_copy);
1338-
1330+
ZVAL_STR_COPY(&params[1], message);
13391331
ZVAL_LONG(&params[0], type);
13401332

13411333
if (error_filename) {
@@ -1369,13 +1361,13 @@ static ZEND_COLD void zend_error_va_list(
13691361
if (call_user_function(CG(function_table), NULL, &orig_user_error_handler, &retval, 4, params) == SUCCESS) {
13701362
if (Z_TYPE(retval) != IS_UNDEF) {
13711363
if (Z_TYPE(retval) == IS_FALSE) {
1372-
zend_error_cb(orig_type, error_filename, error_lineno, format, args);
1364+
zend_error_cb(orig_type, error_filename, error_lineno, message);
13731365
}
13741366
zval_ptr_dtor(&retval);
13751367
}
13761368
} else if (!EG(exception)) {
13771369
/* The user error handler failed, use built-in error handler */
1378-
zend_error_cb(orig_type, error_filename, error_lineno, format, args);
1370+
zend_error_cb(orig_type, error_filename, error_lineno, message);
13791371
}
13801372

13811373
EG(fake_scope) = orig_fake_scope;
@@ -1411,6 +1403,15 @@ static ZEND_COLD void zend_error_va_list(
14111403
}
14121404
/* }}} */
14131405

1406+
static ZEND_COLD void zend_error_va_list(
1407+
int orig_type, const char *error_filename, uint32_t error_lineno,
1408+
const char *format, va_list args)
1409+
{
1410+
zend_string *message = zend_vstrpprintf(0, format, args);
1411+
zend_error_impl(orig_type, error_filename, error_lineno, message);
1412+
zend_string_release(message);
1413+
}
1414+
14141415
static ZEND_COLD void get_filename_lineno(int type, const char **filename, uint32_t *lineno) {
14151416
/* Obtain relevant filename and lineno */
14161417
switch (type) {
@@ -1499,7 +1500,6 @@ ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_at_noreturn(
14991500
/* Should never reach this. */
15001501
abort();
15011502
}
1502-
/* }}} */
15031503

15041504
ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn(int type, const char *format, ...)
15051505
{
@@ -1514,7 +1514,13 @@ ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn(int type, const char *
15141514
/* Should never reach this. */
15151515
abort();
15161516
}
1517-
/* }}} */
1517+
1518+
ZEND_API ZEND_COLD void zend_error_zstr(int type, zend_string *message) {
1519+
const char *filename;
1520+
uint32_t lineno;
1521+
get_filename_lineno(type, &filename, &lineno);
1522+
zend_error_impl(type, filename, lineno, message);
1523+
}
15181524

15191525
ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) /* {{{ */
15201526
{

‎Zend/zend.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ struct _zend_class_entry {
187187
};
188188

189189
typedef struct _zend_utility_functions {
190-
void (*error_function)(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args) ZEND_ATTRIBUTE_PTR_FORMAT(printf, 4, 0);
190+
void (*error_function)(int type, const char *error_filename, const uint32_t error_lineno, zend_string *message);
191191
size_t (*printf_function)(const char *format, ...) ZEND_ATTRIBUTE_PTR_FORMAT(printf, 1, 2);
192192
size_t (*write_function)(const char *str, size_t str_length);
193193
FILE *(*fopen_function)(const char *filename, zend_string **opened_path);
@@ -281,7 +281,7 @@ extern ZEND_API zend_write_func_t zend_write;
281281
extern ZEND_API FILE *(*zend_fopen)(const char *filename, zend_string **opened_path);
282282
extern ZEND_API void (*zend_ticks_function)(int ticks);
283283
extern ZEND_API void (*zend_interrupt_function)(zend_execute_data *execute_data);
284-
extern ZEND_API void (*zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args) ZEND_ATTRIBUTE_PTR_FORMAT(printf, 4, 0);
284+
extern ZEND_API void (*zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, zend_string *message);
285285
extern ZEND_API void (*zend_on_timeout)(int seconds);
286286
extern ZEND_API int (*zend_stream_open_function)(const char *filename, zend_file_handle *handle);
287287
extern void (*zend_printf_to_smart_string)(smart_string *buf, const char *format, va_list ap);
@@ -301,6 +301,7 @@ ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn(int type, const char *
301301
/* If filename is NULL the default filename is used. */
302302
ZEND_API ZEND_COLD void zend_error_at(int type, const char *filename, uint32_t lineno, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 4, 5);
303303
ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_at_noreturn(int type, const char *filename, uint32_t lineno, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 4, 5);
304+
ZEND_API ZEND_COLD void zend_error_zstr(int type, zend_string *message);
304305

305306
ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
306307
ZEND_API ZEND_COLD void zend_type_error(const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2);

‎Zend/zend_exceptions.c‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -820,7 +820,7 @@ ZEND_API zend_class_entry *zend_get_error_exception(void)
820820
}
821821
/* }}} */
822822

823-
ZEND_API ZEND_COLD zend_object *zend_throw_exception(zend_class_entry *exception_ce, const char *message, zend_long code) /* {{{ */
823+
static zend_object *zend_throw_exception_zstr(zend_class_entry *exception_ce, zend_string *message, zend_long code) /* {{{ */
824824
{
825825
zval ex, tmp;
826826

@@ -836,9 +836,8 @@ ZEND_API ZEND_COLD zend_object *zend_throw_exception(zend_class_entry *exception
836836

837837

838838
if (message) {
839-
ZVAL_STRING(&tmp, message);
839+
ZVAL_STR(&tmp, message);
840840
zend_update_property_ex(exception_ce, &ex, ZSTR_KNOWN(ZEND_STR_MESSAGE), &tmp);
841-
zval_ptr_dtor(&tmp);
842841
}
843842
if (code) {
844843
ZVAL_LONG(&tmp, code);
@@ -850,6 +849,15 @@ ZEND_API ZEND_COLD zend_object *zend_throw_exception(zend_class_entry *exception
850849
}
851850
/* }}} */
852851

852+
ZEND_API ZEND_COLD zend_object *zend_throw_exception(zend_class_entry *exception_ce, const char *message, zend_long code) /* {{{ */
853+
{
854+
zend_string *msg_str = zend_string_init(message, strlen(message), 0);
855+
zend_object *ex = zend_throw_exception_zstr(exception_ce, msg_str, code);
856+
zend_string_release(msg_str);
857+
return ex;
858+
}
859+
/* }}} */
860+
853861
ZEND_API ZEND_COLD zend_object *zend_throw_exception_ex(zend_class_entry *exception_ce, zend_long code, const char *format, ...) /* {{{ */
854862
{
855863
va_list arg;
@@ -865,10 +873,10 @@ ZEND_API ZEND_COLD zend_object *zend_throw_exception_ex(zend_class_entry *except
865873
}
866874
/* }}} */
867875

868-
ZEND_API ZEND_COLD zend_object *zend_throw_error_exception(zend_class_entry *exception_ce, const char *message, zend_long code, int severity) /* {{{ */
876+
ZEND_API ZEND_COLD zend_object *zend_throw_error_exception(zend_class_entry *exception_ce, zend_string *message, zend_long code, int severity) /* {{{ */
869877
{
870878
zval ex, tmp;
871-
zend_object *obj = zend_throw_exception(exception_ce, message, code);
879+
zend_object *obj = zend_throw_exception_zstr(exception_ce, message, code);
872880
ZVAL_OBJ(&ex, obj);
873881
ZVAL_LONG(&tmp, severity);
874882
zend_update_property_ex(zend_ce_error_exception, &ex, ZSTR_KNOWN(ZEND_STR_SEVERITY), &tmp);
@@ -879,23 +887,14 @@ ZEND_API ZEND_COLD zend_object *zend_throw_error_exception(zend_class_entry *exc
879887
static void zend_error_va(int type, const char *file, uint32_t lineno, const char *format, ...) /* {{{ */
880888
{
881889
va_list args;
882-
883890
va_start(args, format);
884-
zend_error_cb(type, file, lineno, format, args);
891+
zend_string *message = zend_vstrpprintf(0, format, args);
892+
zend_error_cb(type, file, lineno, message);
893+
zend_string_release(message);
885894
va_end(args);
886895
}
887896
/* }}} */
888897

889-
static void zend_error_helper(int type, const char *filename, const uint32_t lineno, const char *format, ...) /* {{{ */
890-
{
891-
va_list va;
892-
893-
va_start(va, format);
894-
zend_error_cb(type, filename, lineno, format, va);
895-
va_end(va);
896-
}
897-
/* }}} */
898-
899898
/* This function doesn't return if it uses E_ERROR */
900899
ZEND_API ZEND_COLD void zend_exception_error(zend_object *ex, int severity) /* {{{ */
901900
{
@@ -910,9 +909,9 @@ ZEND_API ZEND_COLD void zend_exception_error(zend_object *ex, int severity) /* {
910909
zend_string *file = zval_get_string(GET_PROPERTY_SILENT(&exception, ZEND_STR_FILE));
911910
zend_long line = zval_get_long(GET_PROPERTY_SILENT(&exception, ZEND_STR_LINE));
912911

913-
zend_error_helper(
912+
zend_error_cb(
914913
(ce_exception == zend_ce_parse_error ? E_PARSE : E_COMPILE_ERROR) | E_DONT_BAIL,
915-
ZSTR_VAL(file), line, "%s", ZSTR_VAL(message));
914+
ZSTR_VAL(file), line, message);
916915

917916
zend_string_release_ex(file, 0);
918917
zend_string_release_ex(message, 0);

‎Zend/zend_exceptions.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ ZEND_API ZEND_COLD zend_object *zend_throw_exception_ex(zend_class_entry *except
6161
ZEND_API ZEND_COLD void zend_throw_exception_object(zval *exception);
6262
ZEND_API void zend_clear_exception(void);
6363

64-
ZEND_API zend_object *zend_throw_error_exception(zend_class_entry *exception_ce, const char *message, zend_long code, int severity);
64+
ZEND_API zend_object *zend_throw_error_exception(zend_class_entry *exception_ce, zend_string *message, zend_long code, int severity);
6565

6666
extern ZEND_API void (*zend_throw_exception_hook)(zval *ex);
6767

‎ext/opcache/ZendAccelerator.c‎

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ zend_bool fallback_process = 0; /* process uses file cache fallback */
121121
static zend_op_array *(*accelerator_orig_compile_file)(zend_file_handle *file_handle, int type);
122122
static int (*accelerator_orig_zend_stream_open_function)(const char *filename, zend_file_handle *handle );
123123
static zend_string *(*accelerator_orig_zend_resolve_path)(const char *filename, size_t filename_len);
124-
static void (*accelerator_orig_zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args);
124+
static void (*accelerator_orig_zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, zend_string *message);
125125
static zif_handler orig_chdir = NULL;
126126
static ZEND_INI_MH((*orig_include_path_on_modify)) = NULL;
127127
static int (*orig_post_startup_cb)(void);
@@ -1669,37 +1669,27 @@ static void zend_accel_init_auto_globals(void)
16691669
}
16701670
}
16711671

1672-
static void persistent_error_cb(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args) {
1672+
static void persistent_error_cb(int type, const char *error_filename, const uint32_t error_lineno, zend_string *message) {
16731673
if (ZCG(record_warnings)) {
16741674
zend_recorded_warning *warning = emalloc(sizeof(zend_recorded_warning));
1675-
va_list args_copy;
16761675
warning->type = type;
16771676
warning->error_lineno = error_lineno;
16781677
warning->error_filename = zend_string_init(error_filename, strlen(error_filename), 0);
1679-
va_copy(args_copy, args);
1680-
warning->error_message = zend_vstrpprintf(0, format, args_copy);
1681-
va_end(args_copy);
1678+
warning->error_message = zend_string_copy(message);
16821679

16831680
ZCG(num_warnings)++;
16841681
ZCG(warnings) = erealloc(ZCG(warnings), sizeof(zend_recorded_warning) * ZCG(num_warnings));
16851682
ZCG(warnings)[ZCG(num_warnings)-1] = warning;
16861683
}
1687-
accelerator_orig_zend_error_cb(type, error_filename, error_lineno, format, args);
1688-
}
1689-
1690-
/* Hack to get us a va_list to pass to zend_error_cb. */
1691-
static void replay_warning_helper(const zend_recorded_warning *warning, ...) {
1692-
va_list va;
1693-
va_start(va, warning);
1694-
accelerator_orig_zend_error_cb(
1695-
warning->type, ZSTR_VAL(warning->error_filename), warning->error_lineno, "%s", va);
1696-
va_end(va);
1684+
accelerator_orig_zend_error_cb(type, error_filename, error_lineno, message);
16971685
}
16981686

16991687
static void replay_warnings(zend_persistent_script *script) {
17001688
for (uint32_t i = 0; i < script->num_warnings; i++) {
17011689
zend_recorded_warning *warning = script->warnings[i];
1702-
replay_warning_helper(warning, ZSTR_VAL(warning->error_message));
1690+
accelerator_orig_zend_error_cb(
1691+
warning->type, ZSTR_VAL(warning->error_filename), warning->error_lineno,
1692+
warning->error_message);
17031693
}
17041694
}
17051695

0 commit comments

Comments
 (0)