BUG: Fix refcount leak in ResultType

seberg · charris · commit 7d25b81025a5 · 2021-06-19T07:33:35.000-06:00
This slightly reorganizes the error path handling (duplicating
the freeing for the non-error path).  It just seemed a bit clearer.
diff --git a/numpy/core/src/multiarray/convert_datatype.c b/numpy/core/src/multiarray/convert_datatype.c
@@ -1649,14 +1649,14 @@ PyArray_ResultType(
         Py_DECREF(all_DTypes[i]);
     }
     if (common_dtype == NULL) {
-        goto finish;
+        goto error;
     }
 
     if (common_dtype->abstract) {
         /* (ab)use default descriptor to define a default */
         PyArray_Descr *tmp_descr = common_dtype->default_descr(common_dtype);
         if (tmp_descr == NULL) {
-            goto finish;
+            goto error;
         }
         Py_INCREF(NPY_DTYPE(tmp_descr));
         Py_SETREF(common_dtype, NPY_DTYPE(tmp_descr));
@@ -1689,20 +1689,18 @@ PyArray_ResultType(
                 PyObject *tmp = PyArray_GETITEM(
                         arrs[i-ndtypes], PyArray_BYTES(arrs[i-ndtypes]));
                 if (tmp == NULL) {
-                    Py_SETREF(result, NULL);
-                    goto finish;
+                    goto error;
                 }
                 curr = common_dtype->discover_descr_from_pyobject(common_dtype, tmp);
                 Py_DECREF(tmp);
             }
             if (curr == NULL) {
-                Py_SETREF(result, NULL);
-                goto finish;
+                goto error;
             }
             Py_SETREF(result, common_dtype->common_instance(result, curr));
             Py_DECREF(curr);
             if (result == NULL) {
-                goto finish;
+                goto error;
             }
         }
     }
@@ -1723,16 +1721,21 @@ PyArray_ResultType(
              * Going from error to success should not really happen, but is
              * probably OK if it does.
              */
-            Py_SETREF(result, NULL);
-            goto finish;
+            goto error;
         }
         /* Return the old "legacy" result (could warn here if different) */
         Py_SETREF(result, legacy_result);
     }
 
-  finish:
+    Py_DECREF(common_dtype);
     PyMem_Free(info_on_heap);
     return result;
+
+  error:
+    Py_XDECREF(result);
+    Py_XDECREF(common_dtype);
+    PyMem_Free(info_on_heap);
+    return NULL;
 }
 
 

Original file line number	Diff line number	Diff line change
`@@ -1649,14 +1649,14 @@ PyArray_ResultType(`
`1649`	`1649`	`Py_DECREF(all_DTypes[i]);`
`1650`	`1650`	`}`
`1651`	`1651`	`if (common_dtype == NULL) {`
`1652`		`- goto finish;`
	`1652`	`+ goto error;`
`1653`	`1653`	`}`
`1654`	`1654`
`1655`	`1655`	`if (common_dtype->abstract) {`
`1656`	`1656`	`/* (ab)use default descriptor to define a default */`
`1657`	`1657`	`PyArray_Descr *tmp_descr = common_dtype->default_descr(common_dtype);`
`1658`	`1658`	`if (tmp_descr == NULL) {`
`1659`		`- goto finish;`
	`1659`	`+ goto error;`
`1660`	`1660`	`}`
`1661`	`1661`	`Py_INCREF(NPY_DTYPE(tmp_descr));`
`1662`	`1662`	`Py_SETREF(common_dtype, NPY_DTYPE(tmp_descr));`
`@@ -1689,20 +1689,18 @@ PyArray_ResultType(`
`1689`	`1689`	`PyObject *tmp = PyArray_GETITEM(`
`1690`	`1690`	`arrs[i-ndtypes], PyArray_BYTES(arrs[i-ndtypes]));`
`1691`	`1691`	`if (tmp == NULL) {`
`1692`		`- Py_SETREF(result, NULL);`
`1693`		`- goto finish;`
	`1692`	`+ goto error;`
`1694`	`1693`	`}`
`1695`	`1694`	`curr = common_dtype->discover_descr_from_pyobject(common_dtype, tmp);`
`1696`	`1695`	`Py_DECREF(tmp);`
`1697`	`1696`	`}`
`1698`	`1697`	`if (curr == NULL) {`
`1699`		`- Py_SETREF(result, NULL);`
`1700`		`- goto finish;`
	`1698`	`+ goto error;`
`1701`	`1699`	`}`
`1702`	`1700`	`Py_SETREF(result, common_dtype->common_instance(result, curr));`
`1703`	`1701`	`Py_DECREF(curr);`
`1704`	`1702`	`if (result == NULL) {`
`1705`		`- goto finish;`
	`1703`	`+ goto error;`
`1706`	`1704`	`}`
`1707`	`1705`	`}`
`1708`	`1706`	`}`
`@@ -1723,16 +1721,21 @@ PyArray_ResultType(`
`1723`	`1721`	`* Going from error to success should not really happen, but is`
`1724`	`1722`	`* probably OK if it does.`
`1725`	`1723`	`*/`
`1726`		`- Py_SETREF(result, NULL);`
`1727`		`- goto finish;`
	`1724`	`+ goto error;`
`1728`	`1725`	`}`
`1729`	`1726`	`/* Return the old "legacy" result (could warn here if different) */`
`1730`	`1727`	`Py_SETREF(result, legacy_result);`
`1731`	`1728`	`}`
`1732`	`1729`
`1733`		`- finish:`
	`1730`	`+ Py_DECREF(common_dtype);`
`1734`	`1731`	`PyMem_Free(info_on_heap);`
`1735`	`1732`	`return result;`
	`1733`	`+`
	`1734`	`+ error:`
	`1735`	`+ Py_XDECREF(result);`
	`1736`	`+ Py_XDECREF(common_dtype);`
	`1737`	`+ PyMem_Free(info_on_heap);`
	`1738`	`+ return NULL;`
`1736`	`1739`	`}`
`1737`	`1740`
`1738`	`1741`