Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix MSVC MT/MD incompatibility in PYBIND11_BUILD_ABI #4953

Merged
merged 22 commits into from
Nov 10, 2024
Merged

Fix MSVC MT/MD incompatibility in PYBIND11_BUILD_ABI #4953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
607812a
Fix MSVC MT/MD incompatibility in PYBIND11_BUILD_ABI
isuruf Nov 28, 2023
71b0e9e
Update comment about which PR
isuruf Nov 28, 2023
d1695f1
Use msvc major version
isuruf Nov 30, 2023
4c6c344
Use _MSC_VER/100
isuruf Oct 1, 2024
543b08f
Fix figuring out MD vs MT
isuruf Oct 14, 2024
e7a559d
Add some test runs
isuruf Oct 14, 2024
12a6478
Skip one test
isuruf Oct 14, 2024
10dd08a
Fix preprocessor
isuruf Oct 14, 2024
72b6d2a
simplify code
isuruf Oct 14, 2024
9ce2def
fix if
isuruf Oct 14, 2024
1d2f952
support only msvc 19
isuruf Oct 15, 2024
0131c55
Merge branch 'master' into incompat
rwgk Oct 23, 2024
c3415a9
Fold in changes from experimental PR #5411. Polish error messages.
rwgk Oct 23, 2024
a5b5c3b
Remove `&& defined(_DLL)` (TBD: is it needed? but what is correct?)
rwgk Oct 23, 2024
a3944f3
Fix MT vs MD
isuruf Oct 24, 2024
b9b54a7
Add a couple comments, based on https://github.com/pybind/pybind11/pu…
rwgk Oct 24, 2024
713b427
Replace misleading comment: NVHPC is NOT outdated.
rwgk Oct 28, 2024
79a3770
Merge branch 'master' into incompat
rwgk Oct 28, 2024
ef6c3f0
Update include/pybind11/detail/internals.h
rwgk Oct 28, 2024
fa75ca0
Merge branch 'master' into incompat
rwgk Nov 5, 2024
823a77b
Merge branch 'master' into incompat
rwgk Nov 6, 2024
4819606
Merge branch 'master' into incompat
rwgk Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,24 @@ jobs:
# Inject a couple Windows 2019 runs
- runs-on: windows-2019
python: '3.9'
# Inject a few runs with different runtime libraries
- runs-on: windows-2022
python: '3.9'
args: >
-DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded
- runs-on: windows-2022
python: '3.10'
args: >
-DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreadedDLL
# This needs a python built with MTd
# - runs-on: windows-2022
# python: '3.11'
# args: >
# -DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreadedDebug
- runs-on: windows-2022
python: '3.12'
args: >
-DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreadedDebugDLL
# Extra ubuntu latest job
- runs-on: ubuntu-latest
python: '3.11'
Expand Down
26 changes: 20 additions & 6 deletions include/pybind11/detail/internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,15 +310,29 @@ struct type_info {
# endif
#endif

/// On Linux/OSX, changes in __GXX_ABI_VERSION__ indicate ABI incompatibility.
/// On MSVC, changes in _MSC_VER may indicate ABI incompatibility (#2898).
#ifndef PYBIND11_BUILD_ABI
# if defined(__GXX_ABI_VERSION)
# if defined(__GXX_ABI_VERSION) // Linux/OSX.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this PR is for msvc but this isn't the correct way to check for linux.

Background:
This macro value is the same across all version of clang ( always 1002 ) but changes anytime the gcc internal compiler ABI changes, which generally has no impact. You can find more details on the gcc abi changes here: https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html

Generally you can safely pass types across anything 1002 or newer as long as the _GLIBCXX_USE_CXX11_ABI ( https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html ) is the same.

So if you build two libraries with differing values of _GLIBCXX_USE_CXX11_ABI they can't have the same abi:

~  $ clang++-16 -D_GLIBCXX_USE_CXX11_ABI=0 ./test.cpp -shared && nm ./a.out | grep foo | c++filt 
00000000000011a0 T foo(std::basic_string<char, std::char_traits<char>, std::allocator<char> >)
~  $ clang++-16 -D_GLIBCXX_USE_CXX11_ABI=1 ./test.cpp -shared && nm ./a.out | grep foo | c++filt
00000000000011a0 T foo(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)

But that isn't captured in your ABI checks at all and is significantly more important compared to __GXX_ABI_VERSION

# define PYBIND11_BUILD_ABI "_cxxabi" PYBIND11_TOSTRING(__GXX_ABI_VERSION)
# elif defined(_MSC_VER)
# define PYBIND11_BUILD_ABI "_mscver" PYBIND11_TOSTRING(_MSC_VER)
# elif defined(_MSC_VER) // See PR #4953.
# if defined(_MT) && defined(_DLL) // Corresponding to CL command line options /MD or /MDd.
# if (_MSC_VER) / 100 == 19
# define PYBIND11_BUILD_ABI "_md_mscver19"
# else
# error "Unknown major version for MSC_VER: PLEASE REVISE THIS CODE."
# endif
# elif defined(_MT) // Corresponding to CL command line options /MT or /MTd.
# define PYBIND11_BUILD_ABI "_mt_mscver" PYBIND11_TOSTRING(_MSC_VER)
# else
# if (_MSC_VER) / 100 == 19
# define PYBIND11_BUILD_ABI "_none_mscver19"
# else
# error "Unknown major version for MSC_VER: PLEASE REVISE THIS CODE."
# endif
# endif
# elif defined(__NVCOMPILER) // NVHPC (PGI-based).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For nvhpc you would check _GLIBCXX_USE_CXX11_ABI like you would for gcc or clang. You can combo that with __GNUC__ and __GNUC_MINOR__ to get the gcc compatible version that nvhpc is targetting. IIRC the behavior of nvhpc is always -fabi-version=0 where the latest is defined by the gcc it is targetting ( e.g. GNUC , GNUC_MINOR defines ).

# define PYBIND11_BUILD_ABI "" // TODO: What should be here, to prevent UB?
# else
# define PYBIND11_BUILD_ABI ""
# error "Unknown platform or compiler: PLEASE REVISE THIS CODE."
# endif
#endif

Expand Down