This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Missing 'extern "C"' for _Py_ctype_table
Type: compile error Stage: resolved
Components: C API Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: andrewvaughanj, eric.smith, matrixise, miss-islington, scoder, vstinner
Priority: normal Keywords: patch

Created on 2021-04-12 15:16 by andrewvaughanj, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 25365 merged andrewvaughanj, 2021-04-12 15:19
PR 25386 merged miss-islington, 2021-04-13 11:28
PR 25387 merged miss-islington, 2021-04-13 11:28
Messages (16)
msg390855 - (view) Author: Andrew V. Jones (andrewvaughanj) * Date: 2021-04-12 15:16
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 (e126547c07) 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"`
msg390856 - (view) Author: Stéphane Wirtel (matrixise) * (Python committer) Date: 2021-04-12 15:36
In fact, _Py_ctype_table is limited to the internal parts of the interpreter. So in this case, this one could not be used in an external tool.

You can read: https://docs.python.org/3/c-api/stable.html

I am not sure that you correctly use the API.
msg390858 - (view) Author: Andrew V. Jones (andrewvaughanj) * Date: 2021-04-12 15:46
> In fact, _Py_ctype_table is limited to the internal parts of the interpreter. So in this case, this one could not be used in an external tool.
>

Hmm, so why is this "exposed" by the "world-facing" `Python.h` file?

I should say: we found this bug via Cython; and it was Cython that was accessing/referring to `_Py_ctype_table` -- our Cythonated code pulls in C++ headers, so we need to compile these files as C++.

I am happy to re-assign this as a Cython bug, but the fact it is fixed with an `extern "C"` in Python.h, really makes it feel like it is a Python-proper issue and not a "user" issue.
msg390860 - (view) Author: Andrew V. Jones (andrewvaughanj) * Date: 2021-04-12 15:55
> I am happy to re-assign this as a Cython bug, but the fact it is fixed with an `extern "C"` in Python.h, really makes it feel like it is a Python-proper issue and not a "user" issue.
>

Just to extend on this:

    1) The Cython-generated code uses `Py_ISSPACE` (and not `_Py_ctype_table`), but the expansion of the macro `Py_ISSPACE` then adds `_Py_ctype_table` to the user's code

    2) The "user-fix" is to wrap `#include <Python.h>` in `extern "C"` -- however, given other parts of Python.h already do this, it seems extraneous to expect a C++ user to wrap Python.h in this way
msg390868 - (view) Author: Andrew V. Jones (andrewvaughanj) * Date: 2021-04-12 16:55
> 1) The Cython-generated code uses `Py_ISSPACE` (and not `_Py_ctype_table`), but the expansion of the macro `Py_ISSPACE` then adds `_Py_ctype_table` to the user's code
>

I wrote this up as a Cython bug here (just to see if the Cython team consider this "their" bug): https://github.com/cython/cython/issues/4111
msg390890 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2021-04-12 19:22
These macros are a sort-of documented part of the C-API. At least, they were mentioned in a What's New document:

https://docs.python.org/3/whatsnew/2.7.html?highlight=py_isspace#build-and-c-api-changes

They were added here, for the Py2.7 release:

https://github.com/python/cpython/commit/8374981fb4d781d5ddbca313656bd3f32b49cef4

It looks to me like the header file should use "extern C".
@Eric, do you agree?
msg390896 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2021-04-12 20:14
These files were probably added as part of str.format() or short float repr. I think the fact that they've been moved to Include/cpython means that user code shouldn't be using them. See https://bugs.python.org/issue35134 and https://vstinner.github.io/split-include-directory-python38.html. Probably best to ask Victor.
msg390900 - (view) Author: Andrew V. Jones (andrewvaughanj) * Date: 2021-04-12 20:27
> I think the fact that they've been moved to Include/cpython means that user
> code shouldn't be using them.
>

I think it is fine to say that they shouldn't be used, but then we get this from Victor's blog:

> It was decided that internal header files must not be included implicitly by
> the generic #include <Python.h>, but included explicitly. 
>

So, is it the case that we have two issues here:

    1) Cython is using stuff it shouldn't (I can do a PR against Cython)

    2) Python.h is exposing more than it should (so, if Python "core" wants something from pyctype.h, it should be explicitly including pyctype.h and not getting via Python.h)

