Fix nullptr dereference when loading an external-only module_local type

This commit is contained in:
Dean Moldovan 2017-09-03 01:31:47 +02:00
parent 3c4933cb50
commit 7b1de1e551
6 changed files with 40 additions and 6 deletions

View File

@ -28,6 +28,10 @@ v2.2.1 (Not yet released)
* Relax overly strict ``py::picke()`` check for matching get and set types. * Relax overly strict ``py::picke()`` check for matching get and set types.
`#1064 <https://github.com/pybind/pybind11/pull/1064>`_. `#1064 <https://github.com/pybind/pybind11/pull/1064>`_.
* Fixed a nullptr dereference when loading a ``py::module_local`` type
that's only registered in an external module.
`#1058 <https://github.com/pybind/pybind11/pull/1058>`_.
v2.2.0 (August 31, 2017) v2.2.0 (August 31, 2017)
----------------------------------------------------- -----------------------------------------------------

View File

@ -482,9 +482,10 @@ inline PyObject *make_new_instance(PyTypeObject *type);
class type_caster_generic { class type_caster_generic {
public: public:
PYBIND11_NOINLINE type_caster_generic(const std::type_info &type_info) PYBIND11_NOINLINE type_caster_generic(const std::type_info &type_info)
: typeinfo(get_type_info(type_info)) { } : typeinfo(get_type_info(type_info)), cpptype(&type_info) { }
type_caster_generic(const type_info *typeinfo) : typeinfo(typeinfo) { } type_caster_generic(const type_info *typeinfo)
: typeinfo(typeinfo), cpptype(typeinfo ? typeinfo->cpptype : nullptr) { }
bool load(handle src, bool convert) { bool load(handle src, bool convert) {
return load_impl<type_caster_generic>(src, convert); return load_impl<type_caster_generic>(src, convert);
@ -610,7 +611,7 @@ public:
type_info *foreign_typeinfo = reinterpret_borrow<capsule>(getattr(pytype, local_key)); type_info *foreign_typeinfo = reinterpret_borrow<capsule>(getattr(pytype, local_key));
// Only consider this foreign loader if actually foreign and is a loader of the correct cpp type // Only consider this foreign loader if actually foreign and is a loader of the correct cpp type
if (foreign_typeinfo->module_local_load == &local_load if (foreign_typeinfo->module_local_load == &local_load
|| !same_type(*typeinfo->cpptype, *foreign_typeinfo->cpptype)) || (cpptype && !same_type(*cpptype, *foreign_typeinfo->cpptype)))
return false; return false;
if (auto result = foreign_typeinfo->module_local_load(src.ptr(), foreign_typeinfo)) { if (auto result = foreign_typeinfo->module_local_load(src.ptr(), foreign_typeinfo)) {
@ -722,6 +723,7 @@ public:
} }
const type_info *typeinfo = nullptr; const type_info *typeinfo = nullptr;
const std::type_info *cpptype = nullptr;
void *value = nullptr; void *value = nullptr;
}; };

View File

@ -8,9 +8,9 @@ public:
int i = -1; int i = -1;
}; };
/// Registered with py::local in both main and secondary modules: /// Registered with py::module_local in both main and secondary modules:
using LocalType = LocalBase<0>; using LocalType = LocalBase<0>;
/// Registered without py::local in both modules: /// Registered without py::module_local in both modules:
using NonLocalType = LocalBase<1>; using NonLocalType = LocalBase<1>;
/// A second non-local type (for stl_bind tests): /// A second non-local type (for stl_bind tests):
using NonLocal2 = LocalBase<2>; using NonLocal2 = LocalBase<2>;
@ -21,6 +21,10 @@ using MixedLocalGlobal = LocalBase<4>;
/// Mixed: global first, then local /// Mixed: global first, then local
using MixedGlobalLocal = LocalBase<5>; using MixedGlobalLocal = LocalBase<5>;
/// Registered with py::module_local only in the secondary module:
using ExternalType1 = LocalBase<6>;
using ExternalType2 = LocalBase<7>;
using LocalVec = std::vector<LocalType>; using LocalVec = std::vector<LocalType>;
using LocalVec2 = std::vector<NonLocal2>; using LocalVec2 = std::vector<NonLocal2>;
using LocalMap = std::unordered_map<std::string, LocalType>; using LocalMap = std::unordered_map<std::string, LocalType>;
@ -39,7 +43,7 @@ PYBIND11_MAKE_OPAQUE(NonLocalMap2);
// Simple bindings (used with the above): // Simple bindings (used with the above):
template <typename T, int Adjust, typename... Args> template <typename T, int Adjust = 0, typename... Args>
py::class_<T> bind_local(Args && ...args) { py::class_<T> bind_local(Args && ...args) {
return py::class_<T>(std::forward<Args>(args)...) return py::class_<T>(std::forward<Args>(args)...)
.def(py::init<int>()) .def(py::init<int>())

View File

@ -20,6 +20,10 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) {
// Definitions here are tested by importing both this module and the // Definitions here are tested by importing both this module and the
// relevant pybind11_tests submodule from a test_whatever.py // relevant pybind11_tests submodule from a test_whatever.py
// test_load_external
bind_local<ExternalType1>(m, "ExternalType1", py::module_local());
bind_local<ExternalType2>(m, "ExternalType2", py::module_local());
// test_exceptions.py // test_exceptions.py
m.def("raise_runtime_error", []() { PyErr_SetString(PyExc_RuntimeError, "My runtime error"); throw py::error_already_set(); }); m.def("raise_runtime_error", []() { PyErr_SetString(PyExc_RuntimeError, "My runtime error"); throw py::error_already_set(); });
m.def("raise_value_error", []() { PyErr_SetString(PyExc_ValueError, "My value error"); throw py::error_already_set(); }); m.def("raise_value_error", []() { PyErr_SetString(PyExc_ValueError, "My value error"); throw py::error_already_set(); });

View File

@ -15,6 +15,10 @@
#include <numeric> #include <numeric>
TEST_SUBMODULE(local_bindings, m) { TEST_SUBMODULE(local_bindings, m) {
// test_load_external
m.def("load_external1", [](ExternalType1 &e) { return e.i; });
m.def("load_external2", [](ExternalType2 &e) { return e.i; });
// test_local_bindings // test_local_bindings
// Register a class with py::module_local: // Register a class with py::module_local:
bind_local<LocalType, -1>(m, "LocalType", py::module_local()) bind_local<LocalType, -1>(m, "LocalType", py::module_local())

View File

@ -3,6 +3,22 @@ import pytest
from pybind11_tests import local_bindings as m from pybind11_tests import local_bindings as m
def test_load_external():
"""Load a `py::module_local` type that's only registered in an external module"""
import pybind11_cross_module_tests as cm
assert m.load_external1(cm.ExternalType1(11)) == 11
assert m.load_external2(cm.ExternalType2(22)) == 22
with pytest.raises(TypeError) as excinfo:
assert m.load_external2(cm.ExternalType1(21)) == 21
assert "incompatible function arguments" in str(excinfo.value)
with pytest.raises(TypeError) as excinfo:
assert m.load_external1(cm.ExternalType2(12)) == 12
assert "incompatible function arguments" in str(excinfo.value)
def test_local_bindings(): def test_local_bindings():
"""Tests that duplicate `py::module_local` class bindings work across modules""" """Tests that duplicate `py::module_local` class bindings work across modules"""