BUG: nditer: Memory leak from setting 'base' attribute two ways

Numpy uses its 'base' attribute in two different ways, both
to manage the memory of an external data buffer, and to track
an array for 'UPDATEIFCOPY' purposes. The nditer was creating
a temporary view with negative strides to match the strides of
input arrays, but then also wanted to use UPDATEIFCOPY. Unfortunately
this choice to overload 'base' for both purposes prevents that
from working, and the result was a memory leak.

This bug has been fixed by choosing instead to retain positive
strides and not fully match the order of input memory when
doing the UPDATEIFCOPY.

2 files changed, 7 insertions, 47 deletions

diff --git a/numpy/core/src/multiarray/nditer_constr.c b/numpy/core/src/multiarray/nditer_constr.c
index d02a9ae4f..941ce91c0 100644
--- a/numpy/core/src/multiarray/nditer_constr.c
+++ b/numpy/core/src/multiarray/nditer_constr.c
@@ -2599,7 +2599,6 @@ npyiter_new_temp_array(NpyIter *iter, PyTypeObject *subtype,
     npy_int8 *perm = NIT_PERM(iter);
     npy_intp new_shape[NPY_MAXDIMS], strides[NPY_MAXDIMS],
              stride = op_dtype->elsize;
-    char reversestride[NPY_MAXDIMS], anyreverse = 0;
     NpyIter_AxisData *axisdata;
     npy_intp sizeof_axisdata;
     npy_intp i;
@@ -2628,7 +2627,6 @@ npyiter_new_temp_array(NpyIter *iter, PyTypeObject *subtype,
     axisdata = NIT_AXISDATA(iter);
     sizeof_axisdata = NIT_AXISDATA_SIZEOF(itflags, ndim, nop);
 
-    memset(reversestride, 0, NPY_MAXDIMS);
     /* Initialize the strides to invalid values */
     for (i = 0; i < NPY_MAXDIMS; ++i) {
         strides[i] = NPY_MAX_INTP;
@@ -2652,13 +2650,6 @@ npyiter_new_temp_array(NpyIter *iter, PyTypeObject *subtype,
                                     "for iterator dimension %d to %d\n", (int)i,
                                     (int)idim, (int)stride);
                 strides[i] = stride;
-                if (p < 0) {
-                    reversestride[i] = 1;
-                    anyreverse = 1;
-                }
-                else {
-                    reversestride[i] = 0;
-                }
                 if (shape == NULL) {
                     new_shape[i] = NAD_SHAPE(axisdata);
                     stride *= new_shape[i];
@@ -2721,13 +2712,6 @@ npyiter_new_temp_array(NpyIter *iter, PyTypeObject *subtype,
                                     "for iterator dimension %d to %d\n", (int)i,
                                     (int)idim, (int)stride);
                 strides[i] = stride;
-                if (p < 0) {
-                    reversestride[i] = 1;
-                    anyreverse = 1;
-                }
-                else {
-                    reversestride[i] = 0;
-                }
                 if (shape == NULL) {
                     new_shape[i] = NAD_SHAPE(axisdata);
                     stride *= new_shape[i];
@@ -2816,33 +2800,6 @@ npyiter_new_temp_array(NpyIter *iter, PyTypeObject *subtype,
         return NULL;
     }
 
-    /* If there are any reversed axes, create a view that reverses them */
-    if (anyreverse) {
-        char *dataptr = PyArray_DATA(ret);
-        PyArrayObject *newret;
-
-        for (idim = 0; idim < op_ndim; ++idim) {
-            if (reversestride[idim]) {
-                dataptr += strides[idim]*(shape[idim]-1);
-                strides[idim] = -strides[idim];
-            }
-        }
-        Py_INCREF(op_dtype);
-        newret = (PyArrayObject *)PyArray_NewFromDescr(subtype,
-                              op_dtype, op_ndim,
-                              shape, strides, dataptr,
-                              NPY_ARRAY_WRITEABLE, NULL);
-        if (newret == NULL) {
-            Py_DECREF(ret);
-            return NULL;
-        }
-        if (PyArray_SetBaseObject(newret, (PyObject *)ret) < 0) {
-            Py_DECREF(newret);
-            return NULL;
-        }
-        ret = newret;
-    }
-
     /* Make sure all the flags are good */
     PyArray_UpdateFlags(ret, NPY_ARRAY_UPDATE_ALL);
 
diff --git a/numpy/core/tests/test_nditer.py b/numpy/core/tests/test_nditer.py
index 5bb359406..3d18a9b98 100644
--- a/numpy/core/tests/test_nditer.py
+++ b/numpy/core/tests/test_nditer.py
@@ -840,6 +840,7 @@ def test_iter_array_cast():
     assert_equal(i.operands[0], a)
     assert_equal(i.operands[0].dtype, np.dtype('f8'))
     # The memory layout of the temporary should match a (a is (48,4,16))
+    # except negative strides get flipped to positive strides.
     assert_equal(i.operands[0].strides, (96,8,32))
     a = a[::-1,:,::-1]
     i = nditer(a, [], [['readonly','copy']],
@@ -847,7 +848,7 @@ def test_iter_array_cast():
             op_dtypes=[np.dtype('f8')])
     assert_equal(i.operands[0], a)
     assert_equal(i.operands[0].dtype, np.dtype('f8'))
-    assert_equal(i.operands[0].strides, (-96,8,-32))
+    assert_equal(i.operands[0].strides, (96,8,32))
 
     # Same-kind cast 'f8' -> 'f4' -> 'f8'
     a = np.arange(24, dtype='f8').reshape(2,3,4).T
@@ -871,10 +872,12 @@ def test_iter_array_cast():
             casting='unsafe',
             op_dtypes=[np.dtype('f4')])
     assert_equal(i.operands[0].dtype, np.dtype('f4'))
-    assert_equal(i.operands[0].strides, (-4,))
-    i.operands[0][:] = 1
+    # Even though the stride was negative in 'a', it
+    # becomes positive in the temporary
+    assert_equal(i.operands[0].strides, (4,))
+    i.operands[0][:] = [1,2,3]
     i = None
-    assert_equal(a, [1,1,1])
+    assert_equal(a, [1,2,3])
 
 def test_iter_array_cast_errors():
     # Check that invalid casts are caught