[GR-45323] [GR-45310] [GR-45265] Backport fixes for native extensions and tox

timfel · timfel · commit 55449f57b4ff · 2023-04-03T07:51:15.000Z
PullRequest: graalpython/2716
diff --git a/graalpython/com.oracle.graal.python.cext/src/typeobject.c b/graalpython/com.oracle.graal.python.cext/src/typeobject.c
@@ -178,34 +178,19 @@ void PyType_Modified(PyTypeObject* type) {
 static void inherit_special(PyTypeObject *type, PyTypeObject *base) {
 
     /* Copying basicsize is connected to the GC flags */
-	unsigned long flags = PyTypeObject_tp_flags(type);
-	unsigned long base_flags = PyTypeObject_tp_flags(base);
+    unsigned long flags = PyTypeObject_tp_flags(type);
+    unsigned long base_flags = PyTypeObject_tp_flags(base);
     if (!(flags & Py_TPFLAGS_HAVE_GC) &&
         (base_flags & Py_TPFLAGS_HAVE_GC) &&
         (!PyTypeObject_tp_traverse(type) && !PyTypeObject_tp_clear(type))) {
-    	flags |= Py_TPFLAGS_HAVE_GC;
+
+        flags |= Py_TPFLAGS_HAVE_GC;
         if (PyTypeObject_tp_traverse(type) == NULL)
             set_PyTypeObject_tp_traverse(type, PyTypeObject_tp_traverse(base));
         if (PyTypeObject_tp_clear(type) == NULL)
-        	set_PyTypeObject_tp_clear(type, PyTypeObject_tp_clear(base));
-    }
-    {
-        /* The condition below could use some explanation.
-           It appears that tp_new is not inherited for static types
-           whose base class is 'object'; this seems to be a precaution
-           so that old extension types don't suddenly become
-           callable (object.__new__ wouldn't insure the invariants
-           that the extension type's own factory function ensures).
-           Heap types, of course, are under our control, so they do
-           inherit tp_new; static extension types that specify some
-           other built-in type as the default also
-           inherit object.__new__. */
-        if (base != &PyBaseObject_Type ||
-            (flags & Py_TPFLAGS_HEAPTYPE)) {
-            if (PyTypeObject_tp_new(type) == NULL)
-            	set_PyTypeObject_tp_new(type, PyTypeObject_tp_new(base)) ;
-        }
+            set_PyTypeObject_tp_clear(type, PyTypeObject_tp_clear(base));
     }
+
     /* Copy other non-function slots */
 
 #undef COPYVAL
@@ -238,7 +223,7 @@ static void inherit_special(PyTypeObject *type, PyTypeObject *base) {
         flags |= _Py_TPFLAGS_MATCH_SELF;
 
     if (flags != PyTypeObject_tp_flags(type)) {
-    	set_PyTypeObject_tp_flags(type, flags);
+        set_PyTypeObject_tp_flags(type, flags);
     }
 }
 
@@ -372,13 +357,55 @@ static void add_slot(PyTypeObject* cls, PyObject* type_dict, char* name, void* m
     }
 }
 
-#define ADD_MEMBER(__javacls__, __tpdict__, __mname__, __mtype__, __moffset__, __mflags__, __mdoc__)     \
+#define ADD_MEMBER(__javacls__, __tpdict__, __mname__, __mtype__, __moffset__, __mflags__, __mdoc__) \
     add_member((__javacls__), (__tpdict__), (__mname__), (__mtype__), (__moffset__), (__mflags__), (__mdoc__))
 
 
-#define ADD_GETSET(__javacls__, __tpdict__, __name__, __getter__, __setter__, __doc__, __closure__)     \
+#define ADD_GETSET(__javacls__, __tpdict__, __name__, __getter__, __setter__, __doc__, __closure__) \
     add_getset((__javacls__), (__tpdict__), (__name__), (__getter__), (__setter__), (__doc__), (__closure__))
 
+// Set tp_new and the "__new__" key in the type dictionary.
+// Use the Py_TPFLAGS_DISALLOW_INSTANTIATION flag.
+static int
+type_ready_set_new(PyTypeObject *type, PyObject *dict, PyTypeObject *base)
+{
+    /* The condition below could use some explanation.
+
+       It appears that tp_new is not inherited for static types whose base
+       class is 'object'; this seems to be a precaution so that old extension
+       types don't suddenly become callable (object.__new__ wouldn't insure the
+       invariants that the extension type's own factory function ensures).
+
+       Heap types, of course, are under our control, so they do inherit tp_new;
+       static extension types that specify some other built-in type as the
+       default also inherit object.__new__. */
+    newfunc tp_new = PyTypeObject_tp_new(type);
+    unsigned long tp_flags = PyTypeObject_tp_flags(type);
+    if (tp_new == NULL
+        && base == &PyBaseObject_Type
+        && !(tp_flags & Py_TPFLAGS_HEAPTYPE))
+    {
+        set_PyTypeObject_tp_flags(type, tp_flags |= Py_TPFLAGS_DISALLOW_INSTANTIATION);
+    }
+
+    if (!(type->tp_flags & Py_TPFLAGS_DISALLOW_INSTANTIATION)) {
+        if (tp_new != NULL) {
+            // If "__new__" key does not exists in the type dictionary,
+            // set it to tp_new_wrapper().
+            add_slot(type, dict, "__new__", tp_new, METH_KEYWORDS | METH_VARARGS, JWRAPPER_NEW, NULL);
+        }
+        else {
+            // tp_new is NULL: inherit tp_new from base
+            set_PyTypeObject_tp_new(type, PyTypeObject_tp_new(base)) ;
+        }
+    }
+    else {
+        // Py_TPFLAGS_DISALLOW_INSTANTIATION sets tp_new to NULL
+        // not supported yet
+        // set_PyTypeObject_tp_new(type, NULL) ;
+    }
+    return 0;
+}
 
 int PyType_Ready(PyTypeObject* cls) {
 #define RETURN_ERROR(__type__) \
@@ -388,7 +415,7 @@ int PyType_Ready(PyTypeObject* cls) {
         return -1; \
 	} while(0)
 
-#define ADD_IF_MISSING(attr, def) if (!(attr)) { attr = def; }
+#define ADD_IF_MISSING(OBJ, SLOT, VAL) if (!(PyTypeObject_##SLOT(OBJ))) { set_PyTypeObject_##SLOT((OBJ), (VAL)); }
 #define ADD_SLOT_CONV(__name__, __meth__, __flags__, __signature__) add_slot(cls, dict, (__name__), (__meth__), (__flags__), (__signature__), NULL)
 
     Py_ssize_t n;
@@ -507,23 +534,15 @@ int PyType_Ready(PyTypeObject* cls) {
     PyObject* mro = GraalPyTruffle_Compute_Mro(cls, truffleString(cls->tp_name));
     set_PyTypeObject_tp_mro(cls, mro);
 
-    /* Inherit special flags from dominant base */
-    if (base != NULL)
-        inherit_special(cls, base);
+    /* set new */
+    type_ready_set_new(cls, dict, base);
 
-    /* Initialize tp_dict properly */
-    bases = mro;
-    assert(bases != NULL);
-    assert(PyTuple_Check(bases));
-    n = PyTuple_GET_SIZE(bases);
-    for (i = 1; i < n; i++) {
-        PyObject *b = PyTuple_GET_ITEM(bases, i);
-        if (PyType_Check(b))
-            inherit_slots(cls, (PyTypeObject *)b);
-    }
+    /* fill dict */
 
-    ADD_IF_MISSING(cls->tp_alloc, PyType_GenericAlloc);
-    ADD_IF_MISSING(cls->tp_new, PyType_GenericNew);
+    /*
+     * NOTE: ADD_SLOT_CONV won't overwrite existing attributes, so the order is crucial and must
+     * reflect CPython's 'slotdefs' array.
+     */
 
     // add special methods defined directly on the type structs
     ADD_SLOT_CONV("__dealloc__", cls->tp_dealloc, -1, JWRAPPER_DIRECT);
@@ -563,25 +582,14 @@ int PyType_Ready(PyTypeObject* cls) {
     ADD_SLOT_CONV("__set__", cls->tp_descr_set, -3, JWRAPPER_DESCR_SET);
     ADD_SLOT_CONV("__init__", cls->tp_init, METH_KEYWORDS | METH_VARARGS, JWRAPPER_INITPROC);
     ADD_SLOT_CONV("__alloc__", cls->tp_alloc, -2, JWRAPPER_ALLOC);
-    ADD_SLOT_CONV("__new__", cls->tp_new, METH_KEYWORDS | METH_VARARGS, JWRAPPER_NEW);
+    /* Note: '__new__' was added here previously but we don't do it similar to CPython.
+       They also skip it because the appropriate 'slotdef' doesn't have a wrapper.
+       Adding '__new__' is done by function 'type_ready_set_new'. */
     ADD_SLOT_CONV("__free__", cls->tp_free, -1, JWRAPPER_DIRECT);
     ADD_SLOT_CONV("__del__", cls->tp_del, -1, JWRAPPER_DIRECT);
     ADD_SLOT_CONV("__finalize__", cls->tp_finalize, -1, JWRAPPER_DIRECT);
 
-    PySequenceMethods* sequences = PyTypeObject_tp_as_sequence(cls);
-    if (sequences) {
-    	// sequence functions first, so that the number functions take precendence
-        ADD_SLOT_CONV("__len__", sequences->sq_length, -1, JWRAPPER_LENFUNC);
-        ADD_SLOT_CONV("__add__", sequences->sq_concat, -2, JWRAPPER_BINARYFUNC);
-        ADD_SLOT_CONV("__mul__", sequences->sq_repeat, -2, JWRAPPER_SSIZE_ARG);
-        ADD_SLOT_CONV("__getitem__", sequences->sq_item, -2, JWRAPPER_GETITEM);
-        ADD_SLOT_CONV("__setitem__", sequences->sq_ass_item, -3, JWRAPPER_SETITEM);
-        ADD_SLOT_CONV("__delitem__", sequences->sq_ass_item, -3, JWRAPPER_DELITEM);
-        ADD_SLOT_CONV("__contains__", sequences->sq_contains, -2, JWRAPPER_OBJOBJPROC);
-        ADD_SLOT_CONV("__iadd__", sequences->sq_inplace_concat, -2, JWRAPPER_BINARYFUNC);
-        ADD_SLOT_CONV("__imul__", sequences->sq_inplace_repeat, -2, JWRAPPER_SSIZE_ARG);
-    }
-
+    // 'tp_as_number' takes precedence over 'tp_as_mapping' and 'tp_as_sequence' !
     PyNumberMethods* numbers = PyTypeObject_tp_as_number(cls);
     if (numbers) {
         ADD_SLOT_CONV("__add__", numbers->nb_add, -2, JWRAPPER_BINARYFUNC_L);
@@ -635,6 +643,7 @@ int PyType_Ready(PyTypeObject* cls) {
         ADD_SLOT_CONV("__imatmul__", numbers->nb_inplace_matrix_multiply, -2, JWRAPPER_BINARYFUNC_L);
     }
 
+    // 'tp_as_mapping' takes precedence over 'tp_as_sequence' !
     PyMappingMethods* mappings = PyTypeObject_tp_as_mapping(cls);
     if (mappings) {
         ADD_SLOT_CONV("__len__", mappings->mp_length, -1, JWRAPPER_LENFUNC);
@@ -643,6 +652,20 @@ int PyType_Ready(PyTypeObject* cls) {
         ADD_SLOT_CONV("__delitem__", mappings->mp_ass_subscript, -3, JWRAPPER_MP_DELITEM);
     }
 
+    PySequenceMethods* sequences = PyTypeObject_tp_as_sequence(cls);
+    if (sequences) {
+        // sequence functions first, so that the number functions take precendence
+        ADD_SLOT_CONV("__len__", sequences->sq_length, -1, JWRAPPER_LENFUNC);
+        ADD_SLOT_CONV("__add__", sequences->sq_concat, -2, JWRAPPER_BINARYFUNC);
+        ADD_SLOT_CONV("__mul__", sequences->sq_repeat, -2, JWRAPPER_SSIZE_ARG);
+        ADD_SLOT_CONV("__getitem__", sequences->sq_item, -2, JWRAPPER_GETITEM);
+        ADD_SLOT_CONV("__setitem__", sequences->sq_ass_item, -3, JWRAPPER_SETITEM);
+        ADD_SLOT_CONV("__delitem__", sequences->sq_ass_item, -3, JWRAPPER_DELITEM);
+        ADD_SLOT_CONV("__contains__", sequences->sq_contains, -2, JWRAPPER_OBJOBJPROC);
+        ADD_SLOT_CONV("__iadd__", sequences->sq_inplace_concat, -2, JWRAPPER_BINARYFUNC);
+        ADD_SLOT_CONV("__imul__", sequences->sq_inplace_repeat, -2, JWRAPPER_SSIZE_ARG);
+    }
+
     PyAsyncMethods* async = PyTypeObject_tp_as_async(cls);
     if (async) {
         ADD_SLOT_CONV("__await__", async->am_await, -1, JWRAPPER_DIRECT);
@@ -655,6 +678,22 @@ int PyType_Ready(PyTypeObject* cls) {
         // TODO ...
     }
 
+    /* Inherit slots */
+    if (base != NULL)
+        inherit_special(cls, base);
+    bases = mro;
+    assert(bases != NULL);
+    assert(PyTuple_Check(bases));
+    n = PyTuple_GET_SIZE(bases);
+    for (i = 1; i < n; i++) {
+        PyObject *b = PyTuple_GET_ITEM(bases, i);
+        if (PyType_Check(b))
+            inherit_slots(cls, (PyTypeObject *)b);
+    }
+
+    ADD_IF_MISSING(cls, tp_alloc, PyType_GenericAlloc);
+    ADD_IF_MISSING(cls, tp_new, PyType_GenericNew);
+
     // process inherited slots
     // CPython doesn't do that in 'PyType_Ready' but we must because a native type can inherit
     // dynamic slots from a managed Python class. Since the managed Python class may be created
diff --git a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_object.py b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_object.py
@@ -731,6 +731,53 @@ def test_doc(self):
         assert len(obj.some_member.__doc__) == len(expected_doc)
         assert obj.some_member.__doc__ == expected_doc
 
+    def test_multiple_inheritance_with_native(self):
+        _A = CPyExtType("_A","")
+        class B:
+            def __getattr__(self, name):
+                return name
+        class X(_A, B):
+            b = 2
+        x = X()
+        assert x.foo == "foo"
+
+    def test_slot_precedence(self):
+        MapAndSeq = CPyExtType("MapAndSeq",
+                               '''
+                               static PyObject * mas_nb_add(PyObject *self, PyObject *other) {
+                                   return PyUnicode_FromString("mas_nb_add");
+                               }
+                               static Py_ssize_t mas_sq_length(PyObject *self) {
+                                   return 111;
+                               }
+                               static PyObject *mas_sq_item(PyObject *self, Py_ssize_t idx) {
+                                   return PyUnicode_FromString("sq_item");
+                               }
+                               static PyObject * mas_sq_concat(PyObject *self, PyObject *other) {
+                                   return PyUnicode_FromString("mas_sq_concat");
+                               }
+                               static Py_ssize_t mas_mp_length(PyObject *self) {
+                                   return 222;
+                               }
+                               static PyObject * mas_mp_subscript(PyObject *self, PyObject *key) {
+                                   return PyUnicode_FromString("mp_subscript");
+                               }
+                               ''',
+                               nb_add='mas_nb_add',
+                               sq_length='mas_sq_length',
+                               sq_item='mas_sq_item',
+                               sq_concat='mas_sq_concat',
+                               mp_length='mas_mp_length',
+                               mp_subscript='mas_mp_subscript',
+                               )
+        obj = MapAndSeq()
+        # Note: len(obj) uses 'PyObject_Lenght' which does not use the attribute but first tries
+        # 'sq_length' and falls back to 'mp_length'. Therefore, we just look at '__len__' here.
+        assert obj.__len__() == 222
+        assert obj['hello'] == 'mp_subscript'
+        assert obj + 'hello' == 'mas_nb_add'
+
+
 class CBytes: 
     def __bytes__(self):
         return b'abc'
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PosixModuleBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PosixModuleBuiltins.java
@@ -1900,6 +1900,30 @@ boolean access(PosixPath path, int mode, int dirFd, boolean effectiveIds, boolea
         }
     }
 
+    @Builtin(name = "fchmod", minNumOfPositionalArgs = 2, parameterNames = {"fd", "mode"})
+    @ArgumentClinic(name = "fd", conversion = ClinicConversion.Int)
+    @ArgumentClinic(name = "mode", conversion = ClinicConversion.Int)
+    @GenerateNodeFactory
+    abstract static class FChmodNode extends PythonBinaryClinicBuiltinNode {
+        @Override
+        protected ArgumentClinicProvider getArgumentClinic() {
+            return PosixModuleBuiltinsClinicProviders.FChmodNodeClinicProviderGen.INSTANCE;
+        }
+
+        @Specialization
+        PNone fchmod(VirtualFrame frame, int fd, int mode,
+                        @Cached SysModuleBuiltins.AuditNode auditNode,
+                        @CachedLibrary("getPosixSupport()") PosixSupportLibrary posixLib) {
+            auditNode.audit("os.chmod", fd, mode, -1);
+            try {
+                posixLib.fchmod(getPosixSupport(), fd, mode);
+            } catch (PosixException e) {
+                throw raiseOSErrorFromPosixException(frame, e, fd);
+            }
+            return PNone.NONE;
+        }
+    }
+
     @Builtin(name = "chmod", minNumOfPositionalArgs = 2, parameterNames = {"path", "mode"}, varArgsMarker = true, keywordOnlyNames = {"dir_fd", "follow_symlinks"})
     @ArgumentClinic(name = "path", conversionClass = PathConversionNode.class, args = {"false", "true"})
     @ArgumentClinic(name = "mode", conversion = ClinicConversion.Int)
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextTypeBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextTypeBuiltins.java
@@ -80,6 +80,7 @@
 import com.oracle.graal.python.builtins.objects.cext.common.CExtContext;
 import com.oracle.graal.python.builtins.objects.cext.common.CExtContext.Store;
 import com.oracle.graal.python.builtins.objects.common.DynamicObjectStorage;
+import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageGetItem;
 import com.oracle.graal.python.builtins.objects.dict.DictBuiltins;
 import com.oracle.graal.python.builtins.objects.dict.PDict;
 import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
@@ -292,12 +293,14 @@ abstract static class PyTruffleType_AddSlot extends CApi7BuiltinNode {
         @Specialization
         @TruffleBoundary
         static int addSlot(Object clazz, PDict tpDict, TruffleString memberName, Object cfunc, int flags, int wrapper, Object memberDoc) {
-            // create wrapper descriptor
-            Object wrapperDescriptor = CreateFunctionNodeGen.getUncached().execute(memberName, cfunc, wrapper, clazz, flags, PythonObjectFactory.getUncached());
-            WriteAttributeToDynamicObjectNode.getUncached().execute(wrapperDescriptor, SpecialAttributeNames.T___DOC__, memberDoc);
+            if (!HashingStorageGetItem.hasKeyUncached(tpDict.getDictStorage(), memberName)) {
+                // create wrapper descriptor
+                Object wrapperDescriptor = CreateFunctionNodeGen.getUncached().execute(memberName, cfunc, wrapper, clazz, flags, PythonObjectFactory.getUncached());
+                WriteAttributeToDynamicObjectNode.getUncached().execute(wrapperDescriptor, SpecialAttributeNames.T___DOC__, memberDoc);
 
-            // add wrapper descriptor to tp_dict
-            PyDictSetItem.executeUncached(tpDict, memberName, wrapperDescriptor);
+                // add wrapper descriptor to tp_dict
+                PyDictSetItem.executeUncached(tpDict, memberName, wrapperDescriptor);
+            }
             return 0;
         }
     }
