From 98cebe4dedb52550ce621cf9338283dd7262ea83 Mon Sep 17 00:00:00 2001
From: Sebastian Berg <sebastian@sipsolutions.net>
Date: Sun, 3 Jul 2022 00:23:31 -0700
Subject: 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
---
 Cython/Compiler/ExprNodes.py | 17 ++++++++++-------
 tests/buffers/bufaccess.pyx  | 45 ++++++++++++++++++++++++++++++++++++++++++++
 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
@@ -1004,6 +1004,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):
     """
-- 
cgit v1.2.1