BUG: non-integers can end up in dtype offsets #8080
Conversation
| @@ -1072,7 +1072,13 @@ _convert_from_dict(PyObject *obj, int align) | |||
| goto fail; | |||
| } | |||
| offset = PyInt_AsLong(off); | |||
| PyTuple_SET_ITEM(tup, 1, off); | |||
| if (PyErr_Occurred()) { | |||
There was a problem hiding this comment.
if ((offset == -1) && PyErr_Occured()) { avoids the check most of the time I think.
There was a problem hiding this comment.
Two rather unrelated side points I am not sure about:
- What about floats, etc.? Maybe it would make sense to use the Index conversion stuff?
- Where do we check
intsize? We only support integer sized offsets (since dtype is limited to int).
There was a problem hiding this comment.
what is the index conversion stuff? I don't think I'm familiar and searching didn't help.
There was a problem hiding this comment.
also PyInt_AsLong takes care of floats etc
There was a problem hiding this comment.
I mean our own (or the python version we mostly call there) NpyPyIntAsIntp (or whate it was), which specificially does not allow floats.
d730ee6 to
1063cb2
Compare
|
Got it, and updated. I also added a check that |
Fix is to convert offsets to python ints at dtype creation. Fixes numpy#8059
1063cb2 to
7efef9d
Compare
|
Nice. Hmmmpff, I may have thought too little about my comments, though. At least if we want to backport this, the more strict integer conversion could be a regression (though maybe it cannot be, because this is almost always only called indirectly). |
|
Anyone trying to use weird non-integer offsets would probably cause numpy to blow up in other ways ...probably. Also, I checked and #8059 is not a new bug I introduced, it is old. I checked in 1.8.0 and 1.10.0 and the examples raise exceptions back then too. Personally I would not worry about a backport, I think, since this is such an old and obscure oversight. |
|
OK, then I think I am happy. Thanks. |
Fix is to convert offsets to python ints at dtype creation.
Fixes #8059