From: Nikita Popov Date: Fri, 6 Mar 2020 13:57:55 +0000 (+0100) Subject: Make exit() unwind properly X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=75a04eac978333467ccd98225d7ef21942ce9e91;p=php Make exit() unwind properly exit() is now internally implemented by throwing an exception, performing a normal stack unwind and a clean shutdown. This ensures that no persistent resource leaks occur. The exception is internal, cannot be caught and does not result in the execution of finally blocks. This may be relaxed in the future. Closes GH-5768. --- diff --git a/Zend/tests/exit_exception_handler.phpt b/Zend/tests/exit_exception_handler.phpt new file mode 100644 index 0000000000..eabd0fb906 --- /dev/null +++ b/Zend/tests/exit_exception_handler.phpt @@ -0,0 +1,14 @@ +--TEST-- +Exception handler should not be invoked for exit() +--FILE-- + +--EXPECT-- +Exit diff --git a/Zend/tests/exit_finally_1.phpt b/Zend/tests/exit_finally_1.phpt new file mode 100644 index 0000000000..8e7ba74842 --- /dev/null +++ b/Zend/tests/exit_finally_1.phpt @@ -0,0 +1,19 @@ +--TEST-- +exit() and finally (1) +--FILE-- + +--EXPECT-- +Exit diff --git a/Zend/tests/exit_finally_2.phpt b/Zend/tests/exit_finally_2.phpt new file mode 100644 index 0000000000..0a3f5dc7a1 --- /dev/null +++ b/Zend/tests/exit_finally_2.phpt @@ -0,0 +1,23 @@ +--TEST-- +exit() and finally (2) +--FILE-- +getMessage()}\n"; +} + +?> +--EXPECT-- +Exit diff --git a/Zend/tests/exit_finally_3.phpt b/Zend/tests/exit_finally_3.phpt new file mode 100644 index 0000000000..12d7747ade --- /dev/null +++ b/Zend/tests/exit_finally_3.phpt @@ -0,0 +1,19 @@ +--TEST-- +exit() and finally (3) +--FILE-- + +--EXPECT-- +Exit diff --git a/Zend/zend.c b/Zend/zend.c index e855f46cda..2765f8c3ea 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -1621,6 +1621,11 @@ ZEND_API ZEND_COLD void zend_user_exception_handler(void) /* {{{ */ zval orig_user_exception_handler; zval params[1], retval2; zend_object *old_exception; + + if (zend_is_unwind_exit(EG(exception))) { + return; + } + old_exception = EG(exception); EG(exception) = NULL; ZVAL_OBJ(¶ms[0], old_exception); @@ -1666,8 +1671,7 @@ ZEND_API int zend_execute_scripts(int type, zval *retval, int file_count, ...) / zend_user_exception_handler(); } if (EG(exception)) { - zend_exception_error(EG(exception), E_ERROR); - ret = FAILURE; + ret = zend_exception_error(EG(exception), E_ERROR); } } destroy_op_array(op_array); diff --git a/Zend/zend_exceptions.c b/Zend/zend_exceptions.c index 594186682b..63f994e580 100644 --- a/Zend/zend_exceptions.c +++ b/Zend/zend_exceptions.c @@ -41,6 +41,9 @@ ZEND_API zend_class_entry *zend_ce_value_error; ZEND_API zend_class_entry *zend_ce_arithmetic_error; ZEND_API zend_class_entry *zend_ce_division_by_zero_error; +/* Internal pseudo-exception that is not exposed to userland. */ +static zend_class_entry zend_ce_unwind_exit; + ZEND_API void (*zend_throw_exception_hook)(zval *ex); static zend_object_handlers default_exception_handlers; @@ -81,6 +84,12 @@ void zend_exception_set_previous(zend_object *exception, zend_object *add_previo if (exception == add_previous || !add_previous || !exception) { return; } + + if (zend_is_unwind_exit(add_previous)) { + OBJ_RELEASE(add_previous); + return; + } + ZVAL_OBJ(&pv, add_previous); if (!instanceof_function(Z_OBJCE(pv), zend_ce_throwable)) { zend_error_noreturn(E_CORE_ERROR, "Previous exception must implement Throwable"); @@ -803,6 +812,8 @@ void zend_register_default_exception(void) /* {{{ */ INIT_CLASS_ENTRY(ce, "DivisionByZeroError", class_DivisionByZeroError_methods); zend_ce_division_by_zero_error = zend_register_internal_class_ex(&ce, zend_ce_arithmetic_error); zend_ce_division_by_zero_error->create_object = zend_default_exception_new; + + INIT_CLASS_ENTRY(zend_ce_unwind_exit, "UnwindExit", NULL); } /* }}} */ @@ -898,10 +909,11 @@ static void zend_error_va(int type, const char *file, uint32_t lineno, const cha /* }}} */ /* This function doesn't return if it uses E_ERROR */ -ZEND_API ZEND_COLD void zend_exception_error(zend_object *ex, int severity) /* {{{ */ +ZEND_API ZEND_COLD int zend_exception_error(zend_object *ex, int severity) /* {{{ */ { zval exception, rv; zend_class_entry *ce_exception; + int result = FAILURE; ZVAL_OBJ(&exception, ex); ce_exception = ex->ce; @@ -961,11 +973,15 @@ ZEND_API ZEND_COLD void zend_exception_error(zend_object *ex, int severity) /* { zend_string_release_ex(str, 0); zend_string_release_ex(file, 0); + } else if (ce_exception == &zend_ce_unwind_exit) { + /* We successfully unwound, nothing more to do */ + result = SUCCESS; } else { zend_error(severity, "Uncaught exception '%s'", ZSTR_VAL(ce_exception->name)); } OBJ_RELEASE(ex); + return result; } /* }}} */ @@ -987,3 +1003,16 @@ ZEND_API ZEND_COLD void zend_throw_exception_object(zval *exception) /* {{{ */ zend_throw_exception_internal(exception); } /* }}} */ + +ZEND_API ZEND_COLD void zend_throw_unwind_exit() +{ + ZEND_ASSERT(!EG(exception)); + EG(exception) = zend_objects_new(&zend_ce_unwind_exit); + EG(opline_before_exception) = EG(current_execute_data)->opline; + EG(current_execute_data)->opline = EG(exception_op); +} + +ZEND_API zend_bool zend_is_unwind_exit(zend_object *ex) +{ + return ex->ce == &zend_ce_unwind_exit; +} diff --git a/Zend/zend_exceptions.h b/Zend/zend_exceptions.h index ced74bf9f1..fdae31a013 100644 --- a/Zend/zend_exceptions.h +++ b/Zend/zend_exceptions.h @@ -66,7 +66,10 @@ ZEND_API zend_object *zend_throw_error_exception(zend_class_entry *exception_ce, extern ZEND_API void (*zend_throw_exception_hook)(zval *ex); /* show an exception using zend_error(severity,...), severity should be E_ERROR */ -ZEND_API ZEND_COLD void zend_exception_error(zend_object *exception, int severity); +ZEND_API ZEND_COLD int zend_exception_error(zend_object *exception, int severity); + +ZEND_API ZEND_COLD void zend_throw_unwind_exit(void); +ZEND_API zend_bool zend_is_unwind_exit(zend_object *ex); #include "zend_globals.h" diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index f1babc3d02..d62f104f22 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -1141,8 +1141,7 @@ ZEND_API int zend_eval_stringl_ex(const char *str, size_t str_len, zval *retval_ result = zend_eval_stringl(str, str_len, retval_ptr, string_name); if (handle_exceptions && EG(exception)) { - zend_exception_error(EG(exception), E_ERROR); - result = FAILURE; + result = zend_exception_error(EG(exception), E_ERROR); } return result; } diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 768101f9b3..241c56851c 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -6893,8 +6893,8 @@ ZEND_VM_COLD_HANDLER(79, ZEND_EXIT, ANY, ANY) } while (0); FREE_OP1(); } - zend_bailout(); - ZEND_VM_NEXT_OPCODE(); /* Never reached */ + zend_throw_unwind_exit(); + HANDLE_EXCEPTION(); } ZEND_VM_HANDLER(57, ZEND_BEGIN_SILENCE, ANY, ANY) @@ -7271,7 +7271,7 @@ ZEND_VM_HELPER(zend_dispatch_try_catch_finally_helper, ANY, ANY, uint32_t try_ca zend_object *ex = EG(exception); /* Walk try/catch/finally structures upwards, performing the necessary actions */ - while (try_catch_offset != (uint32_t) -1) { + for (; try_catch_offset != (uint32_t) -1; try_catch_offset--) { zend_try_catch_element *try_catch = &EX(func)->op_array.try_catch_array[try_catch_offset]; @@ -7281,6 +7281,11 @@ ZEND_VM_HELPER(zend_dispatch_try_catch_finally_helper, ANY, ANY, uint32_t try_ca ZEND_VM_JMP_EX(&EX(func)->op_array.opcodes[try_catch->catch_op], 0); } else if (op_num < try_catch->finally_op) { + if (ex && zend_is_unwind_exit(ex)) { + /* Don't execute finally blocks on exit (for now) */ + continue; + } + /* Go to finally block */ zval *fast_call = EX_VAR(EX(func)->op_array.opcodes[try_catch->finally_end].op1.var); cleanup_live_vars(execute_data, op_num, try_catch->finally_op); @@ -7310,8 +7315,6 @@ ZEND_VM_HELPER(zend_dispatch_try_catch_finally_helper, ANY, ANY, uint32_t try_ca ex = Z_OBJ_P(fast_call); } } - - try_catch_offset--; } /* Uncaught exception */ diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index a6f2ed827e..c1975fd0b9 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -2322,8 +2322,8 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_EXIT_SPEC_HANDLER } while (0); FREE_OP(opline->op1_type, opline->op1.var); } - zend_bailout(); - ZEND_VM_NEXT_OPCODE(); /* Never reached */ + zend_throw_unwind_exit(); + HANDLE_EXCEPTION(); } static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_BEGIN_SILENCE_SPEC_HANDLER(ZEND_OPCODE_HANDLER_ARGS) @@ -2476,9 +2476,10 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_dispatch_try { /* May be NULL during generator closing (only finally blocks are executed) */ zend_object *ex = EG(exception); + zend_bool is_unwind_exit = ex && zend_is_unwind_exit(ex); /* Walk try/catch/finally structures upwards, performing the necessary actions */ - while (try_catch_offset != (uint32_t) -1) { + for (; try_catch_offset != (uint32_t) -1; try_catch_offset--) { zend_try_catch_element *try_catch = &EX(func)->op_array.try_catch_array[try_catch_offset]; @@ -2488,6 +2489,11 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_dispatch_try ZEND_VM_JMP_EX(&EX(func)->op_array.opcodes[try_catch->catch_op], 0); } else if (op_num < try_catch->finally_op) { + if (is_unwind_exit) { + /* Don't execute finally blocks on exit (for now) */ + continue; + } + /* Go to finally block */ zval *fast_call = EX_VAR(EX(func)->op_array.opcodes[try_catch->finally_end].op1.var); cleanup_live_vars(execute_data, op_num, try_catch->finally_op); @@ -2517,8 +2523,6 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_dispatch_try ex = Z_OBJ_P(fast_call); } } - - try_catch_offset--; } /* Uncaught exception */ diff --git a/ext/curl/tests/bug68937.phpt b/ext/curl/tests/bug68937.phpt index d9164c57a7..9f28117594 100644 --- a/ext/curl/tests/bug68937.phpt +++ b/ext/curl/tests/bug68937.phpt @@ -3,7 +3,6 @@ Bug # #68937 (Segfault in curl_multi_exec) --SKIPIF-- --FILE-- --FILE-- --EXPECT-- write: goodbye cruel world - -Warning: Unknown: Cannot call session save handler in a recursive manner in Unknown on line 0 - -Warning: Unknown: Failed to write session data using user defined save handler. (session.save_path: ) in Unknown on line 0 -close: goodbye cruel world diff --git a/ext/soap/soap.c b/ext/soap/soap.c index 6356ac7379..1cc6d31ec0 100644 --- a/ext/soap/soap.c +++ b/ext/soap/soap.c @@ -1382,8 +1382,10 @@ PHP_METHOD(SoapServer, handle) xmlFreeDoc(doc_request); if (EG(exception)) { - php_output_discard(); - _soap_server_exception(service, function, ZEND_THIS); + if (!zend_is_unwind_exit(EG(exception))) { + php_output_discard(); + _soap_server_exception(service, function, ZEND_THIS); + } goto fail; } @@ -1576,15 +1578,17 @@ PHP_METHOD(SoapServer, handle) efree(fn_name); if (EG(exception)) { - php_output_discard(); - _soap_server_exception(service, function, ZEND_THIS); - if (service->type == SOAP_CLASS) { + if (!zend_is_unwind_exit(EG(exception))) { + php_output_discard(); + _soap_server_exception(service, function, ZEND_THIS); + if (service->type == SOAP_CLASS) { #if defined(HAVE_PHP_SESSION) && !defined(COMPILE_DL_SESSION) - if (soap_obj && service->soap_class.persistence != SOAP_PERSISTENCE_SESSION) { + if (soap_obj && service->soap_class.persistence != SOAP_PERSISTENCE_SESSION) { #else - if (soap_obj) { + if (soap_obj) { #endif - zval_ptr_dtor(soap_obj); + zval_ptr_dtor(soap_obj); + } } } goto fail; diff --git a/ext/xsl/tests/bug33853.phpt b/ext/xsl/tests/bug33853.phpt index 815ea03a25..ea39072db4 100644 --- a/ext/xsl/tests/bug33853.phpt +++ b/ext/xsl/tests/bug33853.phpt @@ -3,7 +3,6 @@ Bug #33853 (php:function call __autoload with lowercase param) --SKIPIF-- --FILE--