8000 bpo-43816: add 'extern "C"' to pyctype.h by aytey · Pull Request #25365 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-43816: add 'extern "C"' to pyctype.h #25365

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
Merged
merged 2 commits into from
Apr 13, 2021
Merged

bpo-43816: add 'extern "C"' to pyctype.h #25365

merged 2 commits into from
Apr 13, 2021

Conversation

aytey
Copy link
Contributor
@aytey aytey commented Apr 12, 2021 •
edited by bedevere-bot
Loading

Issue

With Python 3.9.4, and when compiling with Visual Studio 2019, we have noticed that the variable _Py_ctype_table is not scoped with in an extern "C" block, and where the Python library (python39.lib) has been compiled with a C compiler.

This causes an issue when trying to refer to _Py_ctype_table from a C++ file, as the compiler tries to name-mangle the use of _Py_ctype_table, but the linker cannot then tie the mangled name to non-mangled named from python39.lib.

Example:

#include "Python.h"
int main() { return _Py_ctype_table[0]; }

Compilation:

cl.exe /Fe:test.exe /TP /I include test.cpp /link libs/python39.lib
Microsoft (R) C/C++ Optimizing Compiler Version 19.28.29336 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

test.cpp
Microsoft (R) Incremental Linker Version 14.28.29336.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:test.exe
libs/python39.lib
test.obj
test.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) unsigned int const * const _Py_ctype_table" (__imp_?_Py_ctype_table@@3QBIB) referenced in function main
test.exe : fatal error LNK1120: 1 unresolved externals

With cl.exe:

cl.exe /Bv  
Microsoft (R) C/C++ Optimizing Compiler Version 19.28.29336 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

Compiler Passes:
 Z:\home\avj\visual_studio\MSVC\14.28.29333\bin\HostX64\x64\cl.exe:        Version 19.28.29336.0
 Z:\home\avj\visual_studio\MSVC\14.28.29333\bin\HostX64\x64\c1.dll:        Version 19.28.29336.0
 Z:\home\avj\visual_studio\MSVC\14.28.29333\bin\HostX64\x64\c1xx.dll:      Version 19.28.29336.0
 Z:\home\avj\visual_studio\MSVC\14.28.29333\bin\HostX64\x64\c2.dll:        Version 19.28.29336.0
 Z:\home\avj\visual_studio\MSVC\14.28.29333\bin\HostX64\x64\c1xx.dll:      Version 19.28.29336.0
 Z:\home\avj\visual_studio\MSVC\14.28.29333\bin\HostX64\x64\link.exe:      Version 14.28.29336.0
 Z:\home\avj\visual_studio\MSVC\14.28.29333\bin\HostX64\x64\mspdb140.dll:  Version 14.28.29336.0
 Z:\home\avj\visual_studio\MSVC\14.28.29333\bin\HostX64\x64\1033\clui.dll: Version 19.28.29336.0

A naïve check of Python.h (e126547) seems to suggest that:

  • There are 82 includes
  • 64 of these contain extern "C"
  • 8 do not contain extern "C"
  • The remaining 10 are either system includes or pyconfig.h

For the 8 that do not contain extern "C", none of these use PyAPI_DATA. This leads me to believe that it is an oversight that pyctype.h does not have extern "C"

Resolution

This PR resolves this issue by adding an extern "C" declaration to the top of pyctype.h (if compiling as C++).

Signed-off-by: Andrew V. Jones andrew.jones@vector.com

https://bugs.python.org/issue43816

@aytey aytey mentioned this pull request Apr 12, 2021
Closed
vstinner
vstinner reviewed Apr 12, 2021
View reviewed changes
aytey added 2 commits April 13, 2021 08:24
@aytey
bpo-43816: add 'extern "C"' to pyctype.h
468e639 
Signed-off-by: Andrew V. Jones <andrew.jones@vector.com>
@aytey
bpo-43816: ensuring whitespace consistency around #ifdef __cplusplus
6976d57 
Signed-off-by: Andrew V. Jones <andrew.jones@vector.com>
@aytey
Copy link
Contributor Author
aytey commented Apr 13, 2021

Any idea why test_ssl failed on macOS after the rebase?

@vstinner vstinner merged commit 54db51c into python:master Apr 13, 2021
@miss-islington
Copy link
Contributor

Thanks @andrewvaughanj for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @andrewvaughanj for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @andrewvaughanj and @vstinner, I had trouble checking out the 3.9 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 54db51c9114ac49030832f5134979ca866ffd21c 3.9

@bedevere-bot
Copy link

GH-25386 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 13, 2021
@aytey @miss-islington
bpo-43816: Add extern "C" to Include/cpython/pyctype.h (pythonGH-25365)
330bd11 
Signed-off-by: Andrew V. Jones <andrew.jones@vector.com>
(cherry picked from commit 54db51c)

Co-authored-by: Andrew V. Jones <andrewvaughanj@gmail.com>
@vstinner vstinner added needs backport to 3.9 only security fixes and removed needs backport to 3.9 only security fixes labels Apr 13, 2021
@miss-islington
Copy link
Contributor

Thanks @andrewvaughanj for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-25387 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Apr 13, 2021
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 13, 2021
@aytey @miss-islington
bpo-43816: Add extern "C" to Include/cpython/pyctype.h (pythonGH-25365)
f940222 
Signed-off-by: Andrew V. Jones <andrew.jones@vector.com>
(cherry picked from commit 54db51c)

Co-authored-by: Andrew V. Jones <andrewvaughanj@gmail.com>
miss-islington added a commit that referenced this pull request Apr 13, 2021
@miss-islington @aytey
bpo-43816: Add extern "C" to Include/cpython/pyctype.h (GH-25365)
47a894d 
Signed-off-by: Andrew V. Jones <andrew.jones@vector.com>
(cherry picked from commit 54db51c)

Co-authored-by: Andrew V. Jones <andrewvaughanj@gmail.com>
vstinner pushed a commit that referenced this pull request Apr 13, 2021
@miss-islington @aytey
bpo-43816: Add extern "C" to Include/cpython/pyctype.h (GH-25365) (GH…
15ad30d 
…-25387)

Signed-off-by: Andrew V. Jones <andrew.jones@vector.com>
(cherry picked from commit 54db51c)

Co-authored-by: Andrew V. Jones <andrewvaughanj@gmail.com>

Co-authored-by: Andrew V. Jones <andrewvaughanj@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner left review comments

Assignees

@vstinner vstinner

Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aytey @miss-islington @bedevere-bot @vstinner @the-knights-who-say-ni
0