JIT: Propagate trace too long from zend_jit_trace_record_fake_init_call()

zend_jit_trace_record_fake_init_call() silently truncated its recording
when the trace buffer became full: TRACE_RECORD() broke out of the loop
and the index, already past the limit, was returned as a success. Report
the failure as -(int)ZEND_JIT_TRACE_STOP_TOO_LONG instead.

The callers did check for a negative result, but mapped it to
ZEND_JIT_TRACE_STOP_BAD_FUNC, which dates back to the time when -1 meant
"this pending call can't be traced" (those returns were replaced by
"continue recording" in 097edc8). BAD_FUNC is a successful stop
reason, so the truncated recording (a start record, a partial fake init
call sequence and an end record) is handed to the trace compiler; at
trace start this produces a trace that loops forever. Propagate the
reported stop reason instead, so that the incomplete recording is
discarded as too long. zend_jit_trace_subtrace() now reports its own
failure the same way instead of returning a bare -1, which would have
been propagated as ZEND_JIT_TRACE_STOP_RECURSIVE_CALL.

Author: arnaud-lb

ext/opcache/jit/zend_jit_vm_helpers.c

@@ -628,7 +628,7 @@ static uint8_t zend_jit_trace_bad_stop_event(const zend_op *opline, int count)
 
 static int zend_jit_trace_record_fake_init_call_ex(zend_execute_data *call, zend_jit_trace_rec *trace_buffer, int idx, uint32_t is_megamorphic, uint32_t init_level)
 {
-	zend_jit_trace_stop stop ZEND_ATTRIBUTE_UNUSED = ZEND_JIT_TRACE_STOP_ERROR;
+	zend_jit_trace_stop stop = ZEND_JIT_TRACE_STOP_ERROR;
 
 	do {
 		zend_function *func;
@@ -671,15 +671,21 @@ static int zend_jit_trace_record_fake_init_call_ex(zend_execute_data *call, zend
 			ZEND_ADD_CALL_FLAG(call, ZEND_CALL_MEGAMORPHIC);
 		}
 		TRACE_RECORD(ZEND_JIT_TRACE_INIT_CALL, ZEND_JIT_TRACE_FAKE_INFO(init_level), func);
+
+		return idx;
 	} while (0);
-	return idx;
+
+	/* TRACE_RECORD() may jump here */
+	return -(int)stop;
 }
 
+/* Returns the new trace buffer index, or -(int)zend_jit_trace_stop on failure */
 static int zend_jit_trace_record_fake_init_call(zend_execute_data *call, zend_jit_trace_rec *trace_buffer, int idx, uint32_t is_megamorphic)
 {
 	return zend_jit_trace_record_fake_init_call_ex(call, trace_buffer, idx, is_megamorphic, 0);
 }
 
+/* Returns the new trace buffer index, or -(int)zend_jit_trace_stop on failure */
 static int zend_jit_trace_subtrace(zend_execute_data *call, zend_jit_trace_rec *trace_buffer, int start, int end, uint8_t event, const zend_op_array *op_array, const zend_op *opline)
 {
 	int idx;
@@ -692,7 +698,7 @@ static int zend_jit_trace_subtrace(zend_execute_data *call, zend_jit_trace_rec *
 		}
 	}
 	if (idx + (end - start) >= JIT_G(max_trace_length) - 2) {
-		return -1;
+		return -(int)ZEND_JIT_TRACE_STOP_TOO_LONG;
 	}
 	memmove(trace_buffer + idx, trace_buffer + start, (end - start) * sizeof(zend_jit_trace_rec));
 	return idx + (end - start);
@@ -810,12 +816,15 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data  *ex,
 	if (prev_call) {
 		int ret = zend_jit_trace_record_fake_init_call(prev_call, trace_buffer, idx, is_megamorphic);
 		if (ret < 0) {
-			TRACE_END(ZEND_JIT_TRACE_END, ZEND_JIT_TRACE_STOP_BAD_FUNC, opline);
+			/* The recorded prefix is incomplete (some pending calls are
+			 * missing), so it must not be compiled. */
+			stop = (zend_jit_trace_stop)-ret;
+			TRACE_END(ZEND_JIT_TRACE_END, stop, opline);
 #ifdef HAVE_GCC_GLOBAL_REGS
 			execute_data = save_execute_data;
 			opline = save_opline;
 #endif
-			return ZEND_JIT_TRACE_STOP_BAD_FUNC;
+			return stop;
 		}
 		idx = ret;
 	}
@@ -1199,7 +1208,7 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data  *ex,
 						if (prev_call) {
 							int ret = zend_jit_trace_record_fake_init_call(prev_call, trace_buffer, idx, 0);
 							if (ret < 0) {
-								stop = ZEND_JIT_TRACE_STOP_BAD_FUNC;
+								stop = (zend_jit_trace_stop)-ret;
 								break;
 							}
 							idx = ret;
@@ -1227,7 +1236,7 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data  *ex,
 						if (prev_call) {
 							int ret = zend_jit_trace_record_fake_init_call(prev_call, trace_buffer, idx, 0);
 							if (ret < 0) {
-								stop = ZEND_JIT_TRACE_STOP_BAD_FUNC;
+								stop = (zend_jit_trace_stop)-ret;
 								break;
 							}
 							idx = ret;
@@ -1364,7 +1373,7 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data  *ex,
 					int ret = zend_jit_trace_subtrace(EX(call), trace_buffer,
 						last_loop, idx, ZEND_JIT_TRACE_START_LOOP, op_array, opline);
 					if (ret < 0) {
-						stop = ZEND_JIT_TRACE_STOP_TOO_LONG;
+						stop = (zend_jit_trace_stop)-ret;
 						break;
 					}
 					idx = ret;

ext/opcache/tests/jit/fake_init_call_too_long_recursive_ret.phpt

@@ -0,0 +1,30 @@
+--TEST--
+JIT: trace buffer overflow while recording fake init calls on recursive return
+--INI--
+opcache.jit_max_trace_length=19
+--FILE--
+<?php
+
+function id($x) { return $x; }
+
+/* The return trace started after the recursive call of rec() unrolls one
+ * return. At that point the parent rec() frame still has the two id() calls
+ * under construction, and recording a fake init call for them overflows the
+ * trace buffer. */
+function rec($n) {
+    if ($n <= 0) {
+        return 0;
+    }
+    return id(id(rec($n - 1)));
+}
+
+$s = 0;
+for ($i = 0; $i < 50; $i++) {
+    $s += rec(10);
+}
+
+var_dump($s);
+
+?>
+--EXPECT--
+int(0)

ext/opcache/tests/jit/fake_init_call_too_long_side_ret.phpt

@@ -0,0 +1,59 @@
+--TEST--
+JIT: trace buffer overflow while recording fake init calls on side trace return
+--INI--
+opcache.jit_max_trace_length=55
+--FILE--
+<?php
+
+function id($x) { return $x; }
+
+/* The side trace started at the guard of $a[$k] records the (rarely taken)
+ * long branch and returns from g() into outer(), where the two id() calls are
+ * still under construction. Recording a fake init call for them overflows the
+ * trace buffer at that point. */
+function g(array $a, int $k) {
+    $x = $a[$k];
+    if ($x < 0) {
+            $x = $x + 1;
+            $x = $x + 2;
+            $x = $x + 3;
+            $x = $x + 4;
+            $x = $x + 5;
+            $x = $x + 6;
+            $x = $x + 7;
+            $x = $x + 8;
+            $x = $x + 9;
+            $x = $x + 10;
+            $x = $x + 11;
+            $x = $x + 12;
+            $x = $x + 13;
+            $x = $x + 14;
+            $x = $x + 15;
+            $x = $x + 16;
+            $x = $x + 17;
+            $x = $x + 18;
+            $x = $x + 19;
+            $x = $x + 20;
+            $x = $x + 21;
+            $x = $x + 22;
+            $x = $x + 23;
+            $x = $x + 24;
+    }
+    return 1;
+}
+
+function outer(array $a, int $k) { return id(id(g($a, $k))); }
+
+function driver(array $a) {
+    $s = 0;
+    for ($i = 0; $i < 300; $i++) {
+        $s += outer($a, $i % 11);
+    }
+    return $s;
+}
+
+var_dump(driver([1, 1, 1, 1, 1, 1, 1, 1, 1, 1, -1]));
+
+?>
+--EXPECT--
+int(300)

ext/opcache/tests/jit/fake_init_call_too_long_start.phpt

@@ -0,0 +1,33 @@
+--TEST--
+JIT: trace buffer overflow while recording fake init calls at trace start
+--INI--
+opcache.jit_max_trace_length=8
+--FILE--
+<?php
+
+class C { static function f($x) { return $x + 1; } }
+
+function sink(array $a) { return array_sum($a); }
+function id($x) { return $x; }
+
+/* array_map() is compiled to a foreach loop, so the loop header sits between
+ * the INIT_FCALL of sink()/id() and their DO_UCALL. A root loop trace started
+ * there has to record a fake init call for each of the 6 pending calls, which
+ * does not fit into a trace buffer limited to 8 records. The recording must be
+ * aborted; the partially recorded trace must not be compiled. */
+function test(array $a, $o) {
+    return sink(id(id(id(id(id(array_map($o::f(...), $a)))))));
+}
+
+$a = [1, 2, 3];
+$o = new C();
+
+for ($i = 0; $i < 5; $i++) {
+    $r = test($a, $o);
+}
+
+var_dump($r);
+
+?>
+--EXPECT--
+int(9)