Fix phpGH-13817: Segmentation fault for enabled observers after pass 4

Instead of fixing up temporaries count in between observer steps, just apply the additional temporary in the two affected observer steps.

Closes phpGH-14018.

Author: bwoebi

NEWS

@@ -17,6 +17,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-14286 (ffi enum type (when enum has no name) make memory
     leak). (nielsdos, dstogov)
 
+- Opcache:
+  . Fixed bug GH-13817 (Segmentation fault for enabled observers after pass 4).
+    (Bob)
+
 - Soap:
   . Fixed bug #55639 (Digest autentication dont work). (nielsdos)
 

Zend/Optimizer/compact_vars.c

@@ -18,6 +18,7 @@
 
 #include "Optimizer/zend_optimizer_internal.h"
 #include "zend_bitset.h"
+#include "zend_observer.h"
 
 /* This pass removes all CVs and temporaries that are completely unused. It does *not* merge any CVs or TMPs.
  * This pass does not operate on SSA form anymore. */
@@ -117,7 +118,7 @@ void zend_optimizer_compact_vars(zend_op_array *op_array) {
 		op_array->last_var = num_cvs;
 	}
 
-	op_array->T = num_tmps;
+	op_array->T = num_tmps + ZEND_OBSERVER_ENABLED; // reserve last temporary for observers if enabled
 
 	free_alloca(vars_map, use_heap2);
 }

Zend/Optimizer/optimize_temp_vars_5.c

@@ -26,6 +26,7 @@
 #include "zend_execute.h"
 #include "zend_vm.h"
 #include "zend_bitset.h"
+#include "zend_observer.h"
 
 #define INVALID_VAR ((uint32_t)-1)
 #define GET_AVAILABLE_T()					\
@@ -173,5 +174,5 @@ void zend_optimize_temporary_variables(zend_op_array *op_array, zend_optimizer_c
 	}
 
 	zend_arena_release(&ctx->arena, checkpoint);
-	op_array->T = max + 1;
+	op_array->T = max + 1 + ZEND_OBSERVER_ENABLED; // reserve last temporary for observers if enabled
 }

Zend/Optimizer/zend_optimizer.c

@@ -31,7 +31,6 @@
 #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
@@ -1097,8 +1096,6 @@ 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;
 }
 
@@ -1128,8 +1125,6 @@ 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) {
@@ -1557,12 +1552,6 @@ 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]);
@@ -1578,8 +1567,6 @@ 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);

ext/opcache/tests/gh13817.phpt

@@ -0,0 +1,51 @@
+--TEST--
+GH-13712 (Segmentation fault for enabled observers after pass 4)
+--EXTENSIONS--
+opcache
+zend_test
+--INI--
+zend_test.observer.enabled=1
+zend_test.observer.show_output=1
+zend_test.observer.observe_all=1
+opcache.enable=1
+opcache.enable_cli=1
+opcache.optimization_level=0x4069
+--FILE--
+<?php
+
+function inner() {
+	echo "Ok\n";
+}
+
+function foo() {
+	// If stack size is wrong, inner() will corrupt the previous observed frame
+	inner();
+}
+
+// After foo() def so that we land here, with step_two undone for foo() first
+function outer() {
+	// Pass 15 does constant string propagation, which gives a ZEND_INIT_DYNAMIC_FCALL on a const which Pass 4 will optimize
+	// Pass 4 must calc the right stack size here
+	(NAME)();
+}
+
+const NAME = "foo";
+
+outer();
+
+?>
+--EXPECTF--
+<!-- init '%s' -->
+<file '%s'>
+  <!-- init outer() -->
+  <outer>
+    <!-- init foo() -->
+    <foo>
+      <!-- init inner() -->
+      <inner>
+Ok
+      </inner>
+    </foo>
+  </outer>
+</file '%s'>
+