Repository navigation
Fix usage of PyMODINIT_FUNC #67873
Description
| BPO | 23685 |
|---|---|
| Nosy | @loewis, @Yhg1s, @pitrou, @vstinner, @tjguk, @zware, @zooba |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2015-03-20.09:59:35.967>
created_at = <Date 2015-03-17.10:13:53.146>
labels = ['build', 'OS-windows']
title = 'Fix usage of PyMODINIT_FUNC'
updated_at = <Date 2015-03-20.09:59:35.965>
user = 'https://github.com/vstinner'bugs.python.org fields:
activity = <Date 2015-03-20.09:59:35.965>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2015-03-20.09:59:35.967>
closer = 'vstinner'
components = ['Build', 'Windows']
creation = <Date 2015-03-17.10:13:53.146>
creator = 'vstinner'
dependencies = []
files = ['38517', '38527']
hgrepos = []
issue_num = 23685
keywords = ['patch']
message_count = 10.0
messages = ['238272', '238302', '238303', '238306', '238310', '238311', '238312', '238318', '238397', '238634']
nosy_count = 8.0
nosy_names = ['loewis', 'twouters', 'pitrou', 'vstinner', 'tim.golden', 'python-dev', 'zach.ware', 'steve.dower']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue23685'
versions = ['Python 3.5']
Activity
vstinner commentedon Mar 17, 2015
Attached patch fixes the usage of the PyMODINIT_FUNC macro.
My patch is based on Thomas Wouters's patch of the issue bpo-11410.
I don't understand why Modules/pyexpat.c redefined PyMODINIT_FUNC if not defined. In which case PyMODINIT_FUNC was not defined?
I'm not sure that PC/config.c should use PyMODINIT_FUNC instead of use "PyObject*".
@Steve.Dower: Does my change to PC/config.c look correct or not?
zooba commentedon Mar 17, 2015
I don't think we should be using PyMODINIT_FUNC for builtin modules, since that will make the init functions publicly available from python35.dll. That said, I do like being able to be consistent here... can we define PyMODINIT_FUNC differently when building pythoncore? That at least would keep the definitions in the same place if we ever change it (and since there's an open PEP regarding extension modules, it doesn't seem impossible).
The change for pyexpat looks right, since that's an external module. I have no ideas why PyMODINIT_FUNC wouldn't always be defined there.
vstinner commentedon Mar 17, 2015
"I don't think we should be using PyMODINIT_FUNC for builtin modules, since that will make the init functions publicly available from python35.dll."
Do you mean that my change on PC/config.c is wrong?
For example, Modules/arraymodule.c already contains:
"PyMODINIT_FUNC PyInit_array(void)".
Is the PyInit_array symbol exported today or not on Windows?
If you think that the PyInit_array symbol should be private, maybe there is already an issue in Modules/arraymodule.c?
"can we define PyMODINIT_FUNC differently when building pythoncore?"
We can add a different macro for builtin modules. Using the issue bpo-11410, we may use it to hide the symbols: __attribute__((visibility("hidden"))).
zooba commentedon Mar 17, 2015
Just had a look in Include/pyport.h and we're already defining PyMODINIT_FUNC differently for building core, so all your changes should be fine. The redefinition you removed from pyexpat.c was clearly never meant to be used for builtin modules.
vstinner commentedon Mar 17, 2015
builtin_modules.patch: add _PyBUILTIN_MODINIT_FUNC macro and use it on modules which are builtins on POSIX.
I checked: it's not necessary to modify Modules/config.c to make the symbol hidden: only the C code in Modules/ need to set the attribute on the function.
So Modules/config.c, Modules/config.c.in, Modules/makesetup, PC/config.c, Tools/freeze/makeconfig.py, ... are unchanged by the patch.
--
On Linux with GCC >= 4.0, you can test manually to hide PyInit_xxx symbols using:
python-dev commentedon Mar 17, 2015
New changeset 22a0c925a7c2 by Victor Stinner in branch 'default':
Issue bpo-23685: Fix usage of PyMODINIT_FUNC in _json, _scproxy, nis, pyexpat
https://hg.python.org/cpython/rev/22a0c925a7c2
vstinner commentedon Mar 17, 2015
I commited PyMODINIT_FUNC.patch without the useless change on PC/config.c.
zooba commentedon Mar 17, 2015
Sounds good. Wasn't quite sure if you were after any effect or just consistency :)
vstinner commentedon Mar 18, 2015
builtin_modules.patch looks complex. It can be simplified by using the Py_BUILD_CORE define.
vstinner commentedon Mar 20, 2015
In this case, it can be done in the issue bpo-11410. I consider that the initial issue is fixed, so I close the issue.