From ad01e5ad713a3d2f3fa974ecfc8fbab186e3b1de Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Sat, 10 Dec 2022 10:11:49 +0100 Subject: [PATCH 1/3] segfault in test_stl_binders.py:test_map_string_double requires: - a build with all optimizations disabled - the definition of a function accepting std::map --- .github/workflows/ci.yml | 10 ++++++++++ tests/test_stl.cpp | 1 + 2 files changed, 11 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1c6a63f409..7802d0c366 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,6 +47,12 @@ jobs: args: > -DPYBIND11_FINDPYTHON=ON -DCMAKE_CXX_FLAGS="-D_=1" + - runs-on: ubuntu-20.04 + python: '3.8' + # Add a build with no optimizations + args: > + -DPYBIND11_FINDPYTHON=ON + -DCMAKE_BUILD_TYPE=none -DCMAKE_CXX_FLAGS="-DNDEBUG" - runs-on: ubuntu-20.04 python: 'pypy-3.8' args: > @@ -293,6 +299,9 @@ jobs: std: 20 - clang: 14 std: 20 + - clang: 10 + std: 14 + args: -DCMAKE_BUILD_TYPE=none -DCMAKE_CXX_FLAGS="-DNDEBUG" name: "🐍 3 • Clang ${{ matrix.clang }} • C++${{ matrix.std }} • x64" container: "silkeh/clang:${{ matrix.clang }}" @@ -310,6 +319,7 @@ jobs: -DPYBIND11_WERROR=ON -DDOWNLOAD_CATCH=ON -DCMAKE_CXX_STANDARD=${{ matrix.std }} + ${{ matrix.args }} -DPYTHON_EXECUTABLE=$(python3 -c "import sys; print(sys.executable)") - name: Build diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index d45465d681..356b4ff068 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -204,6 +204,7 @@ TEST_SUBMODULE(stl, m) { m.def("load_map", [](const std::map &map) { return map.at("key") == "value" && map.at("key2") == "value2"; }); + m.def("copy_map", [](std::map map) { auto m = std::move(map); }); // test_set m.def("cast_set", []() { return std::set{"key1", "key2"}; }); From 429ec5ded7403c7d4ed55129f2627ee8ec5d5b0c Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Sun, 11 Dec 2022 05:46:59 +0100 Subject: [PATCH 2/3] Add call to copy_map() ... which fails essentially in my use case, but still succeeds here. --- tests/test_stl.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_stl.py b/tests/test_stl.py index d30c382113..9e440a8894 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -67,6 +67,11 @@ def test_map(doc): assert doc(m.load_map) == "load_map(arg0: Dict[str, str]) -> bool" +def test_copy_map(): + # this segfaults in my use case, but not here: + m.copy_map({"pi": 3.14, "zero": 0.0, "one": 0.0}) + + def test_set(doc): """std::set <-> set""" s = m.cast_set() From 1f0677ac4ee7159fa6bc451be76ba163550171f9 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Sat, 10 Dec 2022 15:02:08 +0100 Subject: [PATCH 3/3] Add debug output --- include/pybind11/stl.h | 44 +++++++++++++++++++++++++++++++++++++++--- tests/CMakeLists.txt | 2 +- tests/test_stl.cpp | 5 ++++- 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 2d144b598b..82ba931df2 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -13,6 +13,7 @@ #include "detail/common.h" #include +#include #include #include #include @@ -116,6 +117,10 @@ struct map_caster { void reserve_maybe(const dict &, void *) {} public: +#define DEBUG(value) std::cerr << __func__ << ": " << &value << " #: " << value.size() << "\n" + + map_caster() { DEBUG(value); } + ~map_caster() { DEBUG(value); } bool load(handle src, bool convert) { if (!isinstance(src)) { return false; @@ -131,11 +136,13 @@ struct map_caster { } value.emplace(cast_op(std::move(kconv)), cast_op(std::move(vconv))); } + DEBUG(value); return true; } template static handle cast(T &&src, return_value_policy policy, handle parent) { + DEBUG(src); dict d; return_value_policy policy_key = policy; return_value_policy policy_value = policy; @@ -156,9 +163,40 @@ struct map_caster { return d.release(); } - PYBIND11_TYPE_CASTER(Type, - const_name("Dict[") + key_conv::name + const_name(", ") + value_conv::name - + const_name("]")); +protected: + Type value; + +public: + static constexpr auto name = _("Dict[") + key_conv::name + _(", ") + value_conv::name + _("]"); + template >::value, int> = 0> + static handle cast(T *src, return_value_policy policy, handle parent) { + if (!src) + return none().release(); + if (policy == return_value_policy::take_ownership) { + auto h = cast(std::move(*src), policy, parent); + delete src; + return h; + } else { + return cast(*src, policy, parent); + } + } + operator Type *() { + DEBUG(value); + return &value; + } + operator Type &() { + DEBUG(value); + return value; + } + operator Type &&() && { + DEBUG(value); + return std::move(value); + } + + template + using cast_op_type = pybind11::detail::movable_cast_op_type; + +#undef DEBUG }; template diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 491f215cef..cf61ba0f74 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -526,7 +526,7 @@ set(PYBIND11_TEST_PREFIX_COMMAND # A single command to compile and run the tests add_custom_target( pytest - COMMAND ${PYBIND11_TEST_PREFIX_COMMAND} ${PYTHON_EXECUTABLE} -m pytest + COMMAND ${PYBIND11_TEST_PREFIX_COMMAND} ${PYTHON_EXECUTABLE} -m pytest -v ${PYBIND11_ABS_PYTEST_FILES} DEPENDS ${test_targets} WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 356b4ff068..2dc7d4ab89 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -204,7 +204,10 @@ TEST_SUBMODULE(stl, m) { m.def("load_map", [](const std::map &map) { return map.at("key") == "value" && map.at("key2") == "value2"; }); - m.def("copy_map", [](std::map map) { auto m = std::move(map); }); + m.def("copy_map", [](std::map map) { + std::cerr << "copy_map: " << &map << " #: " << map.size() << std::endl; + auto m = std::move(map); + }); // test_set m.def("cast_set", []() { return std::set{"key1", "key2"}; });