Check VM interrupt while internal frame is on top

Author: morrisonlevi

UPGRADING.INTERNALS

@@ -333,6 +333,11 @@ PHP 8.4 INTERNALS UPGRADE NOTES
 4. OpCode changes
 ========================
 
+* DO_ICALL, DO_FCALL, and DO_FCALL_BY_NAME now call zend_interrupt_function
+  while the internal frame is still on the stack. This means interrupt handlers
+  will now see the internal call. If your interrupt handler does something like
+  switching EG(current_execute_data), it should not do so if an internal func
+  is on top.
 * New FRAMELESS_ICALL_[0,3] opcodes for faster internal function calls have been
   added. These opcodes don't create a stack frame, but pass arguments via opcode
   operands. They only work for functions that are known at compile-time, and

Zend/tests/fibers/signal-async.phpt

@@ -17,7 +17,7 @@ pcntl_signal(SIGUSR1, function (): void {
 $fiber = new Fiber(function (): void {
     echo "Fiber start\n";
     posix_kill(posix_getpid(), SIGUSR1);
-    time_nanosleep(1);
+    time_nanosleep(1, 0);
     echo "Fiber end\n";
 });
 
@@ -30,8 +30,9 @@ Fiber start
 Fatal error: Uncaught FiberError: Cannot switch fibers in current execution context in %ssignal-async.php:%d
 Stack trace:
 #0 %ssignal-async.php(%d): Fiber::suspend()
-#1 %ssignal-async.php(%d): {closure:%s:%d}(%d, Array)
-#2 [internal function]: {closure:%s:%d}()
-#3 %ssignal-async.php(%d): Fiber->start()
-#4 {main}
+#1 [internal function]: {closure:%s:%d}(%d, Array)
+#2 %ssignal-async.php(%d): posix_kill(%d, %d)
+#3 [internal function]: {closure:%s:%d}()
+#4 %ssignal-async.php(%d): Fiber->start()
+#5 {main}
   thrown in %ssignal-async.php on line %d

Zend/zend.h

@@ -341,7 +341,22 @@ extern ZEND_API size_t (*zend_printf)(const char *format, ...) ZEND_ATTRIBUTE_PT
 extern ZEND_API zend_write_func_t zend_write;
 extern ZEND_API FILE *(*zend_fopen)(zend_string *filename, zend_string **opened_path);
 extern ZEND_API void (*zend_ticks_function)(int ticks);
+
+/* Called by the VM in certain places like at the loop header, user function
+ * entry, and after internal function calls, if EG(vm_interrupt) has been set.
+ *
+ * If this is used to switch the EG(current_execute_data), such as implementing
+ * a coroutine scheduler, then it needs to check the top frame to see if it's
+ * an internal function. If an internal function is on top, then the frame
+ * shouldn't be switched away.
+ *
+ * Prior to PHP 8.0, this check was not necessary. In PHP 8.0,
+ * zend_call_function started calling zend_interrupt_function, and in 8.4 the
+ * DO_*CALL* opcodes started calling the zend_interrupt_function while the
+ * internal frame is still on top.
+ */
 extern ZEND_API void (*zend_interrupt_function)(zend_execute_data *execute_data);
+
 extern ZEND_API void (*zend_error_cb)(int type, zend_string *error_filename, const uint32_t error_lineno, zend_string *message);
 extern ZEND_API void (*zend_on_timeout)(int seconds);
 extern ZEND_API zend_result (*zend_stream_open_function)(zend_file_handle *handle);

Zend/zend_vm_def.h

@@ -4075,6 +4075,14 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL,OBSERVER))
 #endif
 	ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret);
 
+	if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) {
+		if (zend_atomic_bool_load_ex(&EG(timed_out))) {
+			zend_timeout();
+		} else if (zend_interrupt_function) {
+			zend_interrupt_function(execute_data);
+		}
+	}
+
 	EG(current_execute_data) = execute_data;
 	zend_vm_stack_free_args(call);
 
@@ -4097,7 +4105,8 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL,OBSERVER))
 		HANDLE_EXCEPTION();
 	}
 
-	ZEND_VM_SET_OPCODE(opline + 1);
+	CHECK_SYMBOL_TABLES()
+	OPLINE = opline + 1;
 	ZEND_VM_CONTINUE();
 }
 
@@ -4196,6 +4205,14 @@ ZEND_VM_HOT_HANDLER(131, ZEND_DO_FCALL_BY_NAME, ANY, ANY, SPEC(RETVAL,OBSERVER))
 #endif
 		ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret);
 
+		if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) {
+			if (zend_atomic_bool_load_ex(&EG(timed_out))) {
+				zend_timeout();
+			} else if (zend_interrupt_function) {
+				zend_interrupt_function(execute_data);
+			}
+		}
+
 		EG(current_execute_data) = execute_data;
 
 		ZEND_VM_C_GOTO(fcall_by_name_end);
@@ -4225,7 +4242,8 @@ ZEND_VM_C_LABEL(fcall_by_name_end):
 		zend_rethrow_exception(execute_data);
 		HANDLE_EXCEPTION();
 	}
-	ZEND_VM_SET_OPCODE(opline + 1);
+	CHECK_SYMBOL_TABLES()
+	OPLINE = opline + 1;
 	ZEND_VM_CONTINUE();
 }
 
@@ -4316,6 +4334,14 @@ ZEND_VM_HOT_HANDLER(60, ZEND_DO_FCALL, ANY, ANY, SPEC(RETVAL,OBSERVER))
 #endif
 		ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret);
 
+		if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) {
+			if (zend_atomic_bool_load_ex(&EG(timed_out))) {
+				zend_timeout();
+			} else if (zend_interrupt_function) {
+				zend_interrupt_function(execute_data);
+			}
+		}
+
 		EG(current_execute_data) = execute_data;
 
 		ZEND_VM_C_GOTO(fcall_end);
@@ -4344,7 +4370,8 @@ ZEND_VM_C_LABEL(fcall_end):
 		HANDLE_EXCEPTION();
 	}
 
-	ZEND_VM_SET_OPCODE(opline + 1);
+	CHECK_SYMBOL_TABLES()
+	OPLINE = opline + 1;
 	ZEND_VM_CONTINUE();
 }