From 912ce16d0c22f9bf37a8f92b5fc6114fad6e7c08 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 24 Dec 2020 05:09:55 -0800 Subject: [PATCH] Demonstration of Undefined Behavior in handling of polymorphic pointers. (This demo does NOT involve smart pointers at all, unlike the otherwise similar test_smart_ptr_private_first_base.) --- tests/test_private_first_base.cpp | 66 +++++++++++++++++++++++++++++++ tests/test_private_first_base.py | 18 +++++++++ 2 files changed, 84 insertions(+) create mode 100644 tests/test_private_first_base.cpp create mode 100644 tests/test_private_first_base.py diff --git a/tests/test_private_first_base.cpp b/tests/test_private_first_base.cpp new file mode 100644 index 000000000..3e4f3e3a4 --- /dev/null +++ b/tests/test_private_first_base.cpp @@ -0,0 +1,66 @@ +// Demonstration of UB (Undefined Behavior) in handling of polymorphic pointers, +// specifically: +// https://github.com/pybind/pybind11/blob/30eb39ed79d1e2eeff15219ac00773034300a5e6/include/pybind11/cast.h#L229 +// `return reinterpret_cast(vh[0]);` +// casts a `void` pointer to a `base`. The `void` pointer is obtained through +// a `dynamic_cast` here: +// https://github.com/pybind/pybind11/blob/30eb39ed79d1e2eeff15219ac00773034300a5e6/include/pybind11/cast.h#L852 +// `return dynamic_cast(src);` +// The `dynamic_cast` is well-defined: +// https://en.cppreference.com/w/cpp/language/dynamic_cast +// 4) If expression is a pointer to a polymorphic type, and new-type +// is a pointer to void, the result is a pointer to the most derived +// object pointed or referenced by expression. +// But the `reinterpret_cast` above is UB: `test_make_drvd_pass_base` in +// `test_private_first_base.py` fails with a Segmentation Fault (Linux, +// clang++ -std=c++17). +// The only well-defined cast is back to a `drvd` pointer (`static_cast` can be +// used), which can then safely be cast up to a `base` pointer. Note that +// `test_make_drvd_up_cast_pass_drvd` passes because the `void` pointer is cast +// to `drvd` pointer in this situation. + +#include "pybind11_tests.h" + +namespace pybind11_tests { +namespace private_first_base { + +struct base { + base() : base_id(100) {} + virtual ~base() = default; + virtual int id() const { return base_id; } + base(const base&) = default; + int base_id; +}; + +struct private_first_base { // Any class with a virtual function will do. + private_first_base() {} + virtual void some_other_virtual_function() const {} + virtual ~private_first_base() = default; + private_first_base(const private_first_base&) = default; +}; + +struct drvd : private private_first_base, public base { + drvd() {} + int id() const override { return 2 * base_id; } +}; + +inline drvd* make_drvd() { return new drvd; } +inline base* make_drvd_up_cast() { return new drvd; } + +inline int pass_base(const base* b) { return b->id(); } +inline int pass_drvd(const drvd* d) { return d->id(); } + +TEST_SUBMODULE(private_first_base, m) { + py::class_(m, "base"); + py::class_(m, "drvd"); + + m.def("make_drvd", make_drvd, + py::return_value_policy::take_ownership); + m.def("make_drvd_up_cast", make_drvd_up_cast, + py::return_value_policy::take_ownership); + m.def("pass_base", pass_base); + m.def("pass_drvd", pass_drvd); +} + +} // namespace private_first_base +} // namespace pybind11_tests diff --git a/tests/test_private_first_base.py b/tests/test_private_first_base.py new file mode 100644 index 000000000..7004da50e --- /dev/null +++ b/tests/test_private_first_base.py @@ -0,0 +1,18 @@ +# -*- coding: utf-8 -*- +import pytest + +from pybind11_tests import private_first_base as m + + +def test_make_drvd_pass_base(): + d = m.make_drvd() + i = m.pass_base(d) + assert i == 200 + + +def test_make_drvd_up_cast_pass_drvd(): + b = m.make_drvd_up_cast() + # the base return is down-cast immediately. + assert b.__class__.__name__ == "drvd" + i = m.pass_drvd(b) + assert i == 200