]> granicus.if.org Git - php/commitdiff
Make exit() unwind properly
authorNikita Popov <nikita.ppv@gmail.com>
Fri, 6 Mar 2020 13:57:55 +0000 (14:57 +0100)
committerNikita Popov <nikita.ppv@gmail.com>
Mon, 29 Jun 2020 13:50:12 +0000 (15:50 +0200)
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.

17 files changed:
Zend/tests/exit_exception_handler.phpt [new file with mode: 0644]
Zend/tests/exit_finally_1.phpt [new file with mode: 0644]
Zend/tests/exit_finally_2.phpt [new file with mode: 0644]
Zend/tests/exit_finally_3.phpt [new file with mode: 0644]
Zend/zend.c
Zend/zend_exceptions.c
Zend/zend_exceptions.h
Zend/zend_execute_API.c
Zend/zend_vm_def.h
Zend/zend_vm_execute.h
ext/curl/tests/bug68937.phpt
ext/curl/tests/bug68937_2.phpt
ext/opcache/ZendAccelerator.c
ext/session/tests/bug60634.phpt
ext/soap/soap.c
ext/xsl/tests/bug33853.phpt
sapi/phpdbg/phpdbg_prompt.c

diff --git a/Zend/tests/exit_exception_handler.phpt b/Zend/tests/exit_exception_handler.phpt
new file mode 100644 (file)
index 0000000..eabd0fb
--- /dev/null
@@ -0,0 +1,14 @@
+--TEST--
+Exception handler should not be invoked for exit()
+--FILE--
+<?php
+
+set_exception_handler(function($e) {
+    var_dump($e);
+});
+
+exit("Exit\n");
+
+?>
+--EXPECT--
+Exit
diff --git a/Zend/tests/exit_finally_1.phpt b/Zend/tests/exit_finally_1.phpt
new file mode 100644 (file)
index 0000000..8e7ba74
--- /dev/null
@@ -0,0 +1,19 @@
+--TEST--
+exit() and finally (1)
+--FILE--
+<?php
+
+// TODO: In the future, we should execute the finally block.
+
+try {
+    exit("Exit\n");
+} catch (Throwable $e) {
+    echo "Not caught\n";
+} finally {
+    echo "Finally\n";
+}
+echo "Not executed\n";
+
+?>
+--EXPECT--
+Exit
diff --git a/Zend/tests/exit_finally_2.phpt b/Zend/tests/exit_finally_2.phpt
new file mode 100644 (file)
index 0000000..0a3f5dc
--- /dev/null
@@ -0,0 +1,23 @@
+--TEST--
+exit() and finally (2)
+--FILE--
+<?php
+
+// TODO: In the future, we should execute the finally block.
+
+try {
+    try {
+        exit("Exit\n");
+    } catch (Throwable $e) {
+        echo "Not caught\n";
+    } finally {
+        throw new Exception("Finally exception");
+    }
+    echo "Not executed\n";
+} catch (Exception $e) {
+    echo "Caught {$e->getMessage()}\n";
+}
+
+?>
+--EXPECT--
+Exit
diff --git a/Zend/tests/exit_finally_3.phpt b/Zend/tests/exit_finally_3.phpt
new file mode 100644 (file)
index 0000000..12d7747
--- /dev/null
@@ -0,0 +1,19 @@
+--TEST--
+exit() and finally (3)
+--FILE--
+<?php
+
+// TODO: In the future, we should execute the finally block.
+
+function test() {
+    try {
+        exit("Exit\n");
+    } finally {
+        return 42;
+    }
+}
+var_dump(test());
+
+?>
+--EXPECT--
+Exit
index e855f46cda0e982811f9f277bd278f22fd638e19..2765f8c3ead2251cae059ff9660f40a175c0cc98 100644 (file)
@@ -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(&params[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);
index 594186682b89e38d62db6af9b56c1c6ee2a2c8c6..63f994e580fe5922cf2ad10b8e5f766ce7947666 100644 (file)
@@ -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;
+}
index ced74bf9f163cb1497353991e7c40ad6e4c42b58..fdae31a0137c98c5e89a4c09da24ed069a81639c 100644 (file)
@@ -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"
 
