From e77bbaf820ef4efce48ed7d0dd3fa31bd17e74ec Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 18 Sep 2023 19:48:45 +0200 Subject: [PATCH 1/2] Add PyMapping_HasKeyWithError() function Add PyMapping_HasKeyWithError() and PyMapping_HasKeyStringWithError() functions. Fix also undefined behavior in PyMapping_GetOptionalItemString() when PyUnicode_FromString() fails: set '*result' to NULL. --- docs/api.rst | 8 ++++ pythoncapi_compat.h | 26 ++++++++++- tests/test_pythoncapi_compat_cext.c | 68 +++++++++++++++++++---------- 3 files changed, 78 insertions(+), 24 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 6f15f9c..ebabf9d 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -58,6 +58,14 @@ Python 3.13 See `PyMapping_GetOptionalItemString() documentation `__. +.. c:function:: int PyMapping_HasKeyWithError(PyObject *obj, PyObject *key) + + See `PyMapping_HasKeyWithError() documentation `__. + +.. c:function:: int PyMapping_HasKeyStringWithError(PyObject *obj, const char *key) + + See `PyMapping_HasKeyStringWithError() documentation `__. + .. c:function:: int PyModule_Add(PyObject *module, const char *name, PyObject *value) See `PyModule_Add() documentation `__. diff --git a/pythoncapi_compat.h b/pythoncapi_compat.h index ae0a1a4..6530b78 100644 --- a/pythoncapi_compat.h +++ b/pythoncapi_compat.h @@ -711,7 +711,8 @@ PyObject_GetOptionalAttrString(PyObject *obj, const char *name, PyObject **resul #endif -// gh-106307 added PyObject_GetOptionalAttr() to Python 3.13.0a1 +// gh-106307 added PyObject_GetOptionalAttr() and +// PyMapping_GetOptionalItemString() to Python 3.13.0a1 #if PY_VERSION_HEX < 0x030D00A1 static inline int PyMapping_GetOptionalItem(PyObject *obj, PyObject *key, PyObject **result) @@ -738,6 +739,7 @@ PyMapping_GetOptionalItemString(PyObject *obj, const char *key, PyObject **resul key_obj = PyString_FromString(key); #endif if (key_obj == NULL) { + *result = NULL; return -1; } rc = PyMapping_GetOptionalItem(obj, key_obj, result); @@ -746,6 +748,28 @@ PyMapping_GetOptionalItemString(PyObject *obj, const char *key, PyObject **resul } #endif +// gh-108511 added PyMapping_HasKeyWithError() and +// PyMapping_HasKeyStringWithError() to Python 3.13.0a1 +#if PY_VERSION_HEX < 0x030D00A1 +static inline int +PyMapping_HasKeyWithError(PyObject *obj, PyObject *key) +{ + PyObject *res; + int rc = PyMapping_GetOptionalItem(obj, key, &res); + Py_XDECREF(res); + return rc; +} + +static inline int +PyMapping_HasKeyStringWithError(PyObject *obj, const char *key) +{ + PyObject *res; + int rc = PyMapping_GetOptionalItemString(obj, key, &res); + Py_XDECREF(res); + return rc; +} +#endif + // gh-106004 added PyDict_GetItemRef() and PyDict_GetItemStringRef() // to Python 3.13.0a1 diff --git a/tests/test_pythoncapi_compat_cext.c b/tests/test_pythoncapi_compat_cext.c index 0f6dae6..d788017 100644 --- a/tests/test_pythoncapi_compat_cext.c +++ b/tests/test_pythoncapi_compat_cext.c @@ -50,6 +50,9 @@ # define ASSERT_REFCNT(expr) #endif +// Marker to check that pointer value was set +#define UNINITIALIZED_OBJ ((PyObject *)"uninitialized") + static PyObject* create_string(const char *str) @@ -806,7 +809,7 @@ test_weakref(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) } // test PyWeakref_GetRef(), reference is alive - PyObject *ref = Py_True; // marker to check that value was set + PyObject *ref = UNINITIALIZED_OBJ; assert(PyWeakref_GetRef(weakref, &ref) == 1); assert(ref == obj); assert(Py_REFCNT(obj) == (refcnt + 1)); @@ -1016,28 +1019,28 @@ test_getattr(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) // test PyObject_GetOptionalAttr(): attribute exists attr_name = create_string("version"); - value = Py_True; // marker value + value = UNINITIALIZED_OBJ; assert(PyObject_GetOptionalAttr(obj, attr_name, &value) == 1); assert(value != _Py_NULL); Py_DECREF(value); Py_DECREF(attr_name); // test PyObject_GetOptionalAttrString(): attribute exists - value = Py_True; // marker value + value = UNINITIALIZED_OBJ; assert(PyObject_GetOptionalAttrString(obj, "version", &value) == 1); assert(value != _Py_NULL); Py_DECREF(value); // test PyObject_GetOptionalAttr(): attribute doesn't exist attr_name = create_string("nonexistant_attr_name"); - value = Py_True; // marker value + value = UNINITIALIZED_OBJ; assert(PyObject_GetOptionalAttr(obj, attr_name, &value) == 0); assert(value == _Py_NULL); Py_DECREF(attr_name); assert(!PyErr_Occurred()); // test PyObject_GetOptionalAttrString(): attribute doesn't exist - value = Py_True; // marker value + value = UNINITIALIZED_OBJ; assert(PyObject_GetOptionalAttrString(obj, "nonexistant_attr_name", &value) == 0); assert(value == _Py_NULL); assert(!PyErr_Occurred()); @@ -1056,37 +1059,56 @@ test_getitem(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) assert(value != _Py_NULL); PyObject *obj = Py_BuildValue("{sO}", "key", value); assert(obj != _Py_NULL); - PyObject *key; + PyObject *present_key, *missing_key; PyObject *item; + present_key = create_string("key"); + missing_key = create_string("dontexist"); + // test PyMapping_GetOptionalItem(): key is present - key = create_string("key"); - item = Py_True; // marker value - assert(PyMapping_GetOptionalItem(obj, key, &item) == 1); + item = UNINITIALIZED_OBJ; + assert(PyMapping_GetOptionalItem(obj, present_key, &item) == 1); assert(item == value); Py_DECREF(item); - Py_DECREF(key); + assert(!PyErr_Occurred()); + + // test PyMapping_HasKeyWithError(): key is present + assert(PyMapping_HasKeyWithError(obj, present_key) == 1); + assert(!PyErr_Occurred()); // test PyMapping_GetOptionalItemString(): key is present - item = Py_True; // marker value + item = UNINITIALIZED_OBJ; assert(PyMapping_GetOptionalItemString(obj, "key", &item) == 1); assert(item == value); Py_DECREF(item); + // test PyMapping_HasKeyStringWithError(): key is present + assert(PyMapping_HasKeyStringWithError(obj, "key") == 1); + assert(!PyErr_Occurred()); + // test PyMapping_GetOptionalItem(): missing key - key = create_string("dontexist"); - item = Py_True; // marker value - assert(PyMapping_GetOptionalItem(obj, key, &item) == 0); + item = UNINITIALIZED_OBJ; + assert(PyMapping_GetOptionalItem(obj, missing_key, &item) == 0); assert(item == _Py_NULL); - Py_DECREF(key); + assert(!PyErr_Occurred()); + + // test PyMapping_HasKeyWithError(): missing key + assert(PyMapping_HasKeyWithError(obj, missing_key) == 0); + assert(!PyErr_Occurred()); // test PyMapping_GetOptionalItemString(): missing key - item = Py_True; // marker value + item = UNINITIALIZED_OBJ; assert(PyMapping_GetOptionalItemString(obj, "dontexist", &item) == 0); assert(item == _Py_NULL); + // test PyMapping_HasKeyStringWithError(): missing key + assert(PyMapping_HasKeyStringWithError(obj, "dontexist") == 0); + assert(!PyErr_Occurred()); + Py_DECREF(obj); Py_DECREF(value); + Py_DECREF(present_key); + Py_DECREF(missing_key); Py_RETURN_NONE; } @@ -1147,45 +1169,45 @@ test_dict_api(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) PyErr_Clear(); // test PyDict_GetItemRef(), key is present - get_value = Py_Ellipsis; // marker value + get_value = UNINITIALIZED_OBJ; assert(PyDict_GetItemRef(dict, key, &get_value) == 1); assert(get_value == value); Py_DECREF(get_value); // test PyDict_GetItemStringRef(), key is present - get_value = Py_Ellipsis; // marker value + get_value = UNINITIALIZED_OBJ; assert(PyDict_GetItemStringRef(dict, "key", &get_value) == 1); assert(get_value == value); Py_DECREF(get_value); // test PyDict_GetItemRef(), missing key - get_value = Py_Ellipsis; // marker value + get_value = UNINITIALIZED_OBJ; assert(PyDict_GetItemRef(dict, missing_key, &get_value) == 0); assert(!PyErr_Occurred()); assert(get_value == NULL); // test PyDict_GetItemStringRef(), missing key - get_value = Py_Ellipsis; // marker value + get_value = UNINITIALIZED_OBJ; assert(PyDict_GetItemStringRef(dict, "missing_key", &get_value) == 0); assert(!PyErr_Occurred()); assert(get_value == NULL); // test PyDict_GetItemRef(), invalid dict - get_value = Py_Ellipsis; // marker value + get_value = UNINITIALIZED_OBJ; assert(PyDict_GetItemRef(invalid_dict, key, &get_value) == -1); assert(PyErr_ExceptionMatches(PyExc_SystemError)); PyErr_Clear(); assert(get_value == NULL); // test PyDict_GetItemStringRef(), invalid dict - get_value = Py_Ellipsis; // marker value + get_value = UNINITIALIZED_OBJ; assert(PyDict_GetItemStringRef(invalid_dict, "key", &get_value) == -1); assert(PyErr_ExceptionMatches(PyExc_SystemError)); PyErr_Clear(); assert(get_value == NULL); // test PyDict_GetItemRef(), invalid key - get_value = Py_Ellipsis; // marker value + get_value = UNINITIALIZED_OBJ; assert(PyDict_GetItemRef(dict, invalid_key, &get_value) == -1); assert(PyErr_ExceptionMatches(PyExc_TypeError)); PyErr_Clear(); From d8ce583b0f13e66883a1db6823f75d8ac16d7daa Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 19 Sep 2023 13:59:38 +0200 Subject: [PATCH 2/2] Fix UNINITIALIZED_OBJ See https://github.com/python/cpython/commit/ed582a2ed980efba2d0da365ae37bff4a2b99873 --- tests/test_pythoncapi_compat_cext.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_pythoncapi_compat_cext.c b/tests/test_pythoncapi_compat_cext.c index d788017..888397a 100644 --- a/tests/test_pythoncapi_compat_cext.c +++ b/tests/test_pythoncapi_compat_cext.c @@ -51,7 +51,8 @@ #endif // Marker to check that pointer value was set -#define UNINITIALIZED_OBJ ((PyObject *)"uninitialized") +static const char uninitialized[] = "uninitialized"; +#define UNINITIALIZED_OBJ ((PyObject *)uninitialized) static PyObject*