BUG: Validate All Py_* Functions in C Extensions check for NULL

[WillAyd]

@jbrockmendel pointed out in #49034 that we don't consistently validate the return of Py_* functions in the C API. The net effect of this is that our extensions are much harder to refactor, as our lack of checks allows at worst for undefined behavior and at best for segfaults.

If someone were to audit the extensions we could easily add in error handling where missing and shore up these extensions. See also https://docs.python.org/3/c-api/exceptions.html#

[MarcoGorelli]

Does it have to check NULL, or are other checks allowed?

From a "quick and dirty" check, here's a couple I found:

pandas/pandas/_libs/src/ujson/python/date_conversions.c

Lines 92 to 117 in 009c4c6

	char *result = PyObject_Malloc(*len);
	// Check to see if PyDateTime has a timezone.
	// Don't convert to UTC if it doesn't.
	int is_tz_aware = 0;
	if (PyObject_HasAttrString(obj, "tzinfo")) {
	PyObject *offset = extract_utc_offset(obj);
	if (offset == NULL) {
	PyObject_Free(result);
	return NULL;
	}
	is_tz_aware = offset != Py_None;
	Py_DECREF(offset);
	}
	ret = make_iso_8601_datetime(&dts, result, *len, is_tz_aware, base);
	if (ret != 0) {
	PyErr_SetString(PyExc_ValueError,
	"Could not convert datetime value to string");
	PyObject_Free(result);
	return NULL;
	}
	// Note that get_datetime_iso_8601_strlen just gives a generic size
	// for ISO string conversion, not the actual size used
	*len = strlen(result);
	return result;

pandas/pandas/_libs/src/parser/io.c

Line 68 in 348d43f

func = PyObject_GetAttrString(src->obj, "read");

pandas/pandas/_libs/src/ujson/python/JSONtoObj.c

Line 121 in 2410fca

npyarr->shape.ptr = PyObject_Malloc(sizeof(npy_intp) * NPY_MAXDIMS);

pandas/pandas/_libs/src/ujson/python/JSONtoObj.c

Lines 240 to 244 in 2410fca

	type = PyObject_Type(value);
	if (!PyArray_DescrConverter(type, &dtype)) {
	Py_DECREF(type);
	goto fail;
	}

pandas/pandas/_libs/src/ujson/python/objToJSON.c

Lines 185 to 192 in 2f44dba

	PyObject *tz = PyObject_GetAttrString(obj, "tz");
	if (tz != Py_None) {
	// Go through object array if we have dt64tz, since tz info will
	// be lost if values is used directly.
	Py_DECREF(tz);
	values = PyObject_CallMethod(obj, "__array__", NULL);
	return values;
	}

but I don't know which are allowed and which not. tz != Py_None looks fine, for example? But in the second example, I don't see any validation of func (or does Py_XDECREF(func) count?)

[WillAyd]

Those are all good examples of incorrect code. Yea they should all be checking for NULL

[WillAyd]

At least all PyObject* funcs. Other Py* funcs May not return a pointer so could have different requirements (ex: -1 for integral return values)

[jbrockmendel]

ive asked enough times to know this a pipe dream, but this logic would be so much more maintainable if it lived in cython