[GR-47042] Fix object.__new__ picking up wrong tp_alloc

msimacek · msimacek · commit 897faa2ca38d · 2023-07-07T09:07:45.000Z
PullRequest: graalpython/2858
diff --git a/graalpython/com.oracle.graal.python.cext/src/capi.c b/graalpython/com.oracle.graal.python.cext/src/capi.c
@@ -801,8 +801,8 @@ PyAPI_FUNC(void*) PyTruffle_AsciiToString(void* ptr) {
 /* To be used from Java code only.
  * This function is used if a native class inherits from a managed class but uses the 'object.__new__'.
  * This function roughly corresponds to CPython's 'object_new'. */
-PyAPI_FUNC(PyObject*) PyTruffle_Object_New(PyTypeObject* cls, PyTypeObject* dominatingNativeClass, PyObject* args, PyObject* kwds) {
-    return dominatingNativeClass->tp_alloc(cls, 0);
+PyAPI_FUNC(PyObject*) PyTruffle_Object_New(PyTypeObject* cls) {
+    return cls->tp_alloc(cls, 0);
 }
 
 PyAPI_FUNC(void) PyTruffle_PatchType(PyTypeObject* result, PyTypeObject* base) {
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
@@ -110,28 +110,30 @@ def test_int(self):
         assert int(tester) == 42
 
     def test_inherit_slots_with_managed_class(self):
-        TestTpAllocWithManaged = CPyExtType("TestTpAllocWithManaged",
-                             """
-                             static PyObject *test_alloc(PyTypeObject *type, Py_ssize_t nitems) {
-                                 PyErr_SetString(PyExc_RuntimeError, "Should not call this tp_alloc");
-                                 return NULL;
-                             }
-                             """,
-                             tp_alloc="test_alloc"
+        ClassWithTpAlloc = CPyExtType(
+            "ClassWithTpAlloc",
+            """
+            static PyObject *test_alloc(PyTypeObject *type, Py_ssize_t nitems) {
+                PyErr_SetString(PyExc_RuntimeError, "Should not call this tp_alloc");
+                return NULL;
+            }
+            static PyObject* testslots_tp_alloc(PyObject* self, PyObject *cls) {
+                return ((PyTypeObject *)cls)->tp_alloc((PyTypeObject *) cls, 0);
+            }
+            """,
+            tp_alloc="test_alloc",
+            tp_methods='{"call_tp_alloc", (PyCFunction)testslots_tp_alloc, METH_O | METH_STATIC, ""}',
+            tp_flags='Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HEAPTYPE | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC',
+            tp_free='PyObject_GC_Del',
+            ready_code="ClassWithTpAllocType.tp_new = PyBaseObject_Type.tp_new;"
         )
-        TestTpAllocCall = CPyExtType("TestTpAllocCall",
-                                '''
-                                static PyObject* testslots_tp_alloc(PyObject* self, PyObject *cls) {
-                                    return ((PyTypeObject *)cls)->tp_alloc((PyTypeObject *) cls, 0);
-                                }
-                                ''',
-                                tp_methods='{"createObj", (PyCFunction)testslots_tp_alloc, METH_O, ""}'
-                                )
-        class managed(TestTpAllocWithManaged):
+
+        class ManagedSubclass(ClassWithTpAlloc):
             pass
-        t = TestTpAllocCall()
 
-        assert t.createObj(managed) != None
+        assert ClassWithTpAlloc.call_tp_alloc(ManagedSubclass)
+        assert type(ManagedSubclass()) is ManagedSubclass
+        assert type(object.__new__(ManagedSubclass)) is ManagedSubclass
         
 
     def test_float_binops(self):
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java
@@ -123,12 +123,10 @@
 import com.oracle.graal.python.builtins.objects.cext.PythonNativeVoidPtr;
 import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes;
 import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.PCallCapiFunction;
-import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory;
 import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes;
 import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
 import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.NativeToPythonNode;
 import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.PythonToNativeNode;
-import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitionsFactory.NativeToPythonNodeGen;
 import com.oracle.graal.python.builtins.objects.cext.common.CArrayWrappers.CByteArrayWrapper;
 import com.oracle.graal.python.builtins.objects.code.CodeNodes;
 import com.oracle.graal.python.builtins.objects.code.PCode;
@@ -166,15 +164,13 @@
 import com.oracle.graal.python.builtins.objects.str.PString;
 import com.oracle.graal.python.builtins.objects.traceback.PTraceback;
 import com.oracle.graal.python.builtins.objects.tuple.PTuple;
-import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass;
 import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass;
 import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
 import com.oracle.graal.python.builtins.objects.type.SpecialMethodSlot;
 import com.oracle.graal.python.builtins.objects.type.TypeBuiltins;
 import com.oracle.graal.python.builtins.objects.type.TypeFlags;
 import com.oracle.graal.python.builtins.objects.type.TypeNodes;
 import com.oracle.graal.python.builtins.objects.type.TypeNodes.CreateTypeNode;
-import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetMroNode;
 import com.oracle.graal.python.builtins.objects.type.TypeNodes.IsAcceptableBaseNode;
 import com.oracle.graal.python.builtins.objects.type.TypeNodes.IsTypeNode;
 import com.oracle.graal.python.builtins.objects.types.PGenericAlias;
@@ -1803,13 +1799,9 @@ protected PList constructList(Object cls, @SuppressWarnings("unused") Object[] a
     @GenerateNodeFactory
     public abstract static class ObjectNode extends PythonVarargsBuiltinNode {
 
-        @Child private PCallCapiFunction callCapiFunction;
-        @Children private CExtNodes.ToSulongNode[] toSulongNodes;
-        @Child private NativeToPythonNode asPythonObjectNode;
         @Child private SplitArgsNode splitArgsNode;
         @Child private LookupCallableSlotInMRONode lookupInit;
         @Child private LookupCallableSlotInMRONode lookupNew;
-        @Child private ReportAbstractClassNode reportAbstractClassNode;
         @CompilationFinal private ValueProfile profileInit;
         @CompilationFinal private ValueProfile profileNew;
         @CompilationFinal private ValueProfile profileInitFactory;
@@ -1867,68 +1859,47 @@ Object doBuiltinTypeType(PythonBuiltinClassType self, Object[] varargs, PKeyword
         @Specialization(guards = "self.needsNativeAllocation()")
         Object doNativeObjectIndirect(VirtualFrame frame, PythonManagedClass self, Object[] varargs, PKeyword[] kwargs,
                         @Bind("this") Node inliningTarget,
-                        @Cached GetMroNode getMroNode,
-                        @Shared @Cached ReportAbstractClassNode reportAbstractClassNode) {
+                        @Shared @Cached ReportAbstractClassNode reportAbstractClassNode,
+                        @Shared @Cached CallNativeGenericNewNode callNativeGenericNewNode) {
             checkExcessArgs(self, varargs, kwargs);
             if (self.isAbstractClass()) {
                 throw reportAbstractClassNode.execute(frame, inliningTarget, self);
             }
-            Object nativeBaseClass = findFirstNativeBaseClass(getMroNode.execute(self));
-            return callNativeGenericNewNode(self, nativeBaseClass, varargs, kwargs);
+            return callNativeGenericNewNode.execute(inliningTarget, self);
         }
 
         @Specialization(guards = "isNativeClass(self)")
         @SuppressWarnings("truffle-static-method")
         Object doNativeObjectDirect(VirtualFrame frame, Object self, Object[] varargs, PKeyword[] kwargs,
                         @Bind("this") Node inliningTarget,
                         @Exclusive @Cached TypeNodes.GetTypeFlagsNode getTypeFlagsNode,
-                        @Shared @Cached ReportAbstractClassNode reportAbstractClassNode) {
+                        @Shared @Cached ReportAbstractClassNode reportAbstractClassNode,
+                        @Shared @Cached CallNativeGenericNewNode callNativeGenericNewNode) {
             checkExcessArgs(self, varargs, kwargs);
             if ((getTypeFlagsNode.execute(self) & TypeFlags.IS_ABSTRACT) != 0) {
                 throw reportAbstractClassNode.execute(frame, inliningTarget, self);
             }
-            return callNativeGenericNewNode(self, self, varargs, kwargs);
+            return callNativeGenericNewNode.execute(inliningTarget, self);
         }
 
-        @SuppressWarnings("unused")
-        @Fallback
-        Object fallback(Object o, Object[] varargs, PKeyword[] kwargs) {
-            throw raise(TypeError, ErrorMessages.IS_NOT_TYPE_OBJ, "object.__new__(X): X", o);
-        }
+        @GenerateInline
+        @GenerateCached(false)
+        protected abstract static class CallNativeGenericNewNode extends Node {
+            abstract Object execute(Node inliningTarget, Object cls);
 
-        private static Object findFirstNativeBaseClass(PythonAbstractClass[] methodResolutionOrder) {
-            for (Object cls : methodResolutionOrder) {
-                if (PGuards.isNativeClass(cls)) {
-                    return cls;
-                }
+            @Specialization
+            static Object call(Object cls,
+                            @Cached PythonToNativeNode toNativeNode,
+                            @Cached NativeToPythonNode toPythonNode,
+                            @Cached PCallCapiFunction callCapiFunction) {
+                return toPythonNode.execute(callCapiFunction.call(FUN_PY_OBJECT_NEW, toNativeNode.execute(cls)));
             }
-            CompilerDirectives.transferToInterpreterAndInvalidate();
-            throw new IllegalStateException("class needs native allocation but has not native base class");
         }
 
-        private Object callNativeGenericNewNode(Object type, Object nativeBase, Object[] varargs, PKeyword[] kwargs) {
-            if (callCapiFunction == null) {
-                CompilerDirectives.transferToInterpreterAndInvalidate();
-                callCapiFunction = insert(PCallCapiFunction.create());
-            }
-            if (toSulongNodes == null) {
-                CompilerDirectives.transferToInterpreterAndInvalidate();
-                CExtNodes.ToSulongNode[] newToSulongNodes = new CExtNodes.ToSulongNode[4];
-                for (int i = 0; i < newToSulongNodes.length; i++) {
-                    newToSulongNodes[i] = CExtNodesFactory.ToSulongNodeGen.create();
-                }
-                toSulongNodes = insert(newToSulongNodes);
-            }
-            if (asPythonObjectNode == null) {
-                CompilerDirectives.transferToInterpreterAndInvalidate();
-                asPythonObjectNode = insert(NativeToPythonNodeGen.create());
-            }
-            PKeyword[] kwarr = kwargs.length > 0 ? kwargs : null;
-            PTuple targs = factory().createTuple(varargs);
-            PDict dkwargs = factory().createDict(kwarr);
-            return asPythonObjectNode.execute(
-                            callCapiFunction.call(FUN_PY_OBJECT_NEW, toSulongNodes[0].execute(type), toSulongNodes[1].execute(nativeBase), toSulongNodes[2].execute(targs),
-                                            toSulongNodes[3].execute(dkwargs)));
+        @SuppressWarnings("unused")
+        @Fallback
+        Object fallback(Object o, Object[] varargs, PKeyword[] kwargs) {
+            throw raise(TypeError, ErrorMessages.IS_NOT_TYPE_OBJ, "object.__new__(X): X", o);
         }
 
         private void checkExcessArgs(Object type, Object[] varargs, PKeyword[] kwargs) {
diff --git a/graalpython/lib-graalpython/patches/jsonschema/metadata.toml b/graalpython/lib-graalpython/patches/jsonschema/metadata.toml
@@ -0,0 +1,3 @@
+[[rules]]
+# Newer versions depend on rpds_py, which needs rust
+version = '< 4.18'

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+[[rules]]`
	`2`	`+# Newer versions depend on rpds_py, which needs rust`
	`3`	`+version = '< 4.18'`