Skip to content

P
pybind11
Commit 1ac08dbc by Ralf W. Grosse-Kunstleve
Options

Fixing bugs discovered by ASAN. The code is now ASAN, MSAN, UBSAN clean.

parent c3627d21
Showing with 26 additions and 29 deletions
include/pybind11/classh.h
...@@ -266,37 +266,37 @@ public: ...@@ -266,37 +266,37 @@ public:
} }
private: private:
/// Initialize holder object, variant 1: object derives from enable_shared_from_this
template <typename T> template <typename T>
static void init_holder(detail::value_and_holder &/*v_h*/, static void init_holder(bool /*owned*/, detail::value_and_holder &/*v_h*/,
const holder_type * /* unused */, const std::enable_shared_from_this<T> * /* dummy */) { holder_type * /* unused */, const std::enable_shared_from_this<T> * /* dummy */) {
throw std::runtime_error("Not implemented: classh::init_holder enable_shared_from_this."); throw std::runtime_error("Not implemented: classh::init_holder enable_shared_from_this.");
} }
/// Initialize holder object, variant 2: try to construct from existing holder object, if possible static void init_holder(bool owned, detail::value_and_holder &v_h,
static void init_holder(detail::value_and_holder &v_h, holder_type *holder_ptr, const void * /* dummy -- not enable_shared_from_this<T>) */) {
const holder_type *holder_ptr, const void * /* dummy -- not enable_shared_from_this<T>) */) {
if (holder_ptr) { if (holder_ptr) {
new (std::addressof(v_h.holder<holder_type>())) holder_type(std::move(*holder_ptr));
v_h.set_holder_constructed();
} else { // Was: if (inst->owned || detail::always_construct_holder<holder_type>::value)
new (std::addressof(v_h.holder<holder_type>())) holder_type( new (std::addressof(v_h.holder<holder_type>())) holder_type(
std::move(holder_type::from_raw_ptr_take_ownership(v_h.value_ptr<type>()))); std::move(*holder_ptr));
v_h.set_holder_constructed(); } else if (owned) {
new (std::addressof(v_h.holder<holder_type>())) holder_type(
holder_type::from_raw_ptr_take_ownership(v_h.value_ptr<type>()));
}
else {
new (std::addressof(v_h.holder<holder_type>())) holder_type(
holder_type::from_raw_ptr_unowned(v_h.value_ptr<type>()));
} }
v_h.set_holder_constructed();
} }
/// Performs instance initialization including constructing a holder and registering the known
/// instance. Should be called as soon as the `type` value_ptr is set for an instance. Takes an
/// optional pointer to an existing holder to use; if not specified and the instance is
/// `.owned`, a new holder will be constructed to manage the value pointer.
static void init_instance(detail::instance *inst, const void *holder_ptr) { static void init_instance(detail::instance *inst, const void *holder_ptr) {
auto v_h = inst->get_value_and_holder(detail::get_type_info(typeid(type))); auto v_h = inst->get_value_and_holder(detail::get_type_info(typeid(type)));
if (!v_h.instance_registered()) { if (!v_h.instance_registered()) {
register_instance(inst, v_h.value_ptr(), v_h.type); register_instance(inst, v_h.value_ptr(), v_h.type);
v_h.set_instance_registered(); v_h.set_instance_registered();
} }
init_holder(v_h, static_cast<const holder_type *>(holder_ptr), v_h.value_ptr<type>()); // Need for const_cast is a consequence of the type_info::init_instance type:
// void (*init_instance)(instance *, const void *);
init_holder(inst->owned, v_h, static_cast<holder_type *>(const_cast<void *>(holder_ptr)), v_h.value_ptr<type>());
} }
/// Deallocates an instance; via holder, if constructed; otherwise via operator delete. /// Deallocates an instance; via holder, if constructed; otherwise via operator delete.
......
tests/test_classh_wip.cpp
...@@ -15,11 +15,11 @@ struct mpty { ...@@ -15,11 +15,11 @@ struct mpty {
// clang-format off // clang-format off
mpty rtrn_mpty_valu() { mpty obj{"rtrn_valu"}; return obj; } mpty rtrn_mpty_valu() { mpty obj{"rtrn_valu"}; return obj; }
mpty&& rtrn_mpty_rref() { mpty obj{"rtrn_rref"}; return std::move(obj); } mpty&& rtrn_mpty_rref() { static mpty obj; obj.mtxt = "rtrn_rref"; return std::move(obj); }
mpty const& rtrn_mpty_cref() { static mpty obj; obj.mtxt = "rtrn_cref"; return obj; } mpty const& rtrn_mpty_cref() { static mpty obj; obj.mtxt = "rtrn_cref"; return obj; }
mpty& rtrn_mpty_mref() { static mpty obj; obj.mtxt = "rtrn_mref"; return obj; } mpty& rtrn_mpty_mref() { static mpty obj; obj.mtxt = "rtrn_mref"; return obj; }
mpty const* rtrn_mpty_cptr() { static mpty obj; obj.mtxt = "rtrn_cptr"; return &obj; } mpty const* rtrn_mpty_cptr() { return new mpty{"rtrn_cptr"}; }
mpty* rtrn_mpty_mptr() { static mpty obj; obj.mtxt = "rtrn_mptr"; return &obj; } mpty* rtrn_mpty_mptr() { return new mpty{"rtrn_mptr"}; }
std::string pass_mpty_valu(mpty obj) { return "pass_valu:" + obj.mtxt; } std::string pass_mpty_valu(mpty obj) { return "pass_valu:" + obj.mtxt; }
std::string pass_mpty_rref(mpty&& obj) { return "pass_rref:" + obj.mtxt; } std::string pass_mpty_rref(mpty&& obj) { return "pass_rref:" + obj.mtxt; }
...@@ -315,7 +315,9 @@ struct type_caster<std::shared_ptr<mpty>> : smart_holder_type_caster_load<mpty> ...@@ -315,7 +315,9 @@ struct type_caster<std::shared_ptr<mpty>> : smart_holder_type_caster_load<mpty>
object inst = reinterpret_steal<object>(make_new_instance(tinfo->type)); object inst = reinterpret_steal<object>(make_new_instance(tinfo->type));
instance *inst_raw_ptr = reinterpret_cast<instance *>(inst.ptr()); instance *inst_raw_ptr = reinterpret_cast<instance *>(inst.ptr());
inst_raw_ptr->owned = false; // Not actually used. inst_raw_ptr->owned = true;
void *&valueptr = values_and_holders(inst_raw_ptr).begin()->value_ptr();
valueptr = src_raw_void_ptr;
auto smhldr = pybindit::memory::smart_holder::from_shared_ptr(src); auto smhldr = pybindit::memory::smart_holder::from_shared_ptr(src);
tinfo->init_instance(inst_raw_ptr, static_cast<const void *>(&smhldr)); tinfo->init_instance(inst_raw_ptr, static_cast<const void *>(&smhldr));
...@@ -380,7 +382,9 @@ struct type_caster<std::unique_ptr<mpty>> : smart_holder_type_caster_load<mpty> ...@@ -380,7 +382,9 @@ struct type_caster<std::unique_ptr<mpty>> : smart_holder_type_caster_load<mpty>
object inst = reinterpret_steal<object>(make_new_instance(tinfo->type)); object inst = reinterpret_steal<object>(make_new_instance(tinfo->type));
instance *inst_raw_ptr = reinterpret_cast<instance *>(inst.ptr()); instance *inst_raw_ptr = reinterpret_cast<instance *>(inst.ptr());
inst_raw_ptr->owned = false; // Not actually used. inst_raw_ptr->owned = true;
void *&valueptr = values_and_holders(inst_raw_ptr).begin()->value_ptr();
valueptr = src_raw_void_ptr;
auto smhldr = pybindit::memory::smart_holder::from_unique_ptr(std::move(src)); auto smhldr = pybindit::memory::smart_holder::from_unique_ptr(std::move(src));
tinfo->init_instance(inst_raw_ptr, static_cast<const void *>(&smhldr)); tinfo->init_instance(inst_raw_ptr, static_cast<const void *>(&smhldr));
......
tests/test_classh_wip.py
...@@ -15,20 +15,13 @@ def test_mpty_constructors(): ...@@ -15,20 +15,13 @@ def test_mpty_constructors():
def test_cast(): def test_cast():
assert m.get_mtxt(m.rtrn_mpty_valu()) == "rtrn_valu" assert m.get_mtxt(m.rtrn_mpty_valu()) == "rtrn_valu"
# rtrn_rref exercised separately. assert m.get_mtxt(m.rtrn_mpty_rref()) == "rtrn_rref"
assert m.get_mtxt(m.rtrn_mpty_cref()) == "rtrn_cref" assert m.get_mtxt(m.rtrn_mpty_cref()) == "rtrn_cref"
assert m.get_mtxt(m.rtrn_mpty_mref()) == "rtrn_mref" assert m.get_mtxt(m.rtrn_mpty_mref()) == "rtrn_mref"
assert m.get_mtxt(m.rtrn_mpty_cptr()) == "rtrn_cptr" assert m.get_mtxt(m.rtrn_mpty_cptr()) == "rtrn_cptr"
assert m.get_mtxt(m.rtrn_mpty_mptr()) == "rtrn_mptr" assert m.get_mtxt(m.rtrn_mpty_mptr()) == "rtrn_mptr"
def test_cast_rref():
e = m.rtrn_mpty_rref()
assert e.__class__.__name__ == "mpty"
with pytest.raises(RuntimeError):
m.get_mtxt(e) # E.g. basic_string::_M_construct null not valid
def test_load(): def test_load():
assert m.pass_mpty_valu(m.mpty("Valu")) == "pass_valu:Valu" assert m.pass_mpty_valu(m.mpty("Valu")) == "pass_valu:Valu"
assert m.pass_mpty_rref(m.mpty("Rref")) == "pass_rref:Rref" assert m.pass_mpty_rref(m.mpty("Rref")) == "pass_rref:Rref"
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment