Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit b194891

Browse files
peterstenetegpre-commit-ci[bot]henryiiirwgk
authoredJun 3, 2025··
fix: expose required symbol using clang (#5700)
* test: Added test case for visibility of common symbols across shared libraries * style: pre-commit fixes * tests: cmake target name fix * tests: Added visibility test to ci * tests: set the default visibility to hidden * prototype/proof-of-concept fix: PYBIND11_EXPORT_GUARDED_DELETE * Fix silly oversight: actually use PYBIND11_EXPORT_GUARDED_DELETE * Update struct_smart_holder.h * style: pre-commit fixes * Update include/pybind11/detail/struct_smart_holder.h * Update struct_smart_holder.h * ci: fix addition to reusable-standard.yml * Update CMakeLists.txt * refactor: rename tests to test_cross_module_rtti Signed-off-by: Henry Schreiner <[email protected]> --------- Signed-off-by: Henry Schreiner <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Henry Schreiner <[email protected]> Co-authored-by: Ralf W. Grosse-Kunstleve <[email protected]>
1 parent d4d2ec1 commit b194891

File tree

11 files changed

+268
-3
lines changed

11 files changed

+268
-3
lines changed
 

‎.github/workflows/ci.yml

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,9 @@ jobs:
230230
- name: Interface test
231231
run: cmake --build . --target test_cmake_build
232232

233+
- name: Visibility test
234+
run: cmake --build . --target test_cross_module_rtti
235+
233236

234237
manylinux:
235238
name: Manylinux on 🐍 3.13t • GIL
@@ -328,6 +331,9 @@ jobs:
328331
- name: C++ tests
329332
run: cmake --build --preset default --target cpptest
330333

334+
- name: Visibility test
335+
run: cmake --build --preset default --target test_cross_module_rtti
336+
331337
- name: Run Valgrind on Python tests
332338
if: matrix.valgrind
333339
run: cmake --build --preset default --target memcheck
@@ -386,6 +392,8 @@ jobs:
386392
- name: Interface test
387393
run: cmake --build build --target test_cmake_build
388394

395+
- name: Visibility test
396+
run: cmake --build build --target test_cross_module_rtti
389397

390398
# Testing NVCC; forces sources to behave like .cu files
391399
cuda:
@@ -505,6 +513,8 @@ jobs:
505513
- name: Interface test
506514
run: cmake --build build --target test_cmake_build
507515

516+
- name: Visibility test
517+
run: cmake --build build --target test_cross_module_rtti
508518

509519
# Testing on GCC using the GCC docker images (only recent images supported)
510520
gcc:
@@ -556,6 +566,9 @@ jobs:
556566
- name: Interface test
557567
run: cmake --build build --target test_cmake_build
558568

569+
- name: Visibility test
570+
run: cmake --build build --target test_cross_module_rtti
571+
559572
- name: Configure - Exercise cmake -DPYBIND11_TEST_OVERRIDE
560573
if: matrix.gcc == '12'
561574
shell: bash
@@ -638,6 +651,11 @@ jobs:
638651
set +e; source /opt/intel/oneapi/setvars.sh; set -e
639652
cmake --build build-11 --target test_cmake_build
640653
654+
- name: Visibility test
655+
run: |
656+
set +e; source /opt/intel/oneapi/setvars.sh; set -e
657+
cmake --build build-11 --target test_cross_module_rtti
658+
641659
- name: Configure C++17
642660
run: |
643661
set +e; source /opt/intel/oneapi/setvars.sh; set -e
@@ -670,6 +688,10 @@ jobs:
670688
set +e; source /opt/intel/oneapi/setvars.sh; set -e
671689
cmake --build build-17 --target test_cmake_build
672690
691+
- name: Visibility test
692+
run: |
693+
set +e; source /opt/intel/oneapi/setvars.sh; set -e
694+
cmake --build build-17 --target test_cross_module_rtti
673695
674696
# Testing on CentOS (manylinux uses a centos base).
675697
centos:
@@ -732,6 +754,9 @@ jobs:
732754
- name: Interface test
733755
run: cmake --build build --target test_cmake_build
734756

757+
- name: Visibility test
758+
run: cmake --build build --target test_cross_module_rtti
759+
735760

736761
# This tests an "install" with the CMake tools
737762
install-classic:
@@ -961,6 +986,9 @@ jobs:
961986
- name: Interface test C++20
962987
run: cmake --build build --target test_cmake_build
963988

989+
- name: Visibility test
990+
run: cmake --build build --target test_cross_module_rtti
991+
964992
- name: Configure C++20 - Exercise cmake -DPYBIND11_TEST_OVERRIDE
965993
run: >
966994
cmake -S . -B build_partial
@@ -1034,6 +1062,9 @@ jobs:
10341062
- name: Interface test C++11
10351063
run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build --target test_cmake_build
10361064

1065+
- name: Visibility test
1066+
run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build --target test_cross_module_rtti
1067+
10371068
- name: Clean directory
10381069
run: git clean -fdx
10391070

@@ -1055,6 +1086,9 @@ jobs:
10551086
- name: Interface test C++14
10561087
run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build2 --target test_cmake_build
10571088

1089+
- name: Visibility test
1090+
run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build2 --target test_cross_module_rtti
1091+
10581092
- name: Clean directory
10591093
run: git clean -fdx
10601094

@@ -1076,6 +1110,9 @@ jobs:
10761110
- name: Interface test C++17
10771111
run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build3 --target test_cmake_build
10781112

1113+
- name: Visibility test
1114+
run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build3 --target test_cross_module_rtti
1115+
10791116
windows_clang:
10801117
if: github.event.pull_request.draft == false
10811118

@@ -1143,6 +1180,9 @@ jobs:
11431180
- name: Interface test
11441181
run: cmake --build . --target test_cmake_build -j 2
11451182

1183+
- name: Visibility test
1184+
run: cmake --build . --target test_cross_module_rtti -j 2
1185+
11461186
- name: Clean directory
11471187
run: git clean -fdx
11481188

@@ -1210,6 +1250,9 @@ jobs:
12101250
- name: Interface test
12111251
run: cmake --build . --target test_cmake_build -j 2
12121252

1253+
- name: Visibility test
1254+
run: cmake --build . --target test_cross_module_rtti -j 2
1255+
12131256
- name: CMake Configure - Exercise cmake -DPYBIND11_TEST_OVERRIDE
12141257
run: >
12151258
cmake -S . -B build_partial

‎.github/workflows/reusable-standard.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ jobs:
7474
- name: Interface test
7575
run: cmake --build build --target test_cmake_build
7676

77+
- name: Visibility test
78+
run: cmake --build build --target test_cross_module_rtti
79+
7780
- name: Setuptools helpers test
7881
run: |
7982
uv pip install --python=python --system setuptools

‎CMakePresets.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,13 @@
6161
"name": "tests",
6262
"displayName": "Tests (for workflow)",
6363
"configurePreset": "default",
64-
"targets": ["pytest", "cpptest", "test_cmake_build"]
64+
"targets": ["pytest", "cpptest", "test_cmake_build", "test_cross_module_rtti"]
6565
},
6666
{
6767
"name": "testsvenv",
6868
"displayName": "Tests Venv (for workflow)",
6969
"configurePreset": "venv",
70-
"targets": ["pytest", "cpptest", "test_cmake_build"]
70+
"targets": ["pytest", "cpptest", "test_cmake_build", "test_cross_module_rtti"]
7171
}
7272
],
7373
"workflowPresets": [

‎include/pybind11/detail/struct_smart_holder.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,19 @@ High-level aspects:
5858
#include <typeinfo>
5959
#include <utility>
6060

61+
// IMPORTANT: This code block must stay BELOW the #include <stdexcept> above.
62+
// This is only required on some builds with libc++ (one of three implementations
63+
// in
64+
// https://github.com/llvm/llvm-project/blob/a9b64bb3180dab6d28bf800a641f9a9ad54d2c0c/libcxx/include/typeinfo#L271-L276
65+
// require it)
66+
#if !defined(PYBIND11_EXPORT_GUARDED_DELETE)
67+
# if defined(_LIBCPP_VERSION) && !defined(WIN32) && !defined(_WIN32)
68+
# define PYBIND11_EXPORT_GUARDED_DELETE __attribute__((visibility("default")))
69+
# else
70+
# define PYBIND11_EXPORT_GUARDED_DELETE
71+
# endif
72+
#endif
73+
6174
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
6275
PYBIND11_NAMESPACE_BEGIN(memory)
6376

@@ -78,7 +91,7 @@ static constexpr bool type_has_shared_from_this(const void *) {
7891
return false;
7992
}
8093

81-
struct guarded_delete {
94+
struct PYBIND11_EXPORT_GUARDED_DELETE guarded_delete {
8295
std::weak_ptr<void> released_ptr; // Trick to keep the smart_holder memory footprint small.
8396
std::function<void(void *)> del_fun; // Rare case.
8497
void (*del_ptr)(void *); // Common case.

‎tests/CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,4 +649,7 @@ if(NOT PYBIND11_CUDA_TESTS)
649649

650650
# Test CMake build using functions and targets from subdirectory or installed location
651651
add_subdirectory(test_cmake_build)
652+
653+
# Test visibility of common symbols across shared libraries
654+
add_subdirectory(test_cross_module_rtti)
652655
endif()
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
possibly_uninitialized(PYTHON_MODULE_EXTENSION Python_INTERPRETER_ID)
2+
3+
set(CMAKE_CXX_VISIBILITY_PRESET hidden)
4+
5+
if("${PYTHON_MODULE_EXTENSION}" MATCHES "pypy"
6+
OR "${Python_INTERPRETER_ID}" STREQUAL "PyPy"
7+
OR "${PYTHON_MODULE_EXTENSION}" MATCHES "graalpy")
8+
message(STATUS "Skipping visibility test on PyPy or GraalPy")
9+
add_custom_target(test_cross_module_rtti
10+
)# Dummy target on PyPy or GraalPy. Embedding is not supported.
11+
set(_suppress_unused_variable_warning "${DOWNLOAD_CATCH}")
12+
return()
13+
endif()
14+
15+
if(TARGET Python::Module AND NOT TARGET Python::Python)
16+
message(STATUS "Skipping visibility test since no embed libs found")
17+
add_custom_target(test_cross_module_rtti) # Dummy target since embedding is not supported.
18+
set(_suppress_unused_variable_warning "${DOWNLOAD_CATCH}")
19+
return()
20+
endif()
21+
22+
find_package(Catch 2.13.10)
23+
24+
if(CATCH_FOUND)
25+
message(STATUS "Building interpreter tests using Catch v${CATCH_VERSION}")
26+
else()
27+
message(STATUS "Catch not detected. Interpreter tests will be skipped. Install Catch headers"
28+
" manually or use `cmake -DDOWNLOAD_CATCH=ON` to fetch them automatically.")
29+
return()
30+
endif()
31+
32+
include(GenerateExportHeader)
33+
34+
add_library(test_cross_module_rtti_lib SHARED lib.h lib.cpp)
35+
add_library(test_cross_module_rtti_lib::test_cross_module_rtti_lib ALIAS
36+
test_cross_module_rtti_lib)
37+
target_include_directories(test_cross_module_rtti_lib PUBLIC ${CMAKE_CURRENT_BINARY_DIR})
38+
target_include_directories(test_cross_module_rtti_lib PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
39+
target_compile_features(test_cross_module_rtti_lib PUBLIC cxx_std_11)
40+
41+
generate_export_header(test_cross_module_rtti_lib)
42+
43+
pybind11_add_module(test_cross_module_rtti_bindings SHARED bindings.cpp)
44+
target_link_libraries(test_cross_module_rtti_bindings
45+
PUBLIC test_cross_module_rtti_lib::test_cross_module_rtti_lib)
46+
47+
add_executable(test_cross_module_rtti_main catch.cpp test_cross_module_rtti.cpp)
48+
target_link_libraries(
49+
test_cross_module_rtti_main PUBLIC test_cross_module_rtti_lib::test_cross_module_rtti_lib
50+
pybind11::embed Catch2::Catch2)
51+
52+
# Ensure that we have built the python bindings since we load them in main
53+
add_dependencies(test_cross_module_rtti_main test_cross_module_rtti_bindings)
54+
55+
pybind11_enable_warnings(test_cross_module_rtti_main)
56+
pybind11_enable_warnings(test_cross_module_rtti_bindings)
57+
pybind11_enable_warnings(test_cross_module_rtti_lib)
58+
59+
add_custom_target(
60+
test_cross_module_rtti
61+
COMMAND "$<TARGET_FILE:test_cross_module_rtti_main>"
62+
DEPENDS test_cross_module_rtti_main
63+
WORKING_DIRECTORY "$<TARGET_FILE_DIR:test_cross_module_rtti_main>")
64+
65+
set_target_properties(test_cross_module_rtti_bindings PROPERTIES LIBRARY_OUTPUT_DIRECTORY
66+
"${CMAKE_CURRENT_BINARY_DIR}")
67+
68+
add_dependencies(check test_cross_module_rtti)
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#include <pybind11/pybind11.h>
2+
3+
#include <lib.h>
4+
5+
class BaseTrampoline : public lib::Base, public pybind11::trampoline_self_life_support {
6+
public:
7+
using lib::Base::Base;
8+
int get() const override { PYBIND11_OVERLOAD(int, lib::Base, get); }
9+
};
10+
11+
PYBIND11_MODULE(test_cross_module_rtti_bindings, m) {
12+
pybind11::classh<lib::Base, BaseTrampoline>(m, "Base")
13+
.def(pybind11::init<int, int>())
14+
.def_readwrite("a", &lib::Base::a)
15+
.def_readwrite("b", &lib::Base::b);
16+
17+
m.def("get_foo", [](int a, int b) -> std::shared_ptr<lib::Base> {
18+
return std::make_shared<lib::Foo>(a, b);
19+
});
20+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// The Catch implementation is compiled here. This is a standalone
2+
// translation unit to avoid recompiling it for every test change.
3+
4+
#include <pybind11/embed.h>
5+
6+
// Silence MSVC C++17 deprecation warning from Catch regarding std::uncaught_exceptions (up to
7+
// catch 2.0.1; this should be fixed in the next catch release after 2.0.1).
8+
PYBIND11_WARNING_DISABLE_MSVC(4996)
9+
10+
// Catch uses _ internally, which breaks gettext style defines
11+
#ifdef _
12+
# undef _
13+
#endif
14+
15+
#define CATCH_CONFIG_RUNNER
16+
#include <catch.hpp>
17+
18+
int main(int argc, char *argv[]) {
19+
pybind11::scoped_interpreter guard{};
20+
auto result = Catch::Session().run(argc, argv);
21+
return result < 0xff ? result : 0xff;
22+
}

‎tests/test_cross_module_rtti/lib.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#include <lib.h>
2+
3+
namespace lib {
4+
5+
Base::Base(int a, int b) : a(a), b(b) {}
6+
7+
int Base::get() const { return a + b; }
8+
9+
Foo::Foo(int a, int b) : Base{a, b} {}
10+
11+
int Foo::get() const { return 2 * a + b; }
12+
13+
} // namespace lib

‎tests/test_cross_module_rtti/lib.h

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#pragma once
2+
3+
#include <memory>
4+
#include <test_cross_module_rtti_lib_export.h>
5+
6+
#if defined(_MSC_VER)
7+
__pragma(warning(disable : 4251))
8+
#endif
9+
10+
namespace lib {
11+
12+
class TEST_CROSS_MODULE_RTTI_LIB_EXPORT Base : public std::enable_shared_from_this<Base> {
13+
public:
14+
Base(int a, int b);
15+
virtual ~Base() = default;
16+
17+
virtual int get() const;
18+
19+
int a;
20+
int b;
21+
};
22+
23+
class TEST_CROSS_MODULE_RTTI_LIB_EXPORT Foo : public Base {
24+
public:
25+
Foo(int a, int b);
26+
27+
int get() const override;
28+
};
29+
30+
} // namespace lib
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
2+
#include <pybind11/embed.h>
3+
#include <pybind11/pybind11.h>
4+
5+
#include <catch.hpp>
6+
#include <lib.h>
7+
8+
static constexpr auto script = R"(
9+
import test_cross_module_rtti_bindings
10+
11+
class Bar(test_cross_module_rtti_bindings.Base):
12+
def __init__(self, a, b):
13+
test_cross_module_rtti_bindings.Base.__init__(self, a, b)
14+
15+
def get(self):
16+
return 4 * self.a + self.b
17+
18+
19+
def get_bar(a, b):
20+
return Bar(a, b)
21+
22+
)";
23+
24+
TEST_CASE("Simple case where without is_alias") {
25+
// "Simple" case this will not have `python_instance_is_alias` set in type_cast_base.h:771
26+
auto bindings = pybind11::module_::import("test_cross_module_rtti_bindings");
27+
auto holder = bindings.attr("get_foo")(1, 2);
28+
auto foo = holder.cast<std::shared_ptr<lib::Base>>();
29+
REQUIRE(foo->get() == 4); // 2 * 1 + 2 = 4
30+
}
31+
32+
TEST_CASE("Complex case where with it_alias") {
33+
// "Complex" case this will have `python_instance_is_alias` set in type_cast_base.h:771
34+
pybind11::exec(script);
35+
auto main = pybind11::module::import("__main__");
36+
37+
// The critical part of "Bar" is that it will have the `is_alias` `instance` flag set.
38+
// I'm not quite sure what is required to get that flag, this code is derived from a
39+
// larger code where this issue was observed.
40+
auto holder2 = main.attr("get_bar")(1, 2);
41+
42+
// this will trigger `std::get_deleter<memory::guarded_delete>` in type_cast_base.h:772
43+
// This will fail since the program will see two different typeids for `memory::guarded_delete`
44+
// on from the bindings module and one from "main", which will both have
45+
// `__is_type_name_unique` as true and but still have different values. Hence we will not find
46+
// the deleter and the cast fill fail. See "__eq(__type_name_t __lhs, __type_name_t __rhs)" in
47+
// typeinfo in libc++
48+
auto bar = holder2.cast<std::shared_ptr<lib::Base>>();
49+
REQUIRE(bar->get() == 6); // 4 * 1 + 2 = 6
50+
}

0 commit comments

Comments
 (0)
Please sign in to comment.