Zengtudor/pybind11
1
0
Fork 0
mirror of https://github.com/pybind/pybind11.git synced 2024-11-22 13:15:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix unsigned error value casting

Browse Source 
When casting to an unsigned type from a python 2 `int`, we currently
cast using `(unsigned long long) PyLong_AsUnsignedLong(src.ptr())`.
If the Python cast fails, it returns (unsigned long) -1, but then we
cast this to `unsigned long long`, which means we get 4294967295, but
because that isn't equal to `(unsigned long long) -1`, we don't detect
the failure.

This commit moves the unsigned casting into a `detail::as_unsigned`
function which, upon error, casts -1 to the final type, and otherwise
casts the return value to the final type to avoid the problematic double
cast when an error occurs.

The error most commonly shows up wherever `long` is 32-bits (e.g. under
both 32- and 64-bit Windows, and under 32-bit linux) when passing a
negative value to a bound function taking an `unsigned long`.

Fixes #929.

The added tests also trigger a latent segfault under PyPy: when casting
to an integer smaller than `long` (e.g. casting to a `uint32_t` on a
64-bit `long` architecture) we check both for a Python error and also
that the resulting intermediate value will fit in the final type.  If
there is no conversion error, but we get a value that would overflow, we
end up calling `PyErr_ExceptionMatches()` illegally: that call is only
allowed when there is a current exception.  Under PyPy, this segfaults
the test suite.  It doesn't appear to segfault under CPython, but the
documentation suggests that it *could* do so.  The fix is to only check
for the exception match if we actually got an error.
This commit is contained in:
Jason Rhinelander 2017-07-01 16:31:49 -04:00
parent 30f6c3b36e
commit 259b2fafea
5 changed files with 87 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
include/pybind11/cast.h
View File

