Skip to content

Commit 63ad40d

Browse files
fangererfangerer
committed
Correctly use native null instead of error marker.
1 parent ff5133c commit 63ad40d

File tree

10 files changed

+182
-50
lines changed

10 files changed

+182
-50
lines changed

‎graalpython/com.oracle.graal.python.cext/src/capi.c

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,9 @@ typedef PyObject* PyObjectPtr;
147147
POLYGLOT_DECLARE_TYPE(PyObjectPtr);
148148

149149
static void initialize_globals() {
150+
// register native NULL
151+
wrapped_null = polyglot_invoke(PY_TRUFFLE_CEXT, polyglot_from_string("PyTruffle_Register_NULL", SRC_CS), NULL);
152+
150153
// None
151154
PyObject* jnone = UPCALL_CEXT_O(polyglot_from_string("Py_None", SRC_CS));
152155
truffle_assign_managed(&_Py_NoneStruct, jnone);
@@ -168,9 +171,6 @@ static void initialize_globals() {
168171
// error marker
169172
void *jerrormarker = UPCALL_CEXT_PTR(polyglot_from_string("Py_ErrorHandler", SRC_CS));
170173
truffle_assign_managed(&marker_struct, jerrormarker);
171-
172-
// register native NULL
173-
polyglot_invoke(PY_TRUFFLE_CEXT, polyglot_from_string("PyTruffle_Register_NULL", SRC_CS), NULL);
174174
}
175175

176176
static void initialize_bufferprocs() {
@@ -188,21 +188,16 @@ static void initialize_capi() {
188188
initialize_bufferprocs();
189189
}
190190

191-
__attribute__((always_inline))
192-
inline void* handle_exception(void* val) {
193-
return val;
194-
}
195-
196191
// Heuristic to test if some value is a pointer object
197192
// TODO we need a reliable solution for that
198193
#define IS_POINTER(__val__) (polyglot_is_value(__val__) && !polyglot_fits_in_i64(__val__))
199194

200195
MUST_INLINE
201196
void* native_to_java(PyObject* obj) {
202-
if (obj == Py_None) {
197+
if (obj == NULL) {
198+
return wrapped_null;
199+
} else if (obj == Py_None) {
203200
return Py_None;
204-
} else if (obj == NULL) {
205-
return Py_NoValue;
206201
} else if (polyglot_is_string(obj)) {
207202
return obj;
208203
} else if (truffle_is_handle_to_managed(obj)) {
@@ -512,6 +507,8 @@ PyObject* WritePySSizeT(PyObject* object, Py_ssize_t offset, PyObject* value) {
512507
return value;
513508
}
514509

510+
PyObject* wrapped_null;
511+
515512
PyObject marker_struct = {
516513
_PyObject_EXTRA_INIT
517514
1, &PyBaseObject_Type

‎graalpython/com.oracle.graal.python.cext/src/capi.h

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ extern void* (*PY_TRUFFLE_CEXT_LANDING_PTR)(void* name, ...);
9797
}
9898

9999
/* Call function with return type 'PyObject *'; does polyglot cast and error handling */
100-
#define UPCALL_O(__recv__, __name__, ...) handle_exception(PY_TRUFFLE_LANDING((__recv__), __name__, ##__VA_ARGS__))
100+
#define UPCALL_O(__recv__, __name__, ...) PY_TRUFFLE_LANDING((__recv__), __name__, ##__VA_ARGS__)
101101

102102
/* Call function with a primitive return; no polyglot cast but error handling */
103103
#define UPCALL_P(__recv__, __name__, ...) (PY_TRUFFLE_LANDING_L((__recv__), __name__, ##__VA_ARGS__))
@@ -109,19 +109,19 @@ extern void* (*PY_TRUFFLE_CEXT_LANDING_PTR)(void* name, ...);
109109
#define UPCALL_L(__recv__, __name__, ...) UPCALL_P(__recv__, __name__, ##__VA_ARGS__)
110110

111111
/* Call function with return type 'double'; no polyglot cast but error handling */
112-
#define UPCALL_D(__recv__, __name__, ...) handle_exception(PY_TRUFFLE_LANDING_D((__recv__), __name__, ##__VA_ARGS__))
112+
#define UPCALL_D(__recv__, __name__, ...) PY_TRUFFLE_LANDING_D((__recv__), __name__, ##__VA_ARGS__)
113113

114114
/* Call function with return type 'void*'; no polyglot cast and no error handling */
115115
#define UPCALL_PTR(__name__, ...) (PY_TRUFFLE_LANDING_PTR(__name__, ##__VA_ARGS__))
116116

117117
/* Call function of 'python_cext' module with return type 'PyObject *'; does polyglot cast and error handling */
118-
#define UPCALL_CEXT_O(__name__, ...) handle_exception(PY_TRUFFLE_CEXT_LANDING(__name__, ##__VA_ARGS__))
118+
#define UPCALL_CEXT_O(__name__, ...) PY_TRUFFLE_CEXT_LANDING(__name__, ##__VA_ARGS__)
119119

120120
/* Call void function of 'python_cext' module; no polyglot cast and no error handling */
121121
#define UPCALL_CEXT_VOID(__name__, ...) (PY_TRUFFLE_CEXT_LANDING(__name__, ##__VA_ARGS__))
122122

123123
/* Call function of 'python_cext' module with return type 'PyObject*'; no polyglot cast but error handling */
124-
#define UPCALL_CEXT_NOCAST(__name__, ...) handle_exception(PY_TRUFFLE_CEXT_LANDING(__name__, ##__VA_ARGS__))
124+
#define UPCALL_CEXT_NOCAST(__name__, ...) PY_TRUFFLE_CEXT_LANDING(__name__, ##__VA_ARGS__)
125125

126126
/* Call function of 'python_cext' module with return type 'void*'; no polyglot cast and no error handling */
127127
#define UPCALL_CEXT_PTR(__name__, ...) (PY_TRUFFLE_CEXT_LANDING_PTR(__name__, ##__VA_ARGS__))
@@ -150,7 +150,6 @@ extern void* (*PY_TRUFFLE_CEXT_LANDING_PTR)(void* name, ...);
150150
#define as_float(obj) ((float)as_double(obj))
151151

152152

153-
void* handle_exception(void* val);
154153
void* native_to_java(PyObject* obj);
155154
extern void* to_java(PyObject* obj);
156155
extern void* to_java_type(PyTypeObject* cls);
@@ -249,7 +248,12 @@ int PyTruffle_Debug(void *arg);
249248
void* PyObjectHandle_ForJavaType(void* jobj);
250249

251250
extern PyObject marker_struct;
252-
#define ERROR_MARKER &marker_struct
251+
extern PyObject* wrapped_null;
252+
253+
/* An error marker object.
254+
* The object should not be converted to_java and is intended to be returned in the error case.
255+
* That's mainly useful for direct calls (without landing functions) of 'python_cext' functions. */
256+
#define ERROR_MARKER wrapped_null
253257

254258
/* internal functions to avoid unnecessary managed <-> native conversions */
255259

‎graalpython/com.oracle.graal.python.cext/src/exceptions.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ void initialize_exceptions() {
124124
UPCALL_ID(with_traceback);
125125
int PyException_SetTraceback(PyObject *self, PyObject *tb) {
126126
PyObject* result = UPCALL_O(native_to_java(self), _jls_with_traceback, native_to_java(tb));
127-
if (result == ERROR_MARKER) {
127+
if (result == NULL) {
128128
return -1;
129129
} else {
130130
return 0;

‎graalpython/com.oracle.graal.python.cext/src/methodobject.c

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ typedef PyObject *(*PyCFunction)(PyObject *, PyObject *);
4545
PyTypeObject PyCFunction_Type = PY_TRUFFLE_TYPE("builtin_function_or_method", &PyType_Type, Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, sizeof(PyCFunctionObject));
4646

4747
PyObject* PyCFunction_NewEx(PyMethodDef *ml, PyObject *self, PyObject *module) {
48-
PyObject* func = to_sulong(polyglot_invoke(PY_TRUFFLE_CEXT,
48+
return to_sulong(polyglot_invoke(PY_TRUFFLE_CEXT,
4949
"PyCFunction_NewEx",
5050
polyglot_from_string((const char*)(ml->ml_name), SRC_CS),
5151
ml->ml_meth,
@@ -54,9 +54,4 @@ PyObject* PyCFunction_NewEx(PyMethodDef *ml, PyObject *self, PyObject *module) {
5454
self,
5555
module,
5656
polyglot_from_string((const char*)(ml->ml_doc ? ml->ml_doc : ""), SRC_CS)));
57-
if (func == ERROR_MARKER) {
58-
return NULL;
59-
} else {
60-
return func;
61-
}
6257
}

‎graalpython/com.oracle.graal.python.cext/src/sliceobject.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ int PySlice_GetIndicesEx(PyObject *_r, Py_ssize_t length,
5353
Py_ssize_t *slicelength) {
5454
PySliceObject *r = (PySliceObject*)_r;
5555
void *result = to_sulong(polyglot_invoke(PY_TRUFFLE_CEXT, "PySlice_GetIndicesEx", r->start, r->stop, r->step, length));
56-
if (result == ERROR_MARKER) {
56+
if (result == NULL) {
5757
return -1;
5858
}
5959
*start = PyLong_AsSsize_t(PyTuple_GetItem(result, 0));
@@ -66,7 +66,7 @@ int PySlice_GetIndicesEx(PyObject *_r, Py_ssize_t length,
6666
int PySlice_Unpack(PyObject *_r, Py_ssize_t *start, Py_ssize_t *stop, Py_ssize_t *step) {
6767
PySliceObject *r = (PySliceObject*)_r;
6868
void *result = to_sulong(polyglot_invoke(PY_TRUFFLE_CEXT, "PySlice_GetIndicesEx", r->start, r->stop, r->step, PY_SSIZE_T_MAX));
69-
if (result == ERROR_MARKER) {
69+
if (result == NULL) {
7070
return -1;
7171
}
7272
*start = PyLong_AsSsize_t(PyTuple_GetItem(result, 0));
@@ -77,7 +77,7 @@ int PySlice_Unpack(PyObject *_r, Py_ssize_t *start, Py_ssize_t *stop, Py_ssize_t
7777

7878
Py_ssize_t PySlice_AdjustIndices(Py_ssize_t length, Py_ssize_t *start, Py_ssize_t *stop, Py_ssize_t step) {
7979
PyObject *result = to_sulong(polyglot_invoke(PY_TRUFFLE_CEXT, "PySlice_GetIndicesEx", *start, *stop, step, length));
80-
if (result == ERROR_MARKER) {
80+
if (result == NULL) {
8181
return -1;
8282
}
8383
*start = PyLong_AsSsize_t(PyTuple_GetItem(result, 0));

‎graalpython/com.oracle.graal.python.cext/src/unicodeobject.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ char* PyUnicode_AsUTF8AndSize(PyObject *unicode, Py_ssize_t *psize) {
260260
UPCALL_ID(_PyUnicode_AsUTF8String);
261261
PyObject* _PyUnicode_AsUTF8String(PyObject *unicode, const char *errors) {
262262
void *jerrors = errors != NULL ? polyglot_from_string(errors, SRC_CS) : NULL;
263-
return UPCALL_CEXT_O(_jls__PyUnicode_AsUTF8String, native_to_java(unicode), native_to_java(jerrors), ERROR_MARKER);
263+
return UPCALL_CEXT_O(_jls__PyUnicode_AsUTF8String, native_to_java(unicode), native_to_java(jerrors), NULL);
264264
}
265265

266266
// taken from CPython "Python/Objects/unicodeobject.c"
@@ -272,7 +272,7 @@ PyObject * PyUnicode_DecodeUTF32(const char *s, Py_ssize_t size, const char *err
272272
PyObject *result;
273273
void *jerrors = errors != NULL ? polyglot_from_string(errors, SRC_CS) : NULL;
274274
int bo = byteorder != NULL ? *byteorder : 0;
275-
return handle_exception(polyglot_invoke(PY_TRUFFLE_CEXT, "PyTruffle_Unicode_DecodeUTF32", s, size, native_to_java(jerrors), bo, ERROR_MARKER));
275+
return polyglot_invoke(PY_TRUFFLE_CEXT, "PyTruffle_Unicode_DecodeUTF32", s, size, native_to_java(jerrors), bo, NULL);
276276
}
277277

278278
Py_ssize_t PyUnicode_AsWideChar(PyObject *unicode, wchar_t *w, Py_ssize_t size) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/TruffleCextBuiltins.java

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
import com.oracle.graal.python.builtins.objects.cext.NativeWrappers.PythonNativeWrapper;
7575
import com.oracle.graal.python.builtins.objects.cext.NativeWrappers.PythonObjectNativeWrapper;
7676
import com.oracle.graal.python.builtins.objects.cext.PythonNativeClass;
77+
import com.oracle.graal.python.builtins.objects.cext.PythonNativeNull;
7778
import com.oracle.graal.python.builtins.objects.cext.UnicodeObjectNodes.UnicodeAsWideCharNode;
7879
import com.oracle.graal.python.builtins.objects.code.PCode;
7980
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes;
@@ -157,6 +158,7 @@
157158
public class TruffleCextBuiltins extends PythonBuiltins {
158159

159160
private static final String ERROR_HANDLER = "error_handler";
161+
private static final String NATIVE_NULL = "native_null";
160162

161163
@Override
162164
protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFactories() {
@@ -170,6 +172,7 @@ public void initialize(PythonCore core) {
170172
new PythonClass[]{core.lookupType(PythonBuiltinClassType.PythonObject)});
171173
builtinConstants.put("CErrorHandler", errorHandlerClass);
172174
builtinConstants.put(ERROR_HANDLER, core.factory().createPythonObject(errorHandlerClass));
175+
builtinConstants.put(NATIVE_NULL, new PythonNativeNull());
173176
}
174177

175178
/**
@@ -535,6 +538,12 @@ Object doNativeWrapper(String name, @SuppressWarnings("unused") PythonAbstractOb
535538
return result;
536539
}
537540

541+
@Specialization
542+
Object doPythonNativeNull(String name, @SuppressWarnings("unused") PythonNativeNull result) {
543+
checkFunctionResult(name, true);
544+
return result;
545+
}
546+
538547
@Specialization(guards = "isForeignObject(result)")
539548
Object doForeign(String name, TruffleObject result,
540549
@Cached("createBinaryProfile()") ConditionProfile isNullProfile) {
@@ -550,12 +559,12 @@ private void checkFunctionResult(String name, boolean isNull) {
550559
boolean errOccurred = currentException != null;
551560
if (isNull) {
552561
if (!errOccurred) {
553-
throw context.getCore().raise(PythonErrorType.SystemError, isNullNode, "%s returned NULL without setting an error", name);
562+
throw raise(PythonErrorType.SystemError, "%s returned NULL without setting an error", name);
554563
} else {
555564
throw currentException;
556565
}
557566
} else if (errOccurred) {
558-
throw context.getCore().raise(PythonErrorType.SystemError, isNullNode, "%s returned a result with an error set", name);
567+
throw raise(PythonErrorType.SystemError, "%s returned a result with an error set", name);
559568
}
560569
}
561570

@@ -1032,7 +1041,7 @@ abstract static class PyTruffle_Unicode_DecodeUTF32 extends NativeUnicodeBuiltin
10321041
@Child private CExtNodes.ToSulongNode toSulongNode;
10331042

10341043
@Specialization(guards = "isByteArray(o)")
1035-
Object doUnicode(TruffleObject o, long size, @SuppressWarnings("unused") PNone errors, int byteorder, Object errorMarker) {
1044+
Object doUnicode(TruffleObject o, long size, @SuppressWarnings("unused") PythonNativeNull errors, int byteorder, Object errorMarker) {
10361045
return doUnicode(o, size, "strict", byteorder, errorMarker);
10371046
}
10381047

@@ -1627,7 +1636,7 @@ Object upcall(VirtualFrame frame, PythonModule cextModule, Object receiver, Stri
16271636
readErrorHandlerNode = ReadAttributeFromObjectNode.create();
16281637
}
16291638
getContext().setCurrentException(e);
1630-
return toSulongNode.execute(readErrorHandlerNode.execute(cextModule, ERROR_HANDLER));
1639+
return toSulongNode.execute(readErrorHandlerNode.execute(cextModule, NATIVE_NULL));
16311640
}
16321641
}
16331642
}
@@ -1738,7 +1747,7 @@ Object upcall(VirtualFrame frame, PythonModule cextModule, String name, Object[]
17381747
}
17391748
}
17401749

1741-
@Builtin(name = "make_may_raise_wrapper", fixedNumOfPositionalArgs = 2)
1750+
@Builtin(name = "make_may_raise_wrapper", minNumOfPositionalArgs = 1, maxNumOfPositionalArgs = 2)
17421751
@GenerateNodeFactory
17431752
abstract static class MakeMayRaiseWrapperNode extends PythonBuiltinNode {
17441753
static class MayRaiseWrapper extends RootNode {
@@ -1806,4 +1815,19 @@ abstract static class AsDouble extends PythonBuiltinNode {
18061815
return asDoubleNode.execute(object);
18071816
}
18081817
}
1818+
1819+
@Builtin(name = "PyTruffle_Register_NULL", fixedNumOfPositionalArgs = 1)
1820+
@GenerateNodeFactory
1821+
abstract static class PyTruffle_Register_NULL extends PythonUnaryBuiltinNode {
1822+
@Specialization
1823+
Object doIt(Object object,
1824+
@Cached("create()") ReadAttributeFromObjectNode writeAttrNode) {
1825+
Object wrapper = writeAttrNode.execute(getCore().lookupBuiltinModule("python_cext"), NATIVE_NULL);
1826+
if (wrapper instanceof PythonNativeNull) {
1827+
((PythonNativeNull) wrapper).setPtr(object);
1828+
}
1829+
1830+
return wrapper;
1831+
}
1832+
}
18091833
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/CExtNodes.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,11 @@ Object doNativeObject(PythonNativeObject nativeObject) {
281281
return nativeObject.object;
282282
}
283283

284+
@Specialization
285+
Object doNativeNull(PythonNativeNull object) {
286+
return object.getPtr();
287+
}
288+
284289
@Specialization(guards = "!isNativeClass(object)")
285290
Object doPythonClass(PythonClass object) {
286291
return PythonClassNativeWrapper.wrap(object);
@@ -369,11 +374,16 @@ Object doNativeWrapperGeneric(PythonNativeWrapper object) {
369374
return object.getDelegate();
370375
}
371376

372-
@Specialization(guards = {"isForeignObject(object)", "!isNativeWrapper(object)"})
377+
@Specialization(guards = {"isForeignObject(object)", "!isNativeWrapper(object)", "!isNativeNull(object)"})
373378
PythonAbstractObject doNativeObject(TruffleObject object) {
374379
return factory().createNativeObjectWrapper(object);
375380
}
376381

382+
@Specialization
383+
PNone doNativeNull(@SuppressWarnings("unused") PythonNativeNull object) {
384+
return PNone.NO_VALUE;
385+
}
386+
377387
@Specialization
378388
PythonAbstractObject doPythonObject(PythonAbstractObject object) {
379389
return object;
@@ -413,6 +423,10 @@ protected static boolean isPrimitiveNativeWrapper(PythonNativeWrapper object) {
413423
return object instanceof PrimitiveNativeWrapper && !isMaterialized((PrimitiveNativeWrapper) object);
414424
}
415425

426+
protected static boolean isNativeNull(Object object) {
427+
return object instanceof PythonNativeNull;
428+
}
429+
416430
protected boolean isForeignObject(TruffleObject obj) {
417431
// TODO we could probably also just use 'PGuards.isForeignObject'
418432
if (getClassNode == null) {

0 commit comments

Comments
 (0)