[GR-53368] Update autopatch_capi to deal with more usages of m_ml and m_module access.

timfel · timfel · commit 8e8887942038 · 2024-04-24T02:26:53.000Z
PullRequest: graalpython/3303
diff --git a/graalpython/com.oracle.graal.python.cext/include/cpython/methodobject.h b/graalpython/com.oracle.graal.python.cext/include/cpython/methodobject.h
@@ -71,9 +71,10 @@ static inline PyTypeObject* PyCFunction_GET_CLASS(PyObject *func_obj) {
 #endif
 
 /*
- * XXX These functions are GraalPy-only. We need them to replace field access in our patches.
- * Currently used by (at least) cffi patch.
+ * XXX These functions are GraalPy-only. We need them to replace field access.
+ * Currently inserted by our autopatch_capi.py
  */
 PyAPI_FUNC(PyObject*) _PyCFunction_GetModule(PyObject* a);
 PyAPI_FUNC(PyMethodDef*) _PyCFunction_GetMethodDef(PyObject* a);
-
+PyAPI_FUNC(void) _PyCFunction_SetModule(PyObject* a, PyObject* b);
+PyAPI_FUNC(void) _PyCFunction_SetMethodDef(PyObject* a, PyMethodDef *b);
diff --git a/graalpython/com.oracle.graal.python.cext/src/methodobject.c b/graalpython/com.oracle.graal.python.cext/src/methodobject.c
@@ -94,3 +94,11 @@ PyObject* _PyCFunction_GetModule(PyObject *func) {
 PyMethodDef* _PyCFunction_GetMethodDef(PyObject *func) {
     return PyCFunctionObject_m_ml(func);
 }
+
+void _PyCFunction_SetModule(PyObject *func, PyObject *mod) {
+    set_PyCFunctionObject_m_module(func, mod);
+}
+
+void _PyCFunction_SetMethodDef(PyObject *func, PyMethodDef *def) {
+    set_PyCFunctionObject_m_ml(func, def);
+}
diff --git a/graalpython/com.oracle.graal.python.processor/src/com/oracle/graal/python/processor/CApiBuiltinsProcessor.java b/graalpython/com.oracle.graal.python.processor/src/com/oracle/graal/python/processor/CApiBuiltinsProcessor.java
@@ -749,6 +749,7 @@ private static Path resolvePath(Path path) {
     private static final String[] ADDITIONAL = new String[]{"PyCMethod_GetClass", "PyDescrObject_GetName", "PyDescrObject_GetType", "PyInterpreterState_GetIDFromThreadState",
                     "PyMethodDescrObject_GetMethod", "PyObject_GetDoc", "PyObject_SetDoc", "PySlice_Start", "PySlice_Step", "PySlice_Stop", "_PyFrame_SetLineNumber",
                     "_PyCFunction_GetModule", "_PyCFunction_GetMethodDef", "PyCode_GetName",
+                    "_PyCFunction_SetModule", "_PyCFunction_SetMethodDef",
                     "PyCode_GetFileName", "_PyArray_Resize", "_PyArray_Data",
                     "_PyErr_Occurred", "_PyNamespace_New", "_Py_GetErrorHandler",
     };
diff --git a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_functions.py b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_functions.py
@@ -537,6 +537,9 @@ def test_PyMethodDef(self):
             // CPython and other don't have this function; so define it
             #ifndef GRAALVM_PYTHON
             #define _PyCFunction_GetMethodDef(OBJ) (((PyCFunctionObject*) (OBJ))->m_ml)
+            #define _PyCFunction_SetMethodDef(OBJ, VAL) (((PyCFunctionObject*) (OBJ))->m_ml = (VAL))
+            #define _PyCFunction_GetModule(OBJ) (((PyCFunctionObject*) (OBJ))->m_module)
+            #define _PyCFunction_SetModule(OBJ, VAL) (((PyCFunctionObject*) (OBJ))->m_module = (VAL))
             #define PyMethodDescrObject_GetMethod(OBJ) (((PyMethodDescrObject *) (OBJ))->d_method)
             #endif
 
@@ -680,6 +683,32 @@ def test_PyMethodDef(self):
                 return PyUnicode_FromString(def->ml_doc);
             }
 
+            static PyObject *set_def(PyObject *self, PyObject *arg) {
+                if (!PyCFunction_Check(arg)) {
+                    PyErr_SetString(PyExc_TypeError, "<callable> is not a PyCFunction (i.e. builtin_method_or_function)");
+                    return NULL;
+                }
+                PyMethodDef *def = _PyCFunction_GetMethodDef(arg);
+                _PyCFunction_SetMethodDef(arg, def);
+                return PyUnicode_FromString(_PyCFunction_GetMethodDef(arg)->ml_doc);
+            }
+
+            static PyObject *get_set_module(PyObject *self, PyObject *arg) {
+                if (!PyCFunction_Check(arg)) {
+                    PyErr_SetString(PyExc_TypeError, "<callable> is not a PyCFunction (i.e. builtin_method_or_function)");
+                    return NULL;
+                }
+                PyObject *module = _PyCFunction_GetModule(arg);
+                Py_XINCREF(self);
+                _PyCFunction_SetModule(arg, self);
+                if (_PyCFunction_GetModule(arg) != self) {
+                    PyErr_SetString(PyExc_TypeError, "module of function is not self");
+                    return NULL;
+                }
+                Py_XINCREF(self);
+                return self;
+            }
+
             static PyObject *new_meth(PyObject *self, PyObject *args) {
                 PyObject *callable_type = NULL, *callable_self, *callable;
                 if (!PyArg_ParseTuple(args, "OO:new_meth",
@@ -711,7 +740,9 @@ def = PyMethodDescrObject_GetMethod(callable);
             {"call_meth", (PyCFunction)call_meth, METH_VARARGS, ""},
             {"call_meth_descr", (PyCFunction)call_meth_descr, METH_VARARGS, ""},
             {"get_doc", (PyCFunction)get_doc, METH_O, ""},
-            {"new_meth", (PyCFunction)new_meth, METH_VARARGS, ""}
+            {"new_meth", (PyCFunction)new_meth, METH_VARARGS, ""},
+            {"set_def", (PyCFunction)set_def, METH_O, ""},
+            {"get_set_module", (PyCFunction)get_set_module, METH_O, ""}
             ''',
             post_ready_code='''
             PyModule_AddIntMacro(m, METH_NOARGS);
@@ -738,6 +769,8 @@ def = PyMethodDescrObject_GetMethod(callable);
         assert tester.get_flags(tester.native_meth_keywords) == m.METH_VARARGS | m.METH_KEYWORDS
         assert tester.get_doc(tester.native_meth_keywords) == "doc keywords"
         assert tester.call_meth(tester.native_meth_keywords, (1, 2), {"hello": "world"}) == ((1, 2), {"hello": "world"})
+        assert tester.set_def(tester.native_meth_keywords) == "doc keywords"
+        assert tester.get_set_module(tester.get_set_module) == tester
 
         # built-in functions
 
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextSlotBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextSlotBuiltins.java
@@ -145,6 +145,7 @@
 import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
 import com.oracle.graal.python.builtins.objects.type.TypeNodes;
 import com.oracle.graal.python.lib.PyObjectLookupAttr;
+import com.oracle.graal.python.lib.PyObjectSetAttr;
 import com.oracle.graal.python.nodes.HiddenAttr;
 import com.oracle.graal.python.nodes.PGuards;
 import com.oracle.graal.python.nodes.SpecialAttributeNames;
@@ -313,6 +314,26 @@ static Object get(PythonBuiltinObject object,
         }
     }
 
+    @CApiBuiltin(ret = PyMethodDef, args = {PyCFunctionObject, PyMethodDef}, call = Ignored)
+    abstract static class Py_set_PyCFunctionObject_m_ml extends CApiBinaryBuiltinNode {
+        @Specialization
+        static Object get(PythonBuiltinObject object, Object methodDefPtr,
+                        @Bind("this") Node inliningTarget,
+                        @Cached HiddenAttr.WriteNode writeNode) {
+            PBuiltinFunction resolved;
+            if (object instanceof PBuiltinMethod builtinMethod) {
+                resolved = builtinMethod.getBuiltinFunction();
+            } else if (object instanceof PBuiltinFunction builtinFunction) {
+                resolved = builtinFunction;
+            } else {
+                CompilerDirectives.transferToInterpreterAndInvalidate();
+                throw CompilerDirectives.shouldNotReachHere("writing PyMethodDef for an incompatible function/method type: " + object.getClass().getSimpleName());
+            }
+            writeNode.execute(inliningTarget, resolved, METHOD_DEF_PTR, methodDefPtr);
+            return PNone.NO_VALUE;
+        }
+    }
+
     @CApiBuiltin(ret = PyObjectBorrowed, args = {PyCFunctionObject}, call = Ignored)
     abstract static class Py_get_PyCFunctionObject_m_module extends CApiUnaryBuiltinNode {
         @Specialization
@@ -324,6 +345,17 @@ Object get(Object object,
         }
     }
 
+    @CApiBuiltin(ret = Void, args = {PyCFunctionObject, PyObjectBorrowed}, call = Ignored)
+    abstract static class Py_set_PyCFunctionObject_m_module extends CApiBinaryBuiltinNode {
+        @Specialization
+        Object set(Object object, Object value,
+                        @Bind("this") Node inliningTarget,
+                        @Cached PyObjectSetAttr setattr) {
+            setattr.execute(null, inliningTarget, object, T___MODULE__, value);
+            return PNone.NO_VALUE;
+        }
+    }
+
     @CApiBuiltin(ret = PyObjectBorrowed, args = {PyCFunctionObject}, call = Ignored)
     abstract static class Py_get_PyCFunctionObject_m_self extends CApiUnaryBuiltinNode {
         @Specialization
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiFunction.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiFunction.java
@@ -541,6 +541,8 @@ public final class CApiFunction {
     @CApiBuiltin(name = "_PyBytes_Resize", ret = Int, args = {PyObjectPtr, Py_ssize_t}, call = CImpl)
     @CApiBuiltin(name = "_PyCFunction_GetMethodDef", ret = PyMethodDef, args = {PyObject}, call = CImpl)
     @CApiBuiltin(name = "_PyCFunction_GetModule", ret = PyObject, args = {PyObject}, call = CImpl)
+    @CApiBuiltin(name = "_PyCFunction_SetMethodDef", ret = Void, args = {PyObject, PyMethodDef}, call = CImpl)
+    @CApiBuiltin(name = "_PyCFunction_SetModule", ret = Void, args = {PyObject, PyObject}, call = CImpl)
     @CApiBuiltin(name = "_PyDict_ContainsId", ret = Int, args = {PyObject, _PY_IDENTIFIER_PTR}, call = CImpl)
     @CApiBuiltin(name = "_PyDict_GetItemIdWithError", ret = PyObject, args = {PyObject, _PY_IDENTIFIER_PTR}, call = CImpl)
     @CApiBuiltin(name = "_PyDict_GetItemStringWithError", ret = PyObject, args = {PyObject, ConstCharPtrAsTruffleString}, call = CImpl)
diff --git a/graalpython/lib-graalpython/modules/autopatch_capi.py b/graalpython/lib-graalpython/modules/autopatch_capi.py
@@ -157,8 +157,10 @@ def consume_whitespace_forward(idx):
                     r'\W(ob_item)\W': (replace_field_access, 'PySequence_Fast_ITEMS((PyObject*)%receiver)'),
                     r'^\s*()(std::)?free\((const_cast<char \*>)?\(?\w+->m_ml->ml_doc\)?\);': (simple_replace, '//'),
                     r'\W(m_ml\s*->\s*ml_doc)\W': (replace_field_access, 'PyObject_GetDoc((PyObject*)(%receiver))', 'PyObject_SetDoc((PyObject*)(%receiver), %value)'),
-                    r'\W(m_ml)\W': (replace_field_access, '_PyCFunction_GetMethodDef((PyObject*)(%receiver))'),
-                    r'\W(m_module)\W': (replace_field_access, '_PyCFunction_GetModule((PyObject*)(%receiver))'),
+                    # Py_CLEAR/Py_VISIT on a function's module is skipped for us, Java GC takes care of it
+                    r'(Py_(?:CLEAR|VISIT)\((?:(?:\(?\([a-zA-Z0-9_]|[a-zA-Z0-9_])(?:[a-zA-Z0-9_]|\)|\*\)|->)*)->m_module\);)': (simple_replace, ''),
+                    r'\W(m_ml)\W': (replace_field_access, '_PyCFunction_GetMethodDef((PyObject*)(%receiver))', '_PyCFunction_SetMethodDef((PyObject*)(%receiver), %value)'),
+                    r'\W(m_module)\W': (replace_field_access, '_PyCFunction_GetModule((PyObject*)(%receiver))', '_PyCFunction_SetModule((PyObject*)(%receiver), %value)'),
                     r'(&PyTuple_GET_ITEM\(([\(\w](?:\w|->|\.|\(|\))*), 0\))': (simple_replace, r'PySequence_Fast_ITEMS(\2)'),
                     # already defined by GraalPy:
                     r'^\s*()#\s*define\s+Py_SET_TYPE\W': (simple_replace, '//'),
