WIP cross module exception ODR experiments #4283
Conversation
|
This has to be built with different versions of pybind11, right? - see #4282 where I'm setting up a way to do that. |
|
I saw the emails from your PR but haven't looked at it yet. I'm trying to emulate ODR, so that the test is simpler. This is following what worked for #4022. I don't know if and how cross-module exceptions are different or special, compared to the type_caster situation, but it's a starting point I'm familiar with. I'm not certain this will become a permanent test, or only inform something else after I learn something here. |
|
The CI succeeds on all platforms. — I was hoping for at least some failures, because very clearly, the definitions of |
|
I believe ODR is UB, and in this case, because everything's pretty much the same, UB is simply the (un)expected behavior? |
|
Quick note, I cannot explain properly at the moment, but at least on my Linux workstation the finding is conclusive: When adding this to conftest.py
I.e. classical UB. Preliminary conclusion: I still have to figure out what the situation is under macOS and Windows. This is with |
|
@henryiii @Skylion007 @lalaland @jbms does anyone know: what's the equivalent of From looking around I'm thinking it doesn't exist, Windows only has behavior similar to Based on the CI for 42c9554, it looks like I'm still not clear how exactly symbols are resolved when pybind11 extensions are Everything I'm saying here is preliminary and could use multiple sets of extra eyes. I also started another super simple experiment under #4284. |
|
I don't believe Windows has anything like rtld_global. I think #2999 is relevant when you have one c++ shared library directly linking to another c++ shared library and a pybind11 exception type crosses the abi boundary between them. I don't think Python is really directly involved at all here. |
|
Actually, I realized I hadn't looked sufficiently carefully at #3016. The intent of that test case indeed seems to be to allow pybind11 exceptions to be thrown by one pybind11 extension and caught/translated by another, which effectively means pybind11 abi compatibility has to also extend to the pybind11 exception types, which may not be desirable and I don't think was fully intended. In fact there need not be any linking between the two c++ libraries, if the exception type is fully defined in a header. Assigning non-hidden visibility to the exception type actually has two effects:
1 is not relevant in this case because no linking happens anyway (due to rtld_local and lack of explicit delendency from one module to the other. 2 is not relevant when using libstdc++, because it appears that types are always globally identifiable when using libstdc++ regardless of visibility. It seems like at this point some users may be relying on the exception types as part of the abi, as in the example, and therefore the abi version must be bumped whenever their abi changes. If it is desired that the exception types not be a part of the abi, then there are some potential migration paths. |
|
To close the loop here, #4015, which triggered this PR, was closed. "Not an ABI issue" again. I left a comment there: #4105 (comment) @jbms I think I want to use #4284 to trim back what was done with #2999. I'll tag you there when I get to it. |
Background: pybind#2999, pybind#4105, pybind#4283, pybind#4284 In a nutshell: * Only macOS actually needs `PYBIND11_EXPORT_EXCEPTION` (pybind#4284). * Evidently (pybind#4283), under macOS `PYBIND11_EXPORT_EXCEPTION` does not run the risk of introducing ODR violations, * but evidently (pybind#4283) under Linux it does, in the presumably rare/unusual situation that `RTLD_GLOBAL` is used. * Windows does no have the equivalent of `RTLD_GLOBAL`, therefore `PYBIND11_EXPORT_EXCEPTION` has no practical benefit, on the contrary, noisy warning suppression pragmas are needed, therefore it is best left empty.
…#4298) Background: #2999, #4105, #4283, #4284 In a nutshell: * Only macOS actually needs `PYBIND11_EXPORT_EXCEPTION` (#4284). * Evidently (#4283), under macOS `PYBIND11_EXPORT_EXCEPTION` does not run the risk of introducing ODR violations, * but evidently (#4283) under Linux it does, in the presumably rare/unusual situation that `RTLD_GLOBAL` is used. * Windows does no have the equivalent of `RTLD_GLOBAL`, therefore `PYBIND11_EXPORT_EXCEPTION` has no practical benefit, on the contrary, noisy warning suppression pragmas are needed, therefore it is best left empty.
|
Archiving the CI summary here before closing the PR, JIC, for future reference. 2 skipped, 54 successful, and 21 failing checks @github-actions continuous-integration/appveyor/pr — AppVeyor build succeeded docs/readthedocs.org:pybind11 — Read the Docs build succeeded! |
…#4298) Background: #2999, #4105, #4283, #4284 In a nutshell: * Only macOS actually needs `PYBIND11_EXPORT_EXCEPTION` (#4284). * Evidently (#4283), under macOS `PYBIND11_EXPORT_EXCEPTION` does not run the risk of introducing ODR violations, * but evidently (#4283) under Linux it does, in the presumably rare/unusual situation that `RTLD_GLOBAL` is used. * Windows does no have the equivalent of `RTLD_GLOBAL`, therefore `PYBIND11_EXPORT_EXCEPTION` has no practical benefit, on the contrary, noisy warning suppression pragmas are needed, therefore it is best left empty.
Description
Guessing what could be behind #4105
Local testing passes. Let's see what happens in the CI.
Suggested changelog entry: