BUG: Fortify object buffers against included NULLs (#4859)

* BUG: Fortify object buffers against included NULLs

While NumPy tends to not actively create object buffers initialized
only with NULL (rather than filled with None), at least older versions
of NumPy did do that.  And NumPy guards against this.

This guards against embedded NULLs in object buffers interpreting
a NULL as None (and anticipating a NULL value also when setting
the buffer for reference count purposes).

Closes gh-4858

2 files changed, 55 insertions, 7 deletions

diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 9678647ad..69632a4fe 100644
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -4351,17 +4351,17 @@ class BufferIndexNode(_IndexingBaseNode):
         buffer_entry, ptrexpr = self.buffer_lookup_code(code)
 
         if self.buffer_type.dtype.is_pyobject:
-            # Must manage refcounts. Decref what is already there
-            # and incref what we put in.
+            # Must manage refcounts. XDecref what is already there
+            # and incref what we put in (NumPy allows there to be NULL)
             ptr = code.funcstate.allocate_temp(buffer_entry.buf_ptr_type,
                                                manage_ref=False)
             rhs_code = rhs.result()
             code.putln("%s = %s;" % (ptr, ptrexpr))
-            code.put_gotref("*%s" % ptr)
-            code.putln("__Pyx_INCREF(%s); __Pyx_DECREF(*%s);" % (
+            code.put_xgotref("*%s" % ptr)
+            code.putln("__Pyx_INCREF(%s); __Pyx_XDECREF(*%s);" % (
                 rhs_code, ptr))
             code.putln("*%s %s= %s;" % (ptr, op, rhs_code))
-            code.put_giveref("*%s" % ptr)
+            code.put_xgiveref("*%s" % ptr)
             code.funcstate.release_temp(ptr)
         else:
             # Simple case
@@ -4382,8 +4382,11 @@ class BufferIndexNode(_IndexingBaseNode):
             # is_temp is True, so must pull out value and incref it.
             # NOTE: object temporary results for nodes are declared
             #       as PyObject *, so we need a cast
-            code.putln("%s = (PyObject *) *%s;" % (self.result(), self.buffer_ptr_code))
-            code.putln("__Pyx_INCREF((PyObject*)%s);" % self.result())
+            res = self.result()
+            code.putln("%s = (PyObject *) *%s;" % (res, self.buffer_ptr_code))
+            # NumPy does (occasionally) allow NULL to denote None.
+            code.putln("if (unlikely(%s == NULL)) %s = Py_None;" % (res, res))
+            code.putln("__Pyx_INCREF((PyObject*)%s);" % res)
 
     def free_subexpr_temps(self, code):
         for temp in self.index_temps:
diff --git a/tests/buffers/bufaccess.pyx b/tests/buffers/bufaccess.pyx
index 8761e6eb9..2a5e84185 100644
--- a/tests/buffers/bufaccess.pyx
+++ b/tests/buffers/bufaccess.pyx
@@ -1005,6 +1005,51 @@ def assign_to_object(object[object] buf, int idx, obj):
     buf[idx] = obj
 
 @testcase
+def check_object_nulled_1d(MockBuffer[object, ndim=1] buf, int idx, obj):
+    """
+    See comments on printbuf_object above.
+
+    >>> a = object()
+    >>> rc1 = get_refcount(a)
+    >>> A = ObjectMockBuffer(None, [a, a])
+    >>> check_object_nulled_1d(A, 0, a)
+    >>> decref(a)  # new reference "added" to A
+    >>> check_object_nulled_1d(A, 1, a)
+    >>> decref(a)
+    >>> A = ObjectMockBuffer(None, [a, a, a, a], strides=(2,))
+    >>> check_object_nulled_1d(A, 0, a)  # only 0 due to stride
+    >>> decref(a)
+    >>> get_refcount(a) == rc1
+    True
+    """
+    cdef void **data = <void **>buf.buffer
+    data[idx] = NULL
+    res = buf[idx]  # takes None
+    buf[idx] = obj
+    return res
+
+@testcase
+def check_object_nulled_2d(MockBuffer[object, ndim=2] buf, int idx1, int idx2, obj):
+    """
+    See comments on printbuf_object above.
+
+    >>> a = object()
+    >>> rc1 = get_refcount(a)
+    >>> A = ObjectMockBuffer(None, [a, a, a, a], shape=(2, 2))
+    >>> check_object_nulled_2d(A, 0, 0, a)
+    >>> decref(a)  # new reference "added" to A
+    >>> check_object_nulled_2d(A, 1, 1, a)
+    >>> decref(a)
+    >>> get_refcount(a) == rc1
+    True
+    """
+    cdef void **data = <void **>buf.buffer
+    data[idx1 + 2*idx2] = NULL
+    res = buf[idx1, idx2]  # takes None
+    buf[idx1, idx2] = obj
+    return res
+
+@testcase
 def assign_temporary_to_object(object[object] buf):
     """
     See comments on printbuf_object above.