BUG: protect against accessing base attribute of a NULL subarray#19326
BUG: protect against accessing base attribute of a NULL subarray#19326charris merged 4 commits intonumpy:mainfrom
Conversation
Have PyArray_GetCastSafety return -1 if from is NULL
|
If this seems like a reasonable fix, we can add a new test case for it |
|
#19322 makes other changes in the same file, but does not resolve the issue addressed here |
|
|
||
| PyArray_Descr *from_base = (from_sub == NULL) ? NULL : from_sub->base; | ||
| PyArray_Descr *to_base = (to_sub == NULL) ? NULL : to_sub->base; | ||
| NPY_CASTING field_casting = PyArray_GetCastSafety(from_base, to_base, NULL); |
There was a problem hiding this comment.
This is the right idea, but should be given_descrs[0] or given_descrs[1] rather than NULL. I guess the unstructured void case is missing (i.e. there must be a V4 or similar involved here somehow), I don't know yet how or why view even triggers it...
For tests, it may be good to use also check with np.can_cast (casting="unsafe").
There was a problem hiding this comment.
Ah, I had not realized that in your example x.dtype is "V40", than it all adds up.
There was a problem hiding this comment.
Sure, that works without requiring any change to PyArray_GetCastSafety itself. Let me look into a test case and I will update
|
The following is a minimal np.can_cast('V40', np.dtype([('foo', '<f4', (3, 2))]), casting="unsafe")Cases not involving the shape argument in the dtype seem fine on 1.21.0, e.g. np.can_cast(
'V40', np.dtype([('foo', '<i8'), ('bar', [('foo', '<i4'), ('baz', 'u1')])]), casting="unsafe"
)does not segfault |
|
Thanks, this looks good to me. If you are up to it, adding a test for the reverse direction would be good (I know its even stranger). I am still considering if there may be some incorrect corner cases for (The crazy part is that we the first element of the subarray, ignoring the rest! I don't want to backport such a change, but maybe we should consider that a bug... If you change it to |
seberg
left a comment
There was a problem hiding this comment.
Thanks, this works for me. There are probably more subtleties we should test, but there is no pressing reason to do it here.
|
Thanks @grlee77 . |
closes #19325
This PR is to protect against potential segfaults in
void_to_void_resolve_descriptors. This is the first time I have looked at this code, so there may be a better solution, but this seems to work for the example given in #19325