pybind11
pybind11 copied to clipboard
pybind
[BUG] Missing error checking for overflowing integer casts
Issue description
There appears to be some missing error checking around Python int to C++ long casts that may overflow. In the example below, since the Python -> C++ cast fails, I expect a C++ exception to be thrown. However, it appears the Python interpreter's error state ends up set but pybind11 fails to notice.
I suspect that this method needs to check the interpreter's error state (since PyLong_AsLong may fail if the integer is larger than a C long)
https://github.com/pybind/pybind11/blob/98f1bbb8004f654ba9e26717bdf5912fb899b05a/include/pybind11/pytypes.h#L1143
Reproducible example code
#include <pybind11/pybind11.h>
#include <iostream>
int docast(pybind11::object a) {
int b = pybind11::cast<pybind11::int_>(a);
std::cerr << "Should not reach here" << std::endl;
return b;
}
PYBIND11_MODULE(t, m) {
m.def("docast", &docast, "Perform a cast");
}
In [1]: import t
In [2]: t.docast(3123412423423423234234234)
Should not reach here
---------------------------------------------------------------------------
OverflowError Traceback (most recent call last)
OverflowError: Python int too large to convert to C long
The above exception was the direct cause of the following exception:
SystemError Traceback (most recent call last)
<ipython-input-3-688cef4b7a48> in <module>
----> 1 t.docast(3123412423423423234234234)
SystemError: <built-in method docast of PyCapsule object at 0x102917720> returned a result with an error set
Huh, that's funny. What version of pybind11 are you on?
This seems very similar to #2336, but not really.
The error seems to be in the implicit conversion from py::int_ to int:
template <typename T,
detail::enable_if_t<std::is_integral<T>::value, int> = 0>
operator T() const {
return std::is_unsigned<T>::value
? detail::as_unsigned<T>(m_ptr)
: sizeof(T) <= sizeof(long)
? (T) PyLong_AsLong(m_ptr)
: (T) PYBIND11_LONG_AS_LONGLONG(m_ptr);
}
This should definitely throw an exception if an error occurs. The fix seems reasonably trivial (check for -1 and PyErr_Occurred() and throw error_already_set, if so). However, I need to make some tests first, to make sure we fix this and don't regress. Somewhere this week should be possible to do so.
