Invalid execute_data->opline pointers in observer fcall handlers when JIT is enabled

[evaikene]

Description

When observer fcall handlers are used to observer PHP function calls and tracing JIT is enabled, then execute_data->opline pointers in the fcall handler may become unreliable (not NULL and not valid either) causing the PHP process to crash when these pointers are used.

A common scenario would be accessing execute_data->opline->lineno to get the line number. The following simple observer handler should print out the line number for every called user function:

static void observer_begin(zend_execute_data *ex)
{
    if (ZEND_USER_CODE(ex->func->type) && ex->opline) {
        printf("%u\n", ex->opline->lineno);
   }
}

The actual result is that the PHP process crashes due to the execute_data->opline pointer being not NULL and not a valid pointer either:

Process 46323 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x19)
    frame #0: 0x000000010146be44 php-observer.so`observer_begin(ex=0x0000000100c17150) at php_observer.c:58:36
   55  	static void observer_begin(zend_execute_data *ex)
   56  	{
   57  	   if (ZEND_USER_CODE(ex->func->type) && ex->opline) {
-> 58  	       printf("%u\n", ex->opline->lineno);
   59  	   }
   60  	}

> p ex->opline
(const zend_op *) 0x0000000000000001

Backtrace of the crashing PHP process:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x19)
  * frame #0: 0x000000010146be44 php-observer.so`observer_begin(ex=0x0000000100c17150) at php_observer.c:58:36
    frame #1: 0x0000000100734340 php`_zend_observe_fcall_begin(execute_data=0x0000000100c17150) at zend_observer.c:244:3
    frame #2: 0x00000001007343d4 php`zend_observer_fcall_begin(execute_data=0x0000000100c17150) at zend_observer.c:257:3
    frame #3: 0x0000000138008638
    frame #4: 0x00000001005aef34 php`zend_call_function(fci=0x000000016fdfdc68, fci_cache=0x000000016fdfdc40) at zend_execute_API.c:957:3
    frame #5: 0x0000000100350d18 php`zif_call_user_func_array(execute_data=0x0000000100c16500, return_value=0x0000000100c164c0) at basic_functions.c:1514:6
    frame #6: 0x0000000100698880 php`ZEND_DO_FCALL_BY_NAME_SPEC_OBSERVER_HANDLER(execute_data=0x0000000100c16450) at zend_vm_execute.h:1759:3
    frame #7: 0x00000001006155f4 php`execute_ex(ex=0x0000000100c16020) at zend_vm_execute.h:57007:7
    frame #8: 0x00000001006159f0 php`zend_execute(op_array=0x0000000100c9b000, return_value=0x0000000000000000) at zend_vm_execute.h:61604:2
    frame #9: 0x00000001005cfac0 php`zend_execute_scripts(type=8, retval=0x0000000000000000, file_count=3) at zend.c:1883:4
    frame #10: 0x00000001004fc61c php`php_execute_script(primary_file=0x000000016fdfe570) at main.c:2507:13
    frame #11: 0x00000001007d1440 php`php_cli_server_dispatch_script(server=0x00000001009ab9e8, client=0x0000000114f45990) at php_cli_server.c:2122:3
    frame #12: 0x00000001007cee20 php`php_cli_server_dispatch(server=0x00000001009ab9e8, client=0x0000000114f45990) at php_cli_server.c:2312:18
    frame #13: 0x00000001007cd8d4 php`php_cli_server_recv_event_read_request(server=0x00000001009ab9e8, client=0x0000000114f45990) at php_cli_server.c:2641:11
    frame #14: 0x00000001007cdf84 php`php_cli_server_do_event_for_each_fd_callback(_params=0x000000016fdfe870, fd=5, event=1) at php_cli_server.c:2726:5
    frame #15: 0x00000001007cdbe8 php`php_cli_server_poller_iter_on_active(poller=0x00000001009ab9ec, opaque=0x000000016fdfe870, callback=(php`php_cli_server_do_event_for_each_fd_callback at php_cli_server.c:2688)) at php_cli_server.c:932:20
    frame #16: 0x00000001007cd710 php`php_cli_server_do_event_for_each_fd(server=0x00000001009ab9e8, rhandler=(php`php_cli_server_recv_event_read_request at php_cli_server.c:2620), whandler=(php`php_cli_server_send_event at php_cli_server.c:2652)) at php_cli_server.c:2746:17
    frame #17: 0x00000001007ca94c php`php_cli_server_do_event_loop(server=0x00000001009ab9e8) at php_cli_server.c:2758:4
    frame #18: 0x00000001007ca408 php`do_cli_server(argc=3, argv=0x00006000033b3040) at php_cli_server.c:2890:17
    frame #19: 0x00000001007c18b4 php`main(argc=3, argv=0x00006000033b3040) at php_cli.c:1343:18
    frame #20: 0x000000018d6360e0 dyld`start + 2360

I haven't been able to isolate a simple PHP script that would trigger this issue. The crash above was observed when running an application using Yii framework. @dstogov confirmed the bug and has a zend_test patch that reproduces the issue by running bench.php.

PHP Version

PHP 8.3.4

Operating System

macOS 14.4

[dstogov]

Confirmed. The tracing JIT doesn't set the proper EX(opline) value before calling zend_observer_fcall_begin(). Actually, this assignment is not necessary without observer, and was optimized-out on purpose. Now in case of observer this value has to be reconstructed and assigned.

The problem may be simple reproduced, if we add an assertion that the value of EX(opline) belongs to the currently executed function. E.g. running bench.php with the tracing JIT and the following patch leads to assertion.

diff --git a/ext/zend_test/observer.c b/ext/zend_test/observer.c
index 3e870de450a..00e6aec877b 100644
--- a/ext/zend_test/observer.c
+++ b/ext/zend_test/observer.c
@@ -347,6 +347,25 @@ PHP_INI_BEGIN()
 	STD_PHP_INI_BOOLEAN("zend_test.observer.execute_internal", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_execute_internal, zend_zend_test_globals, zend_test_globals)
 PHP_INI_END()
 
+static void validate_opline_observer_begin(zend_execute_data *ex)
+{
+    if (ZEND_USER_CODE(ex->func->type)) {
+        if (ex->opline) {
+        	ZEND_ASSERT(ex->opline >= ex->func->op_array.opcodes
+        		&& ex->opline < ex->func->op_array.opcodes + ex->func->op_array.last);
+        }
+    }
+}
+
+static void validate_opline_observer_end(zend_execute_data *ex, zval *rval)
+{
+}
+
+static zend_observer_fcall_handlers validate_opline_observer_fcall_init(zend_execute_data *ex)
+{
+    return (zend_observer_fcall_handlers){validate_opline_observer_begin, validate_opline_observer_end};
+}
+
 void zend_test_observer_init(INIT_FUNC_ARGS)
 {
 	// Loading via dl() not supported with the observer API
@@ -378,6 +397,8 @@ void zend_test_observer_init(INIT_FUNC_ARGS)
 		zend_test_prev_execute_internal = zend_execute_internal;
 		zend_execute_internal = zend_test_execute_internal;
 	}
+
+	zend_observer_fcall_register(validate_opline_observer_fcall_init);
 }
 
 void zend_test_observer_shutdown(SHUTDOWN_FUNC_ARGS)