◐ Shell
reader mode source ↗
Skip to content

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

Merged
vstinner merged 2 commits into
python:masterfrom
aytey:bpo-43816
Apr 13, 2021
Merged

bpo-43816: add 'extern "C"' to pyctype.h#25365
vstinner merged 2 commits into
python:masterfrom
aytey:bpo-43816

Conversation

@aytey

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

Copy link
Copy Markdown
Contributor

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
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

aytey commented Apr 13, 2021

Copy link
Copy Markdown
Contributor Author

Any idea why test_ssl failed on macOS after the rebase?

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

miss-islington commented Apr 13, 2021

Copy link
Copy Markdown
Contributor

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

@miss-islington

miss-islington commented Apr 13, 2021

Copy link
Copy Markdown
Contributor

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

@miss-islington

miss-islington commented Apr 13, 2021

Copy link
Copy Markdown
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

bedevere-bot commented Apr 13, 2021

Copy link
Copy Markdown

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>
@miss-islington

miss-islington commented Apr 13, 2021

Copy link
Copy Markdown
Contributor

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

@bedevere-bot

bedevere-bot commented Apr 13, 2021

Copy link
Copy Markdown

GH-25387 is a backport of this pull request to the 3.9 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) …
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>
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