Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Prevent stackoverflow on self-containing arrays #9077

Merged
merged 3 commits into from
May 9, 2017

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented May 9, 2017

Approaches #8306. This still errors, but it does so at a python level, without
a segfault.

This makes the following not segfault:

a = np.array([None])
a[0] = a
bool(a)

On the other hand, it still behaves incorrectly

In [8]: bool(a)
---------------------------------------------------------------------------
RecursionError                            Traceback (most recent call last)
RecursionError: maximum recursion depth exceeded while converting array to bool

During handling of the above exception, another exception occurred:

SystemError                               Traceback (most recent call last)
<ipython-input-8-7af9bc0b6a67> in <module>()
----> 1 bool(a)

SystemError: <class 'bool'> returned a result with an error set

But that's a different bug (#9078)

@@ -786,10 +786,15 @@ static int
_array_nonzero(PyArrayObject *mp)
{
npy_intp n;
int res;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be put into the lower if scope


n = PyArray_SIZE(mp);
if (n == 1) {
return PyArray_DESCR(mp)->f->nonzero(PyArray_DATA(mp), mp);
if (Py_EnterRecursiveCall(" while converting array to bool"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs curly braces

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, was just copying cpython there

Approaches numpy#8306. This still errors, but it does so at a python level, without
a segfault.
@eric-wieser
Copy link
Member Author

@juliantaylor: Fixed

@juliantaylor
Copy link
Contributor

needs a test, then lgtm

@eric-wieser
Copy link
Member Author

Tests added - seems that we weren't testing bool(arr) at all

try:
Error = RecursionError
except NameError:
Error = RuntimeError
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should also have a python 2 comment

@juliantaylor
Copy link
Contributor

thanks

@juliantaylor juliantaylor merged commit 790dbcb into numpy:master May 9, 2017
@eric-wieser
Copy link
Member Author

For some reason, this doesn't fix a in a - I can't work out where the recursion is happening, as Py_RichCompare is supposed to already call Py_EnterRecursiveCall

@juliantaylor
Copy link
Contributor

DEPRECATE in arrayobject.c:1411 triggers another enter to the recursion checking. Once one of these errors has been thrown one must abort immediately.

@eric-wieser
Copy link
Member Author

eric-wieser commented May 9, 2017

Can we remove that deprecation in 1.14? We're now at the two year mark

@juliantaylor
Copy link
Contributor

probably not, as far as I recall that was one that caused enough noise to come up on the mailing list, but then I am of the opinion that deprecations are to almost never be removed, others might disagree.

@eric-wieser
Copy link
Member Author

then I am of the opinion that deprecations are to almost never be removed

Well in that case, the deprecation message shouldn't have been "this will raise an error in the future" ;). But fair enough, perhaps 1.14 is too soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants