From 95a09ea5b2269e422e0b72147c4cad9472936059 Mon Sep 17 00:00:00 2001
From: Mikhail Kornaukhov <66915223+mkornaukhov03@users.noreply.github.com>
Date: Thu, 16 Jan 2025 19:33:59 +0300
Subject: [PATCH] Fix usage of resumable function with ArrayAccess methods
 (#1198)

---
 compiler/pipes/calc-bad-vars.cpp              | 29 +++++++++++++---
 .../014_another_method_forked.php             | 34 +++++++++++++++++++
 .../phpt/array_access/103_calls_resumable.php | 29 ++++++++++++++++
 tests/phpt/array_access/104_forked.php        | 24 +++++++++++++
 4 files changed, 111 insertions(+), 5 deletions(-)
 create mode 100644 tests/phpt/array_access/014_another_method_forked.php
 create mode 100644 tests/phpt/array_access/103_calls_resumable.php
 create mode 100644 tests/phpt/array_access/104_forked.php

diff --git a/compiler/pipes/calc-bad-vars.cpp b/compiler/pipes/calc-bad-vars.cpp
index 1b761812bb..bd0331ea1c 100644
--- a/compiler/pipes/calc-bad-vars.cpp
+++ b/compiler/pipes/calc-bad-vars.cpp
@@ -4,7 +4,6 @@
 
 #include "compiler/pipes/calc-bad-vars.h"
 
-#include "compiler/kphp_assert.h"
 #include <algorithm>
 #include <queue>
 #include <vector>
@@ -13,6 +12,7 @@
 #include "compiler/data/class-data.h"
 #include "compiler/data/src-file.h"
 #include "compiler/function-pass.h"
+#include "compiler/kphp_assert.h"
 #include "compiler/pipes/calc-func-dep.h"
 #include "compiler/utils/idmap.h"
 
@@ -549,10 +549,10 @@ class CalcBadVars {
     for (const auto &func : call_graph.functions) {
       if (into_interruptible[func]) {
         func->is_interruptible = true;
-        if (unlikely(func->class_id && func->class_id->is_required_interface)) {
-          std::string func_name = func->name;
-          std::replace(func_name.begin(), func_name.end(), '$', ':');
-          kphp_error(false, fmt_format("{} cannot be interruptible", func_name).c_str());
+        if (unlikely(func->class_id && func->class_id == G->get_class("ArrayAccess"))) {
+          kphp_error(false, fmt_format("Function [{}] is a method of ArrayAccess, it cannot call interruptible functions\n"
+                                       "Function transitively calls the interruptible function along the following chain:\n{}\n",
+                                       func->as_human_readable(), get_call_path(func, to_parents)));
         }
       }
     }
@@ -591,7 +591,13 @@ class CalcBadVars {
         func->wait_prev = to_parents[func];
       }
     }
+
     for (const auto &func : call_graph.functions) {
+      if (func->class_id && func->class_id == G->get_class("ArrayAccess") && func->can_be_implicitly_interrupted_by_other_resumable) {
+        kphp_error(false, fmt_format("Function [{}] is a method of ArrayAccess, it cannot call resumable functions\n"
+                                     "Function transitively calls the resumable function along the following chain:\n{}\n",
+                                     func->as_human_readable(), get_call_path(func, to_parents)));
+      }
       if (func->is_resumable) {
         if (func->should_be_sync) {
           kphp_error (0, fmt_format("Function [{}] marked with @kphp-sync, but turn up to be resumable\n"
@@ -637,6 +643,19 @@ class CalcBadVars {
     my_unique(&main_dep);
   }
 
+  static std::string get_call_path(FunctionPtr func, const IdMap<FunctionPtr> &to_parents) {
+    kphp_assert_msg(func, "Unexpected null FunctionPtr in getting call-chain");
+    std::vector<std::string> names;
+    names.push_back(TermStringFormat::paint(func->as_human_readable(), TermStringFormat::red));
+
+    func = to_parents[func];
+    while (func) {
+      names.push_back(func->as_human_readable());
+      func = to_parents[func];
+    }
+    return vk::join(names, " -> ");
+  };
+
   class MergeRefVarsCallback : public MergeReachalbeCallback<VarPtr> {
   private:
     const IdMap<std::vector<VarPtr>> &to_merge_;
diff --git a/tests/phpt/array_access/014_another_method_forked.php b/tests/phpt/array_access/014_another_method_forked.php
new file mode 100644
index 0000000000..d49225cffa
--- /dev/null
+++ b/tests/phpt/array_access/014_another_method_forked.php
@@ -0,0 +1,34 @@
+@ok
+<?php
+require_once 'kphp_tester_include.php';
+
+function like_rpc_write() {
+    echo "log\n";
+    sched_yield(); // resumable
+}
+
+class A implements \ArrayAccess {
+    public function offsetSet($offset, $value) {}
+    /** @return bool */
+    public function offsetExists($offset) {return true;}
+    public function offsetUnset($offset) {}
+    /** @return mixed */
+    public function offsetGet($offset) {
+        return null;
+    }
+
+    public function to_be_forked() {
+        echo "in to_be_forked\n";
+        like_rpc_write();
+        return null;
+    }
+}
+
+function test() {
+    $x = new A();
+
+    $fut = fork($x->to_be_forked());
+    wait($fut);
+}
+
+test();
diff --git a/tests/phpt/array_access/103_calls_resumable.php b/tests/phpt/array_access/103_calls_resumable.php
new file mode 100644
index 0000000000..8df7bded34
--- /dev/null
+++ b/tests/phpt/array_access/103_calls_resumable.php
@@ -0,0 +1,29 @@
+@kphp_should_fail
+/Function \[ArrayAccess::offsetGet\] is a method of ArrayAccess, it cannot call (resumable|interruptible) functions/
+/Function transitively calls the (resumable|interruptible) function along the following chain:/
+/ArrayAccess::offsetGet -> A::offsetGet -> like_rpc_write -> sched_yield/
+
+<?php
+
+function like_rpc_write() {
+    printf("log\n");
+    sched_yield(); // resumable
+}
+
+class A implements \ArrayAccess {
+    public function offsetSet($offset, $value) {}
+    /** @return bool */
+    public function offsetExists($offset) {return true;}
+    public function offsetUnset($offset) {}
+    /** @return mixed */
+    public function offsetGet($offset) {
+        like_rpc_write();
+        return null;
+    }
+}
+
+function test() {
+    $x = new A();
+}
+
+test();
diff --git a/tests/phpt/array_access/104_forked.php b/tests/phpt/array_access/104_forked.php
new file mode 100644
index 0000000000..5185c8942b
--- /dev/null
+++ b/tests/phpt/array_access/104_forked.php
@@ -0,0 +1,24 @@
+@kphp_should_fail
+/Function \[ArrayAccess::offsetGet\] is a method of ArrayAccess, it cannot call (resumable|interruptible) functions/
+/Function transitively calls the (resumable|interruptible) function along the following chain:/
+/ArrayAccess::offsetGet -> A::offsetGet/
+
+<?php
+
+class A implements \ArrayAccess {
+    public function offsetSet($offset, $value) {}
+    /** @return bool */
+    public function offsetExists($offset) {return true;}
+    public function offsetUnset($offset) {}
+    /** @return mixed */
+    public function offsetGet($offset) {return null;}
+}
+
+function test() {
+    $x = new A();
+    $y = fork($x->offsetGet(42));
+
+    wait($y);
+}
+
+test();