?
msg390910 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-12 21:49
See https://github.com/python/cpython/blob/master/Include/README.rst for the organization of the C API.
msg390949 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2021-04-13 10:00
The macros were moved to "Includes/cpython/", not to "Includes/internal/". That suggests to me that they should use "extern C", so that C++ code that wants to make use of CPython internals can use them.

Basically, the way I see it, *all* header files in "Includes/" and "Includes/cpython/" should work with C++ code and thus have an "extern C". Only the header files in "Includes/internal/" should not have them.

Regardless, I've removed the macro usage from Cython so that the current state doesn't hurt our users.
msg390954 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-13 11:27
New changeset 54db51c9114ac49030832f5134979ca866ffd21c by Andrew V. Jones in branch 'master':
bpo-43816: Add extern "C" to Include/cpython/pyctype.h (GH-25365)
https://github.com/python/cpython/commit/54db51c9114ac49030832f5134979ca866ffd21c
msg390957 - (view) Author: miss-islington (miss-islington) Date: 2021-04-13 11:45
New changeset 47a894d338f436ea8f513f1ad2cc7e1b086e99cc by Miss Islington (bot) in branch '3.8':
bpo-43816: Add extern "C" to Include/cpython/pyctype.h (GH-25365)
https://github.com/python/cpython/commit/47a894d338f436ea8f513f1ad2cc7e1b086e99cc
msg390958 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-13 12:20
New changeset 15ad30d88fead718b7eeff8c54454b82753d520e by Miss Islington (bot) in branch '3.9':
bpo-43816: Add extern "C" to Include/cpython/pyctype.h (GH-25365) (GH-25387)
https://github.com/python/cpython/commit/15ad30d88fead718b7eeff8c54454b82753d520e
msg390959 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-13 12:21
It's now fixed. Thanks Andrew V. Jones for the fix.
msg390960 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-13 12:22
The workaround for this issue is to put extern "C" around #include <Python.h>, or around the header file which includes <Python.h>.
msg390963 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-13 13:31
Stéphane Wirtel: "In fact, _Py_ctype_table is limited to the internal parts of the interpreter. So in this case, this one could not be used in an external tool. You can read: https://docs.python.org/3/c-api/stable.html"

You're right that pyctype.h is excluded from the limited C API. But almost all C extensions use the "regular" C API which includes header files in Include/cpython/.

The internal C API is something different: header files in Include/internal/.

I know that it's complicated, that why a README was written :-)
https://github.com/python/cpython/blob/master/Include/README.rst
History
Date User Action Args
2022-04-11 14:59:44adminsetgithub: 87982
2021-04-13 13:31:26vstinnersetmessages: + msg390963
2021-04-13 12:22:57vstinnersetmessages: + msg390960
2021-04-13 12:21:07vstinnersetstatus: open -> closed
versions: + Python 3.8
messages: + msg390959

resolution: fixed
stage: patch review -> resolved
2021-04-13 12:20:21vstinnersetmessages: + msg390958
2021-04-13 11:45:32miss-islingtonsetmessages: + msg390957
2021-04-13 11:28:51miss-islingtonsetpull_requests: + pull_request24120
2021-04-13 11:28:20miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request24119
2021-04-13 11:27:31vstinnersetmessages: + msg390954
2021-04-13 10:00:00scodersetmessages: + msg390949
2021-04-12 21:49:29vstinnersetmessages: + msg390910
2021-04-12 20:27:49andrewvaughanjsetmessages: + msg390900
2021-04-12 20:14:14eric.smithsetnosy: + vstinner
messages: + msg390896
2021-04-12 19:22:56scodersetversions: + Python 3.10
nosy: + scoder, eric.smith

messages: + msg390890

type: compile error
2021-04-12 16:55:56andrewvaughanjsetmessages: + msg390868
2021-04-12 16:06:54xtreaksetcomponents: + C API
2021-04-12 15:55:57andrewvaughanjsetmessages: + msg390860
2021-04-12 15:46:55andrewvaughanjsetmessages: + msg390858
2021-04-12 15:36:32matrixisesetnosy: + matrixise
messages: + msg390856
2021-04-12 15:19:01andrewvaughanjsetkeywords: + patch
stage: patch review
pull_requests: + pull_request24099
2021-04-12 15:16:23andrewvaughanjcreate