[BUG] Missing error checking for overflowing integer casts

[hawkinsp]

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)

pybind11/include/pybind11/pytypes.h

Line 1143 in 98f1bbb

? (T) PyLong_AsLong(m_ptr)

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

[YannickJadoul]

Huh, that's funny. What version of pybind11 are you on?

This seems very similar to #2336, but not really.

[hawkinsp]

I just verified this with 2.6.1, which appears to be current on Pypi.

[YannickJadoul]

Urgh. Let me check, then. Thanks for reporting, already!

[YannickJadoul]

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.

[tp-nan]

Hi, what's the status of this isssue now?