index f1babc3d021ef67a391d183a25da61ef53cead62..d62f104f229a4e96cfd19c18627610c4de117005 100644 (file)
@@ -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;
 }
index 768101f9b3a75209d80af860095910805e351433..241c56851c6c8d0acc9109b2d1f5d63d30fc3971 100644 (file)
@@ -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 */
index a6f2ed827e4047b7830344cf1b49f5efd04d3a38..c1975fd0b9ae1cf760fffe9090fb859fc11408dc 100644 (file)
@@ -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 */
index d9164c57a782e8e78c97f6652cbc9b43319bdbe7..9f28117594947084063cb1d099b455f1c569cd15 100644 (file)
@@ -3,7 +3,6 @@ Bug # #68937 (Segfault in curl_multi_exec)
 --SKIPIF--
 <?php
 include 'skipif.inc';
-if (getenv('SKIP_ASAN')) die('skip some curl versions leak on longjmp');
 ?>
 --FILE--
 <?php
index 4b09b438c5ec9b13f0b4e075a699305dadb92127..1ce5b52114292e9677dc75fb7314d2120584c109 100644 (file)
@@ -3,7 +3,6 @@ Bug # #68937 (Segfault in curl_multi_exec)
 --SKIPIF--
 <?php
 include 'skipif.inc';
-if (getenv('SKIP_ASAN')) die('skip some curl versions leak on longjmp');
 ?>
 --FILE--
 <?php
index ae4d31518809fd563523da177b378c8c2336d72a..23889327a7a14a1595b3a8335466c9c48feca5eb 100644 (file)
@@ -4484,9 +4484,10 @@ static int accel_preload(const char *config)
                                        zend_user_exception_handler();
                                }
                                if (EG(exception)) {
-                                       zend_exception_error(EG(exception), E_ERROR);
-                                       CG(unclean_shutdown) = 1;
-                                       ret = FAILURE;
+                                       ret = zend_exception_error(EG(exception), E_ERROR);
+                                       if (ret == FAILURE) {
+                                               CG(unclean_shutdown) = 1;
+                                       }
                                }
                        }
                        destroy_op_array(op_array);
index b303134e5bce441d8c8230b548ec4243cabfc7f4..09e64ad4bfe3d354f49e6ce713f3af2a2a4b0ea9 100644 (file)
@@ -40,20 +40,6 @@ session_start();
 session_write_close();
 echo "um, hi\n";
 
-/*
- * write() calls die(). This results in calling session_flush() which calls
- * write() in request shutdown. The code is inside save handler still and
- * calls save close handler.
- *
- * Because session_write_close() fails by die(), write() is called twice.
- * close() is still called at request shutdown since session is active.
- */
-
 ?>
 --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
index 6356ac73790c1969f9246641ffa238f386afbd50..1cc6d31ec0a2832816c2a59f8a3f189434106cd4 100644 (file)
@@ -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;
index 815ea03a257686d79ff2c5a8284473417fa844d9..ea39072db426115b55f7c94dffa3bdc3f8999abc 100644 (file)
@@ -3,7 +3,6 @@ Bug #33853 (php:function call __autoload with lowercase param)
 --SKIPIF--
 <?php
 if (!extension_loaded('xsl')) die('skip xsl not loaded');
-if (getenv('SKIP_ASAN')) die('xfail bailing out across foreign C code');
 ?>
 --FILE--
 <?php
index 2f0e8e2440b606eab19ebd48452370d458c0b8f6..f6b5ef41e5315520af3f9ee9004c0c8a0fe0064c 100644 (file)
@@ -1711,6 +1711,11 @@ void phpdbg_execute_ex(zend_execute_data *execute_data) /* {{{ */
                }
 #endif
 
+               if (exception && zend_is_unwind_exit(exception)) {
+                       /* Restore bailout based exit. */
+                       zend_bailout();
+               }
+
                if (PHPDBG_G(flags) & PHPDBG_PREVENT_INTERACTIVE) {
                        phpdbg_print_opline_ex(execute_data, 0);
                        goto next;