[GR-44691] [GR-45681] Various C API fixes to avoid fatal errors.

fangerer · fangerer · commit 6cdff55894aa · 2023-04-18T12:47:26.000Z
PullRequest: graalpython/2740
diff --git a/graalpython/com.oracle.graal.python.cext/src/capi.c b/graalpython/com.oracle.graal.python.cext/src/capi.c
@@ -432,6 +432,12 @@ PyAPI_FUNC(RESULT) get_##NAME(RECEIVER obj) {                      \
 PyAPI_FUNC(RESULT) get_##NAME(RECEIVER obj) {                    \
     return obj->FIELD? obj->FIELD->NAME : NULL;                   \
 }
+
+#define PRIMITIVE_EMBEDDED_FIELD_GETTER(RECEIVER, FIELD, RESULT, NAME) \
+PyAPI_FUNC(RESULT) get_##FIELD##_##NAME(RECEIVER obj) { \
+    return obj->FIELD.NAME; \
+}
+
 TYPE_FIELD_GETTER(PyObject*, ob_type)
 PRIMITIVE_FIELD_GETTER(PyObject*, Py_ssize_t, ob_refcnt)
 PRIMITIVE_FIELD_GETTER(PyVarObject*, Py_ssize_t, ob_size)
@@ -514,6 +520,8 @@ PRIMITIVE_FIELD_GETTER(PyTypeObject*, unsigned long, tp_flags)
 PRIMITIVE_FIELD_GETTER(PyModuleDef_Base*, Py_ssize_t, m_index)
 PRIMITIVE_FIELD_GETTER(PyModuleDef*, Py_ssize_t, m_size)
 PRIMITIVE_FIELD_GETTER(PyModuleDef*, const char*, m_doc)
+PRIMITIVE_EMBEDDED_FIELD_GETTER(PyComplexObject*, cval, double, real)
+PRIMITIVE_EMBEDDED_FIELD_GETTER(PyComplexObject*, cval, double, imag)
 
 char* get_ob_sval(PyObject* op) {
 	return ((PyBytesObject *)(op))->ob_sval;
diff --git a/graalpython/com.oracle.graal.python.jni/src/capi_native.c b/graalpython/com.oracle.graal.python.jni/src/capi_native.c
@@ -482,13 +482,18 @@ static PyObject* callBuffer[1024];
 PyAPI_FUNC(PyObject *) PyObject_CallFunctionObjArgs(PyObject *callable, ...) {
     va_list myargs;
     va_start(myargs, callable);
+    PyObject *arg;
     int count = 0;
     while (count <= 1024) {
-        PyObject *o = va_arg(myargs, PyObject *);
-        if (o == NULL) {
+        arg = va_arg(myargs, PyObject *);
+        if (arg == NULL) {
             break;
         }
-        callBuffer[count++] = o;
+        callBuffer[count++] = arg;
+    }
+    // test if buffer was too small
+    if (arg != NULL) {
+        Py_FatalError("argument buffer for 'PyObject_CallFunctionObjArgs' is too small");
     }
     PyObject* args = PyTuple_New(count);
     for (int i = 0; i < count; i++) {
@@ -502,9 +507,45 @@ PyAPI_FUNC(PyObject *) PyObject_CallFunctionObjArgs(PyObject *callable, ...) {
     return result;
 }
 
-PyObject * PyObject_CallMethodObjArgs(PyObject *a, PyObject *b, ...)  {
-    printf("PyObject_CallMethodObjArgs not implemented in capi_native - exiting\n");
-    exit(-1);
+PyObject * PyObject_CallMethodObjArgs(PyObject *obj, PyObject *name, ...)  {
+    if ((obj == NULL || name == NULL) && !PyErr_Occurred()) {
+        PyErr_SetString(PyExc_SystemError,
+                         "null argument to internal routine");
+        return NULL;
+    }
+
+    PyObject *callable = NULL;
+    int is_method = _PyObject_GetMethod(obj, name, &callable);
+    if (callable == NULL) {
+        return NULL;
+    }
+    obj = is_method ? obj : NULL;
+
+    va_list myargs;
+    va_start(myargs, name);
+    PyObject *arg;
+    int count = 0;
+    while (count <= 1024) {
+        arg = va_arg(myargs, PyObject *);
+        if (arg == NULL) {
+            break;
+        }
+        callBuffer[count++] = arg;
+    }
+    // test if buffer was too small
+    if (arg != NULL) {
+        Py_FatalError("argument buffer for 'PyObject_CallMethodObjArgs' is too small");
+    }
+    PyObject* args = PyTuple_New(count);
+    for (int i = 0; i < count; i++) {
+        Py_INCREF(callBuffer[i]);
+        PyTuple_SetItem(args, i, callBuffer[i]);
+    }
+    va_end(myargs);
+
+    PyObject* result = PyObject_CallObject(callable, args);
+    Py_DECREF(args);
+    return result;
 }
 
 PyObject * _PyObject_CallMethodIdObjArgs(PyObject *object, struct _Py_Identifier *name, ...) {
diff --git a/graalpython/com.oracle.graal.python.test/src/graalpytest.py b/graalpython/com.oracle.graal.python.test/src/graalpytest.py
@@ -69,6 +69,9 @@
     'test_unittest',
     'test_glob',
     'test_logging',
+    # test_int contains a denial-of-service prevention test that requires to finish within a certain time.
+    # We are actually fast enough but the parallel execution overhead can cause transient failures.
+    'test_int',
 ]
 
 
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextBuiltins.java
@@ -40,6 +40,7 @@
  */
 package com.oracle.graal.python.builtins.modules.cext;
 
+import static com.oracle.graal.python.builtins.PythonBuiltinClassType.MemoryError;
 import static com.oracle.graal.python.builtins.PythonBuiltinClassType.NotImplementedError;
 import static com.oracle.graal.python.builtins.PythonBuiltinClassType.RecursionError;
 import static com.oracle.graal.python.builtins.PythonBuiltinClassType.SystemError;
@@ -259,6 +260,10 @@ public static PException checkThrowableBeforeNative(Throwable t, String where1,
             PException pe = ExceptionUtils.wrapJavaException(soe, null, newException);
             throw pe;
         }
+        if (t instanceof OutOfMemoryError oome) {
+            PBaseException newException = PythonContext.get(null).factory().createBaseException(MemoryError);
+            throw ExceptionUtils.wrapJavaException(oome, null, newException);
+        }
         // everything else: log and convert to PException (SystemError)
         CompilerDirectives.transferToInterpreter();
         PNodeWithContext.printStack();
@@ -1404,10 +1409,11 @@ int doCachedDomainIdx(@SuppressWarnings("unused") int domain, Object pointerObje
             // this will also be called if the allocation failed
             if (!lib.isNull(pointerObject)) {
                 CApiContext cApiContext = getCApiContext();
-                cApiContext.getTraceMallocDomain(cachedDomainIdx).track(pointerObject, size);
+                Object key = CApiContext.asPointer(pointerObject, lib);
+                cApiContext.getTraceMallocDomain(cachedDomainIdx).track(key, size);
                 cApiContext.increaseMemoryPressure(null, getThreadStateNode, this, size);
                 if (LOGGER.isLoggable(Level.FINE)) {
-                    LOGGER.fine(() -> PythonUtils.formatJString("Tracking memory (size: %d): %s", size, CApiContext.asHex(pointerObject)));
+                    LOGGER.fine(() -> PythonUtils.formatJString("Tracking memory (size: %d): %s", size, CApiContext.asHex(key)));
                 }
             }
             return 0;
@@ -1433,20 +1439,23 @@ abstract static class PyTraceMalloc_Untrack extends CApiBinaryBuiltinNode {
         @Specialization(guards = {"isSingleContext()", "domain == cachedDomain"}, limit = "3")
         int doCachedDomainIdx(@SuppressWarnings("unused") int domain, Object pointerObject,
                         @Cached("domain") @SuppressWarnings("unused") long cachedDomain,
-                        @Cached("lookupDomain(domain)") int cachedDomainIdx) {
+                        @Cached("lookupDomain(domain)") int cachedDomainIdx,
+                        @CachedLibrary("pointerObject") InteropLibrary lib) {
 
             CApiContext cApiContext = getCApiContext();
-            long trackedMemorySize = cApiContext.getTraceMallocDomain(cachedDomainIdx).untrack(pointerObject);
+            Object key = CApiContext.asPointer(pointerObject, lib);
+            long trackedMemorySize = cApiContext.getTraceMallocDomain(cachedDomainIdx).untrack(key);
             cApiContext.reduceMemoryPressure(trackedMemorySize);
             if (LOGGER.isLoggable(Level.FINE)) {
-                LOGGER.fine(() -> PythonUtils.formatJString("Untracking memory (size: %d): %s", trackedMemorySize, CApiContext.asHex(pointerObject)));
+                LOGGER.fine(() -> PythonUtils.formatJString("Untracking memory (size: %d): %s", trackedMemorySize, CApiContext.asHex(key)));
             }
             return 0;
         }
 
         @Specialization(replaces = "doCachedDomainIdx")
-        int doGeneric(int domain, Object pointerObject) {
-            return doCachedDomainIdx(domain, pointerObject, domain, lookupDomain(domain));
+        int doGeneric(int domain, Object pointerObject,
+                        @CachedLibrary(limit = "3") InteropLibrary lib) {
+            return doCachedDomainIdx(domain, pointerObject, domain, lookupDomain(domain), lib);
         }
 
         int lookupDomain(int domain) {
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiContext.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiContext.java
@@ -282,8 +282,7 @@ public static Object asPointer(Object ptr, InteropLibrary lib) {
             try {
                 return lib.asPointer(ptr);
             } catch (UnsupportedMessageException e) {
-                CompilerDirectives.transferToInterpreter();
-                throw new IllegalStateException();
+                throw CompilerDirectives.shouldNotReachHere(e);
             }
         }
         return ptr;
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/NativeCAPISymbol.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/NativeCAPISymbol.java
@@ -194,6 +194,8 @@ public enum NativeCAPISymbol implements NativeCExtSymbol {
     FUN_GET_MP_LENGTH("get_mp_length"),
     FUN_GET_MP_SUBSCRIPT("get_mp_subscript"),
     FUN_GET_MP_ASS_SUBSCRIPT("get_mp_ass_subscript"),
+    FUN_GET_PY_COMPLEX_CVAL_REAL("get_cval_real"),
+    FUN_GET_PY_COMPLEX_CVAL_IMAG("get_cval_imag"),
     FUN_GET_PYMODULEDEF_M_METHODS("get_PyModuleDef_m_methods"),
     FUN_GET_PYMODULEDEF_M_SLOTS("get_PyModuleDef_m_slots"),
     FUN_GET_BYTE_ARRAY_TYPE_ID("get_byte_array_typeid"),
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/complex/ComplexBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/complex/ComplexBuiltins.java
@@ -81,6 +81,10 @@
 import com.oracle.graal.python.builtins.modules.SysModuleBuiltins;
 import com.oracle.graal.python.builtins.objects.PNone;
 import com.oracle.graal.python.builtins.objects.PNotImplemented;
+import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
+import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.PCallCapiFunction;
+import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.ToSulongNode;
+import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
 import com.oracle.graal.python.builtins.objects.common.FormatNodeBase;
 import com.oracle.graal.python.builtins.objects.complex.ComplexBuiltinsClinicProviders.FormatNodeClinicProviderGen;
 import com.oracle.graal.python.builtins.objects.floats.FloatBuiltins;
@@ -102,6 +106,7 @@
 import com.oracle.graal.python.runtime.formatting.InternalFormat;
 import com.oracle.graal.python.runtime.formatting.InternalFormat.Spec;
 import com.oracle.graal.python.runtime.object.PythonObjectFactory;
+import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.dsl.Bind;
 import com.oracle.truffle.api.dsl.Cached;
@@ -112,6 +117,8 @@
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.dsl.TypeSystemReference;
 import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.interop.InteropLibrary;
+import com.oracle.truffle.api.interop.UnsupportedMessageException;
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.profiles.InlinedConditionProfile;
 import com.oracle.truffle.api.strings.TruffleString;
@@ -798,12 +805,43 @@ static double get(PComplex self) {
     @Builtin(name = J___HASH__, minNumOfPositionalArgs = 1)
     abstract static class HashNode extends PythonUnaryBuiltinNode {
         @Specialization
-        static long hash(PComplex self) {
+        static long doPComplex(PComplex self) {
+            return complexHash(self.getReal(), self.getImag());
+        }
+
+        @Specialization
+        static long doNative(PythonAbstractNativeObject self,
+                        @Cached ToSulongNode toSulongNode,
+                        @Cached PCallCapiFunction callGetRealNode,
+                        @Cached PCallCapiFunction callGetImagNode) {
+            Object ptr = toSulongNode.execute(self);
+            Object realObj = callGetRealNode.call(NativeCAPISymbol.FUN_GET_PY_COMPLEX_CVAL_REAL, ptr);
+            Object imagObj = callGetImagNode.call(NativeCAPISymbol.FUN_GET_PY_COMPLEX_CVAL_IMAG, ptr);
+            return complexHash(expectDouble(realObj), expectDouble(imagObj));
+        }
+
+        private static long complexHash(double real, double imag) {
             // just like CPython
-            long realHash = PyObjectHashNode.hash(self.getReal());
-            long imagHash = PyObjectHashNode.hash(self.getImag());
+            long realHash = PyObjectHashNode.hash(real);
+            long imagHash = PyObjectHashNode.hash(imag);
             return realHash + SysModuleBuiltins.HASH_IMAG * imagHash;
         }
+
+        private static double expectDouble(Object val) {
+            if (val instanceof Double) {
+                return (double) val;
+            }
+            InteropLibrary lib = InteropLibrary.getUncached(val);
+            if (lib.fitsInDouble(val)) {
+                try {
+                    return lib.asDouble(val);
+                } catch (UnsupportedMessageException e) {
+                    // fall through
+                }
+            }
+            throw CompilerDirectives.shouldNotReachHere();
+        }
+
     }
 
     @GenerateNodeFactory
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java
@@ -2151,6 +2151,12 @@ public PythonNativeWrapper getSingletonNativeWrapper(PythonAbstractObject obj) {
         return null;
     }
 
+    /**
+     * This method is intended to be called to re-acquire the GIL after a {@link StackOverflowError}
+     * was catched. To reduce the probability that re-acquiring the GIL causes again a
+     * {@link StackOverflowError}, it is important to keep this method as simple as possible. In
+     * particular, do not add calls if there is a way to avoid it.
+     */
     public void reacquireGilAfterStackOverflow() {
         while (!ownsGil()) {
             try {
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/exception/PException.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/exception/PException.java
@@ -122,6 +122,12 @@ private static RuntimeException createStacktraceCarrier() {
         return new RuntimeException();
     }
 
+    /*
+     * Note: we use this method to convert a Java StackOverflowError into a Python RecursionError.
+     * At the time when this is done, some Java stack frames were already unwinded but there is no
+     * guarantee on how many. Therefore, it is important that this method is simple. In particular,
+     * do not add calls if that can be avoided.
+     */
     public static PException fromObject(PBaseException actual, Node node, Throwable wrapped) {
         PException pException = new PException(actual, node, wrapped);
         actual.setException(pException);
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/object/PythonObjectFactory.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/object/PythonObjectFactory.java
@@ -933,6 +933,12 @@ public final PBaseException createBaseException(Object cls, Object[] data, PTupl
         return trace(new PBaseException(cls, getShape(cls), data, args));
     }
 
+    /*
+     * Note: we use this method to convert a Java StackOverflowError into a Python RecursionError.
+     * At the time when this is done, some Java stack frames were already unwinded but there is no
+     * guarantee on how many. Therefore, it is important that this method is simple. In particular,
+     * do not add calls if that can be avoided.
+     */
     public final PBaseException createBaseException(Object cls, TruffleString format, Object[] args) {
         return createBaseException(cls, null, format, args);
     }

Original file line number	Diff line number	Diff line change
`@@ -69,6 +69,9 @@`
`69`	`69`	`'test_unittest',`
`70`	`70`	`'test_glob',`
`71`	`71`	`'test_logging',`
	`72`	`+ # test_int contains a denial-of-service prevention test that requires to finish within a certain time.`
	`73`	`+ # We are actually fast enough but the parallel execution overhead can cause transient failures.`
	`74`	`+ 'test_int',`
`72`	`75`	`]`
`73`	`76`
`74`	`77`
Original file line number	Diff line number	Diff line change
`@@ -282,8 +282,7 @@ public static Object asPointer(Object ptr, InteropLibrary lib) {`
`282`	`282`	`try {`
`283`	`283`	`return lib.asPointer(ptr);`
`284`	`284`	`} catch (UnsupportedMessageException e) {`
`285`		`- CompilerDirectives.transferToInterpreter();`
`286`		`- throw new IllegalStateException();`
	`285`	`+ throw CompilerDirectives.shouldNotReachHere(e);`
`287`	`286`	`}`
`288`	`287`	`}`
`289`	`288`	`return ptr;`
Original file line number	Diff line number	Diff line change
`@@ -933,6 +933,12 @@ public final PBaseException createBaseException(Object cls, Object[] data, PTupl`
`933`	`933`	`return trace(new PBaseException(cls, getShape(cls), data, args));`
`934`	`934`	`}`
`935`	`935`
	`936`	`+ /*`
	`937`	`+ * Note: we use this method to convert a Java StackOverflowError into a Python RecursionError.`
	`938`	`+ * At the time when this is done, some Java stack frames were already unwinded but there is no`
	`939`	`+ * guarantee on how many. Therefore, it is important that this method is simple. In particular,`
	`940`	`+ * do not add calls if that can be avoided.`
	`941`	`+ */`
`936`	`942`	`public final PBaseException createBaseException(Object cls, TruffleString format, Object[] args) {`
`937`	`943`	`return createBaseException(cls, null, format, args);`
`938`	`944`	`}`