@ -929,31 +929,27 @@ public:
py_value = (py_type) PyFloat_AsDouble(src.ptr());
else
return false;
} else if (sizeof(T) <= sizeof(long)) {
if (PyFloat_Check(src.ptr()))
return false;
if (std::is_signed<T>::value)
py_value = (py_type) PyLong_AsLong(src.ptr());
else
py_value = (py_type) PyLong_AsUnsignedLong(src.ptr());
} else {
if (PyFloat_Check(src.ptr()))
return false;
if (std::is_signed<T>::value)
py_value = (py_type) PYBIND11_LONG_AS_LONGLONG(src.ptr());
else
py_value = (py_type) PYBIND11_LONG_AS_UNSIGNED_LONGLONG(src.ptr());
} else if (PyFloat_Check(src.ptr())) {
return false;
} else if (std::is_unsigned<py_type>::value) {
py_value = as_unsigned<py_type>(src.ptr());
} else { // signed integer:
py_value = sizeof(T) <= sizeof(long)
? (py_type) PyLong_AsLong(src.ptr())
: (py_type) PYBIND11_LONG_AS_LONGLONG(src.ptr());
}
if ((py_value == (py_type) -1 && PyErr_Occurred()) ||
(std::is_integral<T>::value && sizeof(py_type) != sizeof(T) &&
(py_value < (py_type) std::numeric_limits<T>::min() ||
py_value > (py_type) std::numeric_limits<T>::max()))) {
bool py_err = py_value == (py_type) -1 && PyErr_Occurred();
if (py_err || (std::is_integral<T>::value && sizeof(py_type) != sizeof(T) &&
(py_value < (py_type) std::numeric_limits<T>::min() ||
py_value > (py_type) std::numeric_limits<T>::max()))) {
bool type_error = py_err && PyErr_ExceptionMatches(
#if PY_VERSION_HEX < 0x03000000 && !defined(PYPY_VERSION)
bool type_error = PyErr_ExceptionMatches(PyExc_SystemError);
PyExc_SystemError
#else
bool type_error = PyErr_ExceptionMatches(PyExc_TypeError);
PyExc_TypeError
#endif
);
PyErr_Clear();
if (type_error && convert && PyNumber_Check(src.ptr())) {
auto tmp = reinterpret_borrow<object>(std::is_floating_point<T>::value

2
include/pybind11/common.h
View File

@ -147,7 +147,6 @@
#define PYBIND11_BYTES_SIZE PyBytes_Size
#define PYBIND11_LONG_CHECK(o) PyLong_Check(o)
#define PYBIND11_LONG_AS_LONGLONG(o) PyLong_AsLongLong(o)
#define PYBIND11_LONG_AS_UNSIGNED_LONGLONG(o) PyLong_AsUnsignedLongLong(o)
#define PYBIND11_BYTES_NAME "bytes"
#define PYBIND11_STRING_NAME "str"
#define PYBIND11_SLICE_OBJECT PyObject
@ -167,7 +166,6 @@
#define PYBIND11_BYTES_SIZE PyString_Size
#define PYBIND11_LONG_CHECK(o) (PyInt_Check(o) || PyLong_Check(o))
#define PYBIND11_LONG_AS_LONGLONG(o) (PyInt_Check(o) ? (long long) PyLong_AsLong(o) : PyLong_AsLongLong(o))
#define PYBIND11_LONG_AS_UNSIGNED_LONGLONG(o) (PyInt_Check(o) ? (unsigned long long) PyLong_AsUnsignedLong(o) : PyLong_AsUnsignedLongLong(o))
#define PYBIND11_BYTES_NAME "str"
#define PYBIND11_STRING_NAME "unicode"
#define PYBIND11_SLICE_OBJECT PySliceObject

38
include/pybind11/pytypes.h
View File

@ -931,6 +931,28 @@ private:
}
};
NAMESPACE_BEGIN(detail)
// Converts a value to the given unsigned type. If an error occurs, you get back (Unsigned) -1;
// otherwise you get back the unsigned long or unsigned long long value cast to (Unsigned).
// (The distinction is critically important when casting a returned -1 error value to some other
// unsigned type: (A)-1 != (B)-1 when A and B are unsigned types of different sizes).
template <typename Unsigned>
Unsigned as_unsigned(PyObject *o) {
if (sizeof(Unsigned) <= sizeof(unsigned long)
#if PY_VERSION_HEX < 0x03000000
|| PyInt_Check(o)
#endif
) {
unsigned long v = PyLong_AsUnsignedLong(o);
return v == (unsigned long) -1 && PyErr_Occurred() ? (Unsigned) -1 : (Unsigned) v;
}
else {
unsigned long long v = PyLong_AsUnsignedLongLong(o);
return v == (unsigned long long) -1 && PyErr_Occurred() ? (Unsigned) -1 : (Unsigned) v;
}
}
NAMESPACE_END(detail)
class int_ : public object {
public:
PYBIND11_OBJECT_CVT(int_, object, PYBIND11_LONG_CHECK, PyNumber_Long)
@ -956,17 +978,11 @@ public:
template <typename T,
detail::enable_if_t<std::is_integral<T>::value, int> = 0>
operator T() const {
if (sizeof(T) <= sizeof(long)) {
if (std::is_signed<T>::value)
return (T) PyLong_AsLong(m_ptr);
else
return (T) PyLong_AsUnsignedLong(m_ptr);
} else {
if (std::is_signed<T>::value)
return (T) PYBIND11_LONG_AS_LONGLONG(m_ptr);
else
return (T) PYBIND11_LONG_AS_UNSIGNED_LONGLONG(m_ptr);
}
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);
}
};

6
tests/test_builtin_casters.cpp
View File

@ -72,6 +72,12 @@ TEST_SUBMODULE(builtin_casters, m) {
m.def("string_view32_return", []() { return std::u32string_view(U"utf32 secret \U0001f382"); });
#endif
// test_integer_casting
m.def("i32_str", [](std::int32_t v) { return std::to_string(v); });
m.def("u32_str", [](std::uint32_t v) { return std::to_string(v); });
m.def("i64_str", [](std::int64_t v) { return std::to_string(v); });
m.def("u64_str", [](std::uint64_t v) { return std::to_string(v); });
// test_tuple
m.def("pair_passthrough", [](std::pair<bool, std::string> input) {
return std::make_pair(input.second, input.first);

38
tests/test_builtin_casters.py
View File

@ -143,6 +143,44 @@ def test_string_view(capture):
"""
def test_integer_casting():
"""Issue #929 - out-of-range integer values shouldn't be accepted"""
import sys
assert m.i32_str(-1) == "-1"
assert m.i64_str(-1) == "-1"
assert m.i32_str(2000000000) == "2000000000"
assert m.u32_str(2000000000) == "2000000000"
if sys.version_info < (3,):
assert m.i32_str(long(-1)) == "-1"
assert m.i64_str(long(-1)) == "-1"
assert m.i64_str(long(-999999999999)) == "-999999999999"
assert m.u64_str(long(999999999999)) == "999999999999"
else:
assert m.i64_str(-999999999999) == "-999999999999"
assert m.u64_str(999999999999) == "999999999999"
with pytest.raises(TypeError) as excinfo:
m.u32_str(-1)
assert "incompatible function arguments" in str(excinfo.value)
with pytest.raises(TypeError) as excinfo:
m.u64_str(-1)
assert "incompatible function arguments" in str(excinfo.value)
with pytest.raises(TypeError) as excinfo:
m.i32_str(-3000000000)
assert "incompatible function arguments" in str(excinfo.value)
with pytest.raises(TypeError) as excinfo:
m.i32_str(3000000000)
assert "incompatible function arguments" in str(excinfo.value)
if sys.version_info < (3,):
with pytest.raises(TypeError) as excinfo:
m.u32_str(long(-1))
assert "incompatible function arguments" in str(excinfo.value)
with pytest.raises(TypeError) as excinfo:
m.u64_str(long(-1))
assert "incompatible function arguments" in str(excinfo.value)
def test_tuple(doc):
"""std::pair <-> tuple & std::tuple <-> tuple"""
assert m.pair_passthrough((True, "test")) == ("test", True)