diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7d7d73a665..9e8d3b6fe5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -230,6 +230,9 @@ jobs: - name: Interface test run: cmake --build . --target test_cmake_build + - name: Visibility test + run: cmake --build . --target test_cross_module_rtti + manylinux: name: Manylinux on 🐍 3.13t • GIL @@ -328,6 +331,9 @@ jobs: - name: C++ tests run: cmake --build --preset default --target cpptest + - name: Visibility test + run: cmake --build --preset default --target test_cross_module_rtti + - name: Run Valgrind on Python tests if: matrix.valgrind run: cmake --build --preset default --target memcheck @@ -386,6 +392,8 @@ jobs: - name: Interface test run: cmake --build build --target test_cmake_build + - name: Visibility test + run: cmake --build build --target test_cross_module_rtti # Testing NVCC; forces sources to behave like .cu files cuda: @@ -505,6 +513,8 @@ jobs: - name: Interface test run: cmake --build build --target test_cmake_build + - name: Visibility test + run: cmake --build build --target test_cross_module_rtti # Testing on GCC using the GCC docker images (only recent images supported) gcc: @@ -556,6 +566,9 @@ jobs: - name: Interface test run: cmake --build build --target test_cmake_build + - name: Visibility test + run: cmake --build build --target test_cross_module_rtti + - name: Configure - Exercise cmake -DPYBIND11_TEST_OVERRIDE if: matrix.gcc == '12' shell: bash @@ -638,6 +651,11 @@ jobs: set +e; source /opt/intel/oneapi/setvars.sh; set -e cmake --build build-11 --target test_cmake_build + - name: Visibility test + run: | + set +e; source /opt/intel/oneapi/setvars.sh; set -e + cmake --build build-11 --target test_cross_module_rtti + - name: Configure C++17 run: | set +e; source /opt/intel/oneapi/setvars.sh; set -e @@ -670,6 +688,10 @@ jobs: set +e; source /opt/intel/oneapi/setvars.sh; set -e cmake --build build-17 --target test_cmake_build + - name: Visibility test + run: | + set +e; source /opt/intel/oneapi/setvars.sh; set -e + cmake --build build-17 --target test_cross_module_rtti # Testing on CentOS (manylinux uses a centos base). centos: @@ -732,6 +754,9 @@ jobs: - name: Interface test run: cmake --build build --target test_cmake_build + - name: Visibility test + run: cmake --build build --target test_cross_module_rtti + # This tests an "install" with the CMake tools install-classic: @@ -961,6 +986,9 @@ jobs: - name: Interface test C++20 run: cmake --build build --target test_cmake_build + - name: Visibility test + run: cmake --build build --target test_cross_module_rtti + - name: Configure C++20 - Exercise cmake -DPYBIND11_TEST_OVERRIDE run: > cmake -S . -B build_partial @@ -1034,6 +1062,9 @@ jobs: - name: Interface test C++11 run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build --target test_cmake_build + - name: Visibility test + run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build --target test_cross_module_rtti + - name: Clean directory run: git clean -fdx @@ -1055,6 +1086,9 @@ jobs: - name: Interface test C++14 run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build2 --target test_cmake_build + - name: Visibility test + run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build2 --target test_cross_module_rtti + - name: Clean directory run: git clean -fdx @@ -1076,6 +1110,9 @@ jobs: - name: Interface test C++17 run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build3 --target test_cmake_build + - name: Visibility test + run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build3 --target test_cross_module_rtti + windows_clang: if: github.event.pull_request.draft == false @@ -1143,6 +1180,9 @@ jobs: - name: Interface test run: cmake --build . --target test_cmake_build -j 2 + - name: Visibility test + run: cmake --build . --target test_cross_module_rtti -j 2 + - name: Clean directory run: git clean -fdx @@ -1210,6 +1250,9 @@ jobs: - name: Interface test run: cmake --build . --target test_cmake_build -j 2 + - name: Visibility test + run: cmake --build . --target test_cross_module_rtti -j 2 + - name: CMake Configure - Exercise cmake -DPYBIND11_TEST_OVERRIDE run: > cmake -S . -B build_partial diff --git a/.github/workflows/reusable-standard.yml b/.github/workflows/reusable-standard.yml index b59949316b..d3c769449a 100644 --- a/.github/workflows/reusable-standard.yml +++ b/.github/workflows/reusable-standard.yml @@ -74,6 +74,9 @@ jobs: - name: Interface test run: cmake --build build --target test_cmake_build + - name: Visibility test + run: cmake --build build --target test_cross_module_rtti + - name: Setuptools helpers test run: | uv pip install --python=python --system setuptools diff --git a/CMakePresets.json b/CMakePresets.json index c967c25a37..42bf3ade9d 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -61,13 +61,13 @@ "name": "tests", "displayName": "Tests (for workflow)", "configurePreset": "default", - "targets": ["pytest", "cpptest", "test_cmake_build"] + "targets": ["pytest", "cpptest", "test_cmake_build", "test_cross_module_rtti"] }, { "name": "testsvenv", "displayName": "Tests Venv (for workflow)", "configurePreset": "venv", - "targets": ["pytest", "cpptest", "test_cmake_build"] + "targets": ["pytest", "cpptest", "test_cmake_build", "test_cross_module_rtti"] } ], "workflowPresets": [ diff --git a/include/pybind11/detail/struct_smart_holder.h b/include/pybind11/detail/struct_smart_holder.h index 758182119c..9b2da87837 100644 --- a/include/pybind11/detail/struct_smart_holder.h +++ b/include/pybind11/detail/struct_smart_holder.h @@ -58,6 +58,19 @@ High-level aspects: #include <typeinfo> #include <utility> +// IMPORTANT: This code block must stay BELOW the #include <stdexcept> above. +// This is only required on some builds with libc++ (one of three implementations +// in +// https://github.com/llvm/llvm-project/blob/a9b64bb3180dab6d28bf800a641f9a9ad54d2c0c/libcxx/include/typeinfo#L271-L276 +// require it) +#if !defined(PYBIND11_EXPORT_GUARDED_DELETE) +# if defined(_LIBCPP_VERSION) && !defined(WIN32) && !defined(_WIN32) +# define PYBIND11_EXPORT_GUARDED_DELETE __attribute__((visibility("default"))) +# else +# define PYBIND11_EXPORT_GUARDED_DELETE +# endif +#endif + PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(memory) @@ -78,7 +91,7 @@ static constexpr bool type_has_shared_from_this(const void *) { return false; } -struct guarded_delete { +struct PYBIND11_EXPORT_GUARDED_DELETE guarded_delete { std::weak_ptr<void> released_ptr; // Trick to keep the smart_holder memory footprint small. std::function<void(void *)> del_fun; // Rare case. void (*del_ptr)(void *); // Common case. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 069b4f6f38..0e76e68786 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -649,4 +649,7 @@ if(NOT PYBIND11_CUDA_TESTS) # Test CMake build using functions and targets from subdirectory or installed location add_subdirectory(test_cmake_build) + + # Test visibility of common symbols across shared libraries + add_subdirectory(test_cross_module_rtti) endif() diff --git a/tests/test_cross_module_rtti/CMakeLists.txt b/tests/test_cross_module_rtti/CMakeLists.txt new file mode 100644 index 0000000000..97d2c780cb --- /dev/null +++ b/tests/test_cross_module_rtti/CMakeLists.txt @@ -0,0 +1,68 @@ +possibly_uninitialized(PYTHON_MODULE_EXTENSION Python_INTERPRETER_ID) + +set(CMAKE_CXX_VISIBILITY_PRESET hidden) + +if("${PYTHON_MODULE_EXTENSION}" MATCHES "pypy" + OR "${Python_INTERPRETER_ID}" STREQUAL "PyPy" + OR "${PYTHON_MODULE_EXTENSION}" MATCHES "graalpy") + message(STATUS "Skipping visibility test on PyPy or GraalPy") + add_custom_target(test_cross_module_rtti + )# Dummy target on PyPy or GraalPy. Embedding is not supported. + set(_suppress_unused_variable_warning "${DOWNLOAD_CATCH}") + return() +endif() + +if(TARGET Python::Module AND NOT TARGET Python::Python) + message(STATUS "Skipping visibility test since no embed libs found") + add_custom_target(test_cross_module_rtti) # Dummy target since embedding is not supported. + set(_suppress_unused_variable_warning "${DOWNLOAD_CATCH}") + return() +endif() + +find_package(Catch 2.13.10) + +if(CATCH_FOUND) + message(STATUS "Building interpreter tests using Catch v${CATCH_VERSION}") +else() + message(STATUS "Catch not detected. Interpreter tests will be skipped. Install Catch headers" + " manually or use `cmake -DDOWNLOAD_CATCH=ON` to fetch them automatically.") + return() +endif() + +include(GenerateExportHeader) + +add_library(test_cross_module_rtti_lib SHARED lib.h lib.cpp) +add_library(test_cross_module_rtti_lib::test_cross_module_rtti_lib ALIAS + test_cross_module_rtti_lib) +target_include_directories(test_cross_module_rtti_lib PUBLIC ${CMAKE_CURRENT_BINARY_DIR}) +target_include_directories(test_cross_module_rtti_lib PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) +target_compile_features(test_cross_module_rtti_lib PUBLIC cxx_std_11) + +generate_export_header(test_cross_module_rtti_lib) + +pybind11_add_module(test_cross_module_rtti_bindings SHARED bindings.cpp) +target_link_libraries(test_cross_module_rtti_bindings + PUBLIC test_cross_module_rtti_lib::test_cross_module_rtti_lib) + +add_executable(test_cross_module_rtti_main catch.cpp test_cross_module_rtti.cpp) +target_link_libraries( + test_cross_module_rtti_main PUBLIC test_cross_module_rtti_lib::test_cross_module_rtti_lib + pybind11::embed Catch2::Catch2) + +# Ensure that we have built the python bindings since we load them in main +add_dependencies(test_cross_module_rtti_main test_cross_module_rtti_bindings) + +pybind11_enable_warnings(test_cross_module_rtti_main) +pybind11_enable_warnings(test_cross_module_rtti_bindings) +pybind11_enable_warnings(test_cross_module_rtti_lib) + +add_custom_target( + test_cross_module_rtti + COMMAND "$<TARGET_FILE:test_cross_module_rtti_main>" + DEPENDS test_cross_module_rtti_main + WORKING_DIRECTORY "$<TARGET_FILE_DIR:test_cross_module_rtti_main>") + +set_target_properties(test_cross_module_rtti_bindings PROPERTIES LIBRARY_OUTPUT_DIRECTORY + "${CMAKE_CURRENT_BINARY_DIR}") + +add_dependencies(check test_cross_module_rtti) diff --git a/tests/test_cross_module_rtti/bindings.cpp b/tests/test_cross_module_rtti/bindings.cpp new file mode 100644 index 0000000000..94fa6874f8 --- /dev/null +++ b/tests/test_cross_module_rtti/bindings.cpp @@ -0,0 +1,20 @@ +#include <pybind11/pybind11.h> + +#include <lib.h> + +class BaseTrampoline : public lib::Base, public pybind11::trampoline_self_life_support { +public: + using lib::Base::Base; + int get() const override { PYBIND11_OVERLOAD(int, lib::Base, get); } +}; + +PYBIND11_MODULE(test_cross_module_rtti_bindings, m) { + pybind11::classh<lib::Base, BaseTrampoline>(m, "Base") + .def(pybind11::init<int, int>()) + .def_readwrite("a", &lib::Base::a) + .def_readwrite("b", &lib::Base::b); + + m.def("get_foo", [](int a, int b) -> std::shared_ptr<lib::Base> { + return std::make_shared<lib::Foo>(a, b); + }); +} diff --git a/tests/test_cross_module_rtti/catch.cpp b/tests/test_cross_module_rtti/catch.cpp new file mode 100644 index 0000000000..2debc5ff17 --- /dev/null +++ b/tests/test_cross_module_rtti/catch.cpp @@ -0,0 +1,22 @@ +// The Catch implementation is compiled here. This is a standalone +// translation unit to avoid recompiling it for every test change. + +#include <pybind11/embed.h> + +// Silence MSVC C++17 deprecation warning from Catch regarding std::uncaught_exceptions (up to +// catch 2.0.1; this should be fixed in the next catch release after 2.0.1). +PYBIND11_WARNING_DISABLE_MSVC(4996) + +// Catch uses _ internally, which breaks gettext style defines +#ifdef _ +# undef _ +#endif + +#define CATCH_CONFIG_RUNNER +#include <catch.hpp> + +int main(int argc, char *argv[]) { + pybind11::scoped_interpreter guard{}; + auto result = Catch::Session().run(argc, argv); + return result < 0xff ? result : 0xff; +} diff --git a/tests/test_cross_module_rtti/lib.cpp b/tests/test_cross_module_rtti/lib.cpp new file mode 100644 index 0000000000..927ed044bc --- /dev/null +++ b/tests/test_cross_module_rtti/lib.cpp @@ -0,0 +1,13 @@ +#include <lib.h> + +namespace lib { + +Base::Base(int a, int b) : a(a), b(b) {} + +int Base::get() const { return a + b; } + +Foo::Foo(int a, int b) : Base{a, b} {} + +int Foo::get() const { return 2 * a + b; } + +} // namespace lib diff --git a/tests/test_cross_module_rtti/lib.h b/tests/test_cross_module_rtti/lib.h new file mode 100644 index 0000000000..0925b084ca --- /dev/null +++ b/tests/test_cross_module_rtti/lib.h @@ -0,0 +1,30 @@ +#pragma once + +#include <memory> +#include <test_cross_module_rtti_lib_export.h> + +#if defined(_MSC_VER) +__pragma(warning(disable : 4251)) +#endif + + namespace lib { + + class TEST_CROSS_MODULE_RTTI_LIB_EXPORT Base : public std::enable_shared_from_this<Base> { + public: + Base(int a, int b); + virtual ~Base() = default; + + virtual int get() const; + + int a; + int b; + }; + + class TEST_CROSS_MODULE_RTTI_LIB_EXPORT Foo : public Base { + public: + Foo(int a, int b); + + int get() const override; + }; + +} // namespace lib diff --git a/tests/test_cross_module_rtti/test_cross_module_rtti.cpp b/tests/test_cross_module_rtti/test_cross_module_rtti.cpp new file mode 100644 index 0000000000..64988b77a1 --- /dev/null +++ b/tests/test_cross_module_rtti/test_cross_module_rtti.cpp @@ -0,0 +1,50 @@ + +#include <pybind11/embed.h> +#include <pybind11/pybind11.h> + +#include <catch.hpp> +#include <lib.h> + +static constexpr auto script = R"( +import test_cross_module_rtti_bindings + +class Bar(test_cross_module_rtti_bindings.Base): + def __init__(self, a, b): + test_cross_module_rtti_bindings.Base.__init__(self, a, b) + + def get(self): + return 4 * self.a + self.b + + +def get_bar(a, b): + return Bar(a, b) + +)"; + +TEST_CASE("Simple case where without is_alias") { + // "Simple" case this will not have `python_instance_is_alias` set in type_cast_base.h:771 + auto bindings = pybind11::module_::import("test_cross_module_rtti_bindings"); + auto holder = bindings.attr("get_foo")(1, 2); + auto foo = holder.cast<std::shared_ptr<lib::Base>>(); + REQUIRE(foo->get() == 4); // 2 * 1 + 2 = 4 +} + +TEST_CASE("Complex case where with it_alias") { + // "Complex" case this will have `python_instance_is_alias` set in type_cast_base.h:771 + pybind11::exec(script); + auto main = pybind11::module::import("__main__"); + + // The critical part of "Bar" is that it will have the `is_alias` `instance` flag set. + // I'm not quite sure what is required to get that flag, this code is derived from a + // larger code where this issue was observed. + auto holder2 = main.attr("get_bar")(1, 2); + + // this will trigger `std::get_deleter<memory::guarded_delete>` in type_cast_base.h:772 + // This will fail since the program will see two different typeids for `memory::guarded_delete` + // on from the bindings module and one from "main", which will both have + // `__is_type_name_unique` as true and but still have different values. Hence we will not find + // the deleter and the cast fill fail. See "__eq(__type_name_t __lhs, __type_name_t __rhs)" in + // typeinfo in libc++ + auto bar = holder2.cast<std::shared_ptr<lib::Base>>(); + REQUIRE(bar->get() == 6); // 4 * 1 + 2 = 6 +}