Assertion failure Zend/zend_exceptions.c:197

Description

The following code:

<?php
if (defined("pass3")) {
} else {
function errorHandler($errorNumber, $errorMessage, $fileName, $lineNumber) {
define("pass3", 1);
include(__FILE__);
}
set_error_handler('errorHandler');
}
$resource = zend_test_create_throwing_resource();
try {ldap_delete_ext($fileName,$exception,$fileName);} catch (Exception $e) { echo($e); }

Resulted in this output:

php: /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_exceptions.c:197: void zend_throw_exception_internal(zend_object *): Assertion `is_handle_exception_set() && "HANDLE_EXCEPTION not set?"' failed.
Aborted (core dumped)

PHP Version

nightly

Operating System

No response

Simplified test:

<?php
declare(strict_types=1);
function errorHandler($errorNumber, $errorMessage, $fileName, $lineNumber) {
    $resource = zend_test_create_throwing_resource();
    strlen(null);
}
set_error_handler('errorHandler');
echo $undef;

The assertion may be too conservative, as the HANDLE_EXCEPTION is set later on. But I'm not sure (yet).

diff --git a/Zend/zend_exceptions.c b/Zend/zend_exceptions.c
index 11b615c214a..0c3c6e8f743 100644
--- a/Zend/zend_exceptions.c
+++ b/Zend/zend_exceptions.c
@@ -193,7 +193,7 @@ ZEND_API ZEND_COLD void zend_throw_exception_internal(zend_object *exception) /*
 		zend_exception_set_previous(exception, EG(exception));
 		EG(exception) = exception;
 		if (previous) {
-			ZEND_ASSERT(is_handle_exception_set() && "HANDLE_EXCEPTION not set?");
+			//ZEND_ASSERT(is_handle_exception_set() && "HANDLE_EXCEPTION not set?");
 			return;
 		}
 	}

This can be reproduced a bit simpler:

function test() {
    $resource = zend_test_create_throwing_resource();
    zend_test_create_throwing_resource();
}
test();

I think the issue is that zend_test_create_throwing_resource() will install the exception on the test() frame, unwind to main(), throw for the freed $resource variable, and verify that the exception op is set (which it isn't, as it was set in test() and only rethrown later.

Two possible solutions are:

Switch to the previous stack frame after freeing the stack frames variables. This has some observable effects, namely for destructors. This might or might not matter.
Adjust opline of the restored stack frame earlier. This seems like the less risky solution.

Details

diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
index 7e471b5acd8..fa950f81cfb 100644
--- a/Zend/zend_vm_def.h
+++ b/Zend/zend_vm_def.h
@@ -2925,6 +2925,9 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY)
 
 	if (EXPECTED((call_info & (ZEND_CALL_CODE|ZEND_CALL_TOP|ZEND_CALL_HAS_SYMBOL_TABLE|ZEND_CALL_FREE_EXTRA_ARGS|ZEND_CALL_ALLOCATED|ZEND_CALL_HAS_EXTRA_NAMED_PARAMS)) == 0)) {
 		EG(current_execute_data) = EX(prev_execute_data);
+		if (UNEXPECTED(EG(exception) != NULL)) {
+			zend_rethrow_exception(EG(current_execute_data));
+		}
 		i_free_compiled_variables(execute_data);
 
 #ifdef ZEND_PREFER_RELOAD
@@ -2939,7 +2942,6 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY)
 		execute_data = EX(prev_execute_data);
 
 		if (UNEXPECTED(EG(exception) != NULL)) {
-			zend_rethrow_exception(execute_data);
 			HANDLE_EXCEPTION_LEAVE();
 		}

WDYT?

Looking a bit closer into this, ZEND_DO_ICALL is also susceptible to the same issue. It also restores the previous call frame and then freeing the popped stack frames variables without rethrowing first. I can't reproduce the issue though because the exception keeps the passed reference alive.

--INI--
zend.exception_ignore_args=1
--FILE--
function test() {
    \array_values(zend_test_create_throwing_resource());
}
test();

Maybe it's better to adjust the assertion, rather than adding additional checks everywhere.

I think the assertion should be adjusted because the end result works anyway

Yeah I think I agree.

@nielsdos I think it's ok to just remove these asserts. We don't have a good way to check whether we just switched to this frame and the exception opline will be installed later. Do you want to send a PR, since this was your original idea?

@iluuu1994

@iluuu1994 Well you did the more in-depth analysis, so it's at least a joint effort. I'll make a PR and add your C-o-b tag

Fix GH-17408: Assertion failure Zend/zend_exceptions.c

Merge branch 'PHP-8.3' into PHP-8.4

Merge branch 'PHP-8.4'

Navigation Menu

Repository navigation

Assertion failure Zend/zend_exceptions.c:197 #17408

Description

PHP Version

Operating System

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Assertion failure Zend/zend_exceptions.c:197 #17408

Description

Description

PHP Version

Operating System

Activity

ndossche commented on Jan 9, 2025

ndossche commented on Jan 9, 2025

iluuu1994 commented on Jan 16, 2025

iluuu1994 commented on Jan 16, 2025

ndossche commented on Jan 17, 2025

iluuu1994 commented on Jan 18, 2025

iluuu1994 commented on Jan 20, 2025

ndossche commented on Jan 20, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions