Save previous observer on the VM stack

This avoids a possible significant performance penalty, when some leaf function was observed, deep in the stack.
As a side effect, we are not iterating over prev_execute_data anymore and thus, non-observed fake frames, possibly on stack, cannot have any impact on the observer anymore (especially within zend_observer_fcall_end_all).

Saving the previous observer happens now directly on the VM stack. If there is any observer, function frames are allocated an extra zval (the last temporary), which will, on observed frames, contain the previous observed frame address.

Author: bwoebi

Zend/Optimizer/zend_optimizer.c

@@ -31,6 +31,7 @@
 #include "zend_inference.h"
 #include "zend_dump.h"
 #include "php.h"
+#include "zend_observer.h"
 
 #ifndef ZEND_OPTIMIZER_MAX_REGISTERED_PASSES
 # define ZEND_OPTIMIZER_MAX_REGISTERED_PASSES 32
@@ -1094,6 +1095,8 @@ static void zend_revert_pass_two(zend_op_array *op_array)
 	}
 #endif
 
+	op_array->T -= ZEND_OBSERVER_ENABLED;
+
 	op_array->fn_flags &= ~ZEND_ACC_DONE_PASS_TWO;
 }
 
@@ -1123,6 +1126,8 @@ static void zend_redo_pass_two(zend_op_array *op_array)
 	}
 #endif
 
+	op_array->T += ZEND_OBSERVER_ENABLED; // reserve last temporary for observers if enabled
+
 	opline = op_array->opcodes;
 	end = opline + op_array->last;
 	while (opline < end) {
@@ -1550,6 +1555,12 @@ ZEND_API void zend_optimize_script(zend_script *script, zend_long optimization_l
 			}
 		}
 
+		if (ZEND_OBSERVER_ENABLED) {
+			for (i = 0; i < call_graph.op_arrays_count; i++) {
+				++call_graph.op_arrays[i]->T; // ensure accurate temporary count for stack size precalculation
+			}
+		}
+
 		if (ZEND_OPTIMIZER_PASS_12 & optimization_level) {
 			for (i = 0; i < call_graph.op_arrays_count; i++) {
 				zend_adjust_fcall_stack_size_graph(call_graph.op_arrays[i]);
@@ -1565,6 +1576,8 @@ ZEND_API void zend_optimize_script(zend_script *script, zend_long optimization_l
 					zend_recalc_live_ranges(op_array, needs_live_range);
 				}
 			} else {
+				op_array->T -= ZEND_OBSERVER_ENABLED; // redo_pass_two will re-increment it
+
 				zend_redo_pass_two(op_array);
 				if (op_array->live_range) {
 					zend_recalc_live_ranges(op_array, NULL);

Zend/zend_API.c

@@ -2685,6 +2685,7 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend
 	}
 	internal_function->type = ZEND_INTERNAL_FUNCTION;
 	internal_function->module = EG(current_module);
+	internal_function->T = 0;
 	memset(internal_function->reserved, 0, ZEND_MAX_RESERVED_RESOURCES * sizeof(void*));
 
 	while (ptr->fname) {

Zend/zend_compile.h

@@ -448,12 +448,12 @@ struct _zend_op_array {
 	uint32_t required_num_args;
 	zend_arg_info *arg_info;
 	HashTable *attributes;
+	uint32_t T;         /* number of temporary variables */
 	ZEND_MAP_PTR_DEF(void **, run_time_cache);
 	/* END of common elements */
 
 	int cache_size;     /* number of run_time_cache_slots * sizeof(void*) */
 	int last_var;       /* number of CV variables */
-	uint32_t T;         /* number of temporary variables */
 	uint32_t last;      /* number of opcodes */
 
 	zend_op *opcodes;
@@ -503,6 +503,7 @@ typedef struct _zend_internal_function {
 	uint32_t required_num_args;
 	zend_internal_arg_info *arg_info;
 	HashTable *attributes;
+	uint32_t T;         /* number of temporary variables */
 	ZEND_MAP_PTR_DEF(void **, run_time_cache);
 	/* END of common elements */
 
@@ -528,6 +529,7 @@ union _zend_function {
 		uint32_t required_num_args;
 		zend_arg_info *arg_info;  /* index -1 represents the return value info, if any */
 		HashTable   *attributes;
+		uint32_t T;         /* number of temporary variables */
 		ZEND_MAP_PTR_DEF(void **, run_time_cache);
 	} common;
 

Zend/zend_enum.c

@@ -23,6 +23,7 @@
 #include "zend_interfaces.h"
 #include "zend_enum.h"
 #include "zend_extensions.h"
+#include "zend_observer.h"
 
 #define ZEND_ENUM_DISALLOW_MAGIC_METHOD(propertyName, methodName) \
 	do { \
@@ -407,6 +408,7 @@ static void zend_enum_register_func(zend_class_entry *ce, zend_known_string_id n
 	zif->type = ZEND_INTERNAL_FUNCTION;
 	zif->module = EG(current_module);
 	zif->scope = ce;
+	zif->T = ZEND_OBSERVER_ENABLED;
 	ZEND_MAP_PTR_NEW(zif->run_time_cache);
 	ZEND_MAP_PTR_SET(zif->run_time_cache, zend_arena_alloc(&CG(arena), zend_internal_run_time_cache_reserved_size()));
 

Zend/zend_execute.c

@@ -144,6 +144,7 @@ ZEND_API const zend_internal_function zend_pass_function = {
 	0,                      /* required_num_args */
 	(zend_internal_arg_info *) zend_pass_function_arg_info + 1, /* arg_info */
 	NULL,                   /* attributes        */
+	0,                      /* T                 */
 	NULL,                   /* run_time_cache    */
 	ZEND_FN(pass),          /* handler           */
 	NULL,                   /* module            */

Zend/zend_execute.h

@@ -245,10 +245,10 @@ static zend_always_inline zend_execute_data *zend_vm_stack_push_call_frame_ex(ui
 
 static zend_always_inline uint32_t zend_vm_calc_used_stack(uint32_t num_args, zend_function *func)
 {
-	uint32_t used_stack = ZEND_CALL_FRAME_SLOT + num_args;
+	uint32_t used_stack = ZEND_CALL_FRAME_SLOT + num_args + func->common.T;
 
 	if (EXPECTED(ZEND_USER_CODE(func->type))) {
-		used_stack += func->op_array.last_var + func->op_array.T - MIN(func->op_array.num_args, num_args);
+		used_stack += func->op_array.last_var - MIN(func->op_array.num_args, num_args);
 	}
 	return used_stack * sizeof(zval);
 }

Zend/zend_observer.c

@@ -78,6 +78,18 @@ ZEND_API void zend_observer_post_startup(void)
 		ZEND_VM_SET_OPCODE_HANDLER(EG(exception_op));
 		ZEND_VM_SET_OPCODE_HANDLER(EG(exception_op) + 1);
 		ZEND_VM_SET_OPCODE_HANDLER(EG(exception_op) + 2);
+
+		// Add an observer temporary to store previous observed frames
+		zend_internal_function *zif;
+		ZEND_HASH_FOREACH_PTR(CG(function_table), zif) {
+			++zif->T;
+		} ZEND_HASH_FOREACH_END();
+		zend_class_entry *ce;
+		ZEND_HASH_MAP_FOREACH_PTR(CG(class_table), ce) {
+			ZEND_HASH_MAP_FOREACH_PTR(&ce->function_table, zif) {
+				++zif->T;
+			} ZEND_HASH_FOREACH_END();
+		} ZEND_HASH_FOREACH_END();
 	}
 }
 
@@ -187,6 +199,11 @@ ZEND_API bool zend_observer_remove_end_handler(zend_function *function, zend_obs
 	return zend_observer_remove_handler((void **)&ZEND_OBSERVER_DATA(function) + registered_observers, end);
 }
 
+static inline zend_execute_data **prev_observed_frame(zend_execute_data *execute_data) {
+	zend_function *func = EX(func);
+	return (zend_execute_data **)&Z_PTR_P(EX_VAR_NUM((ZEND_USER_CODE(func->type) ? func->op_array.last_var : ZEND_CALL_NUM_ARGS(execute_data)) + func->common.T - 1));
+}
+
 static void ZEND_FASTCALL _zend_observe_fcall_begin(zend_execute_data *execute_data)
 {
 	if (!ZEND_OBSERVER_ENABLED) {
@@ -208,9 +225,7 @@ static void ZEND_FASTCALL _zend_observe_fcall_begin(zend_execute_data *execute_d
 
 	zend_observer_fcall_end_handler *end_handler = (zend_observer_fcall_end_handler *)possible_handlers_end;
 	if (*end_handler != ZEND_OBSERVER_NOT_OBSERVED) {
-		if (first_observed_frame == NULL) {
-			first_observed_frame = execute_data;
-		}
+		*prev_observed_frame(execute_data) = current_observed_frame;
 		current_observed_frame = execute_data;
 	}
 
@@ -236,29 +251,9 @@ ZEND_API void ZEND_FASTCALL zend_observer_fcall_begin(zend_execute_data *execute
 	}
 }
 
-static inline bool zend_observer_is_skipped_frame(zend_execute_data *execute_data) {
-	zend_function *func = execute_data->func;
-
-	if (!func || !ZEND_OBSERVABLE_FN(func)) {
-		return true;
-	}
-
-	zend_observer_fcall_end_handler end_handler = (&ZEND_OBSERVER_DATA(func))[zend_observers_fcall_list.count];
-	if (end_handler == NULL || end_handler == ZEND_OBSERVER_NOT_OBSERVED) {
-		return true;
-	}
-
-	return false;
-}
-
-ZEND_API void ZEND_FASTCALL zend_observer_fcall_end(zend_execute_data *execute_data, zval *return_value)
-{
+static inline void call_end_observers(zend_execute_data *execute_data, zval *return_value) {
 	zend_function *func = execute_data->func;
 
-	if (!ZEND_OBSERVER_ENABLED || !ZEND_OBSERVABLE_FN(func)) {
-		return;
-	}
-
 	zend_observer_fcall_end_handler *handler = (zend_observer_fcall_end_handler *)&ZEND_OBSERVER_DATA(func) + zend_observers_fcall_list.count;
 	// TODO: Fix exceptions from generators
 	// ZEND_ASSERT(fcall_data);
@@ -270,28 +265,27 @@ ZEND_API void ZEND_FASTCALL zend_observer_fcall_end(zend_execute_data *execute_d
 	do {
 		(*handler)(execute_data, return_value);
 	} while (++handler != possible_handlers_end && *handler != NULL);
+}
 
-	if (first_observed_frame == execute_data) {
-		first_observed_frame = NULL;
-		current_observed_frame = NULL;
-	} else {
-		zend_execute_data *ex = execute_data->prev_execute_data;
-		while (ex && zend_observer_is_skipped_frame(ex)) {
-			ex = ex->prev_execute_data;
-		}
-		current_observed_frame = ex;
+ZEND_API void ZEND_FASTCALL zend_observer_fcall_end(zend_execute_data *execute_data, zval *return_value)
+{
+	if (execute_data != current_observed_frame) {
+		return;
 	}
+	call_end_observers(execute_data, return_value);
+	current_observed_frame = *prev_observed_frame(execute_data);
 }
 
 ZEND_API void zend_observer_fcall_end_all(void)
 {
-	zend_execute_data *ex = current_observed_frame;
-	while (ex != NULL) {
-		if (ex->func) {
-			zend_observer_fcall_end(ex, NULL);
-		}
-		ex = ex->prev_execute_data;
+	zend_execute_data *execute_data = current_observed_frame, *original_execute_data = EG(current_execute_data);
+	current_observed_frame = NULL;
+	while (execute_data) {
+		EG(current_execute_data) = execute_data;
+		call_end_observers(execute_data, NULL);
+		execute_data = *prev_observed_frame(execute_data);
 	}
+	EG(current_execute_data) = original_execute_data;
 }
 
 ZEND_API void zend_observer_error_register(zend_observer_error_cb cb)

Zend/zend_opcode.c

@@ -27,6 +27,7 @@
 #include "zend_API.h"
 #include "zend_sort.h"
 #include "zend_constants.h"
+#include "zend_observer.h"
 
 #include "zend_vm.h"
 
@@ -1049,6 +1050,8 @@ ZEND_API void pass_two(zend_op_array *op_array)
 	CG(context).literals_size = op_array->last_literal;
 #endif
 
+    op_array->T += ZEND_OBSERVER_ENABLED; // reserve last temporary for observers if enabled
+
 	/* Needs to be set directly after the opcode/literal reallocation, to ensure destruction
 	 * happens correctly if any of the following fixups generate a fatal error. */
 	op_array->fn_flags |= ZEND_ACC_DONE_PASS_TWO;

ext/opcache/jit/zend_jit_arm64.dasc

@@ -8407,7 +8407,7 @@ static int zend_jit_push_call_frame(dasm_State **Dst, const zend_op *opline, con
 			stack_check = 0;
 		}
 	} else {
-		used_stack = (ZEND_CALL_FRAME_SLOT + opline->extended_value) * sizeof(zval);
+		used_stack = (ZEND_CALL_FRAME_SLOT + opline->extended_value + ZEND_OBSERVER_ENABLED) * sizeof(zval);
 
 		|	// if (EXPECTED(ZEND_USER_CODE(func->type))) {
 		if (!is_closure) {

ext/opcache/jit/zend_jit_x86.dasc

@@ -9010,7 +9010,7 @@ static int zend_jit_push_call_frame(dasm_State **Dst, const zend_op *opline, con
 			stack_check = 0;
 		}
 	} else {
-		used_stack = (ZEND_CALL_FRAME_SLOT + opline->extended_value) * sizeof(zval);
+		used_stack = (ZEND_CALL_FRAME_SLOT + opline->extended_value + ZEND_OBSERVER_ENABLED) * sizeof(zval);
 
 		|	// if (EXPECTED(ZEND_USER_CODE(func->type))) {
 		if (!is_closure) {

ext/pdo/pdo_dbh.c

@@ -33,6 +33,7 @@
 #include "zend_object_handlers.h"
 #include "zend_hash.h"
 #include "pdo_dbh_arginfo.h"
+#include "zend_observer.h"
 
 static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value);
 
@@ -1246,6 +1247,7 @@ bool pdo_hash_methods(pdo_dbh_object_t *dbh_obj, int kind)
 		func.scope = dbh_obj->std.ce;
 		func.prototype = NULL;
 		ZEND_MAP_PTR(func.run_time_cache) = NULL;
+		func.T = ZEND_OBSERVER_ENABLED;
 		if (funcs->flags) {
 			func.fn_flags = funcs->flags | ZEND_ACC_NEVER_CACHE;
 		} else {