Skip to content

Commit

Permalink
Merge pull request #12354 from simongibbons/fix-fromfile-error-handling
Browse files Browse the repository at this point in the history
BUG: Fix segfault when an error occurs in np.fromfile
  • Loading branch information
eric-wieser committed Nov 10, 2018
2 parents d0e2f1a + fbdcb5b commit 9a39877
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 1 deletion.
30 changes: 30 additions & 0 deletions numpy/core/include/numpy/npy_3kcompat.h
Expand Up @@ -384,6 +384,36 @@ npy_PyFile_CloseFile(PyObject *file)
}


/* This is a copy of _PyErr_ChainExceptions
*/
static NPY_INLINE void
npy_PyErr_ChainExceptions(PyObject *exc, PyObject *val, PyObject *tb)
{
if (exc == NULL)
return;

if (PyErr_Occurred()) {
/* only py3 supports this anyway */
#ifdef NPY_PY3K
PyObject *exc2, *val2, *tb2;
PyErr_Fetch(&exc2, &val2, &tb2);
PyErr_NormalizeException(&exc, &val, &tb);
if (tb != NULL) {
PyException_SetTraceback(val, tb);
Py_DECREF(tb);
}
Py_DECREF(exc);
PyErr_NormalizeException(&exc2, &val2, &tb2);
PyException_SetContext(val2, val);
PyErr_Restore(exc2, val2, tb2);
#endif
}
else {
PyErr_Restore(exc, val, tb);
}
}


/* This is a copy of _PyErr_ChainExceptions, with:
* - a minimal implementation for python 2
* - __cause__ used instead of __context__
Expand Down
11 changes: 10 additions & 1 deletion numpy/core/src/multiarray/multiarraymodule.c
Expand Up @@ -2044,6 +2044,7 @@ static PyObject *
array_fromfile(PyObject *NPY_UNUSED(ignored), PyObject *args, PyObject *keywds)
{
PyObject *file = NULL, *ret;
PyObject *err_type = NULL, *err_value = NULL, *err_traceback = NULL;
char *sep = "";
Py_ssize_t nin = -1;
static char *kwlist[] = {"file", "dtype", "count", "sep", NULL};
Expand Down Expand Up @@ -2079,18 +2080,26 @@ array_fromfile(PyObject *NPY_UNUSED(ignored), PyObject *args, PyObject *keywds)
}
ret = PyArray_FromFile(fp, type, (npy_intp) nin, sep);

/* If an exception is thrown in the call to PyArray_FromFile
* we need to clear it, and restore it later to ensure that
* we can cleanup the duplicated file descriptor properly.
*/
PyErr_Fetch(&err_type, &err_value, &err_traceback);
if (npy_PyFile_DupClose2(file, fp, orig_pos) < 0) {
npy_PyErr_ChainExceptions(err_type, err_value, err_traceback);
goto fail;
}
if (own && npy_PyFile_CloseFile(file) < 0) {
npy_PyErr_ChainExceptions(err_type, err_value, err_traceback);
goto fail;
}
PyErr_Restore(err_type, err_value, err_traceback);
Py_DECREF(file);
return ret;

fail:
Py_DECREF(file);
Py_DECREF(ret);
Py_XDECREF(ret);
return NULL;
}

Expand Down
13 changes: 13 additions & 0 deletions numpy/core/tests/test_multiarray.py
Expand Up @@ -4541,6 +4541,19 @@ def test_file_position_after_tofile(self):
f.close()
assert_equal(pos, 10, err_msg=err_msg)

def test_load_object_array_fromfile(self):
# gh-12300
with open(self.filename, 'w') as f:
# Ensure we have a file with consistent contents
pass

with open(self.filename, 'rb') as f:
assert_raises_regex(ValueError, "Cannot read into object array",
np.fromfile, f, dtype=object)

assert_raises_regex(ValueError, "Cannot read into object array",
np.fromfile, self.filename, dtype=object)

def _check_from(self, s, value, **kw):
if 'sep' not in kw:
y = np.frombuffer(s, **kw)
Expand Down

0 comments on commit 9a39877

Please sign in to comment.