classification
Title: ABI breakage between Python 3.7.4 and 3.7.5: change in PyGC_Head structure
Type: Stage: resolved
Components: C API Versions: Python 3.7
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: inada.naoki, jd, ned.deily, vstinner
Priority: normal Keywords:

Created on 2020-02-10 12:54 by jd, last changed 2020-04-09 15:20 by vstinner. This issue is now closed.

Messages (17)
msg361689 - (view) Author: Julien Danjou (jd) * Date: 2020-02-10 12:54
As I've reported originally on the python-dev list, there seems to be an ABI breakage between 3.7.4 and 3.7.5.

https://mail.python.org/archives/list/python-dev@python.org/message/J2FGZPS5PS7473TONJTPAVSNXRGV3TFL/

The culprit commit is https://github.com/python/cpython/commit/8766cb74e186d3820db0a855ccd780d6d84461f7

This happens on a custom C module (built via Cython) when using including <internal/pystate.h> with -DPy_BUILD_CORE. I'm not sure it'd happen otherwise.

I've tried to provide a minimal use case, but since it seems to be a memory overflow, the backtrace does not make any sense and it's hard to reproduce without the orignal code.
msg361695 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-02-10 13:34
The latest 3.7 release is Python 3.7.6: Dec. 18, 2019. So at least two 3.7 release are impacted. Moreover, Python 3.8.0 and 3.8.1 have also been released with the change.
msg361696 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-02-10 13:34
You can find the rationale for this change in two issues:

* bpo-27987
* bpo-36618

First, it was a warning in clang ubsan (Undefined Behavior Sanitizer).
Then ctypes started to crash when Python was compiled with clang. It
means that compiling Python 3.7 with clang also had the issue.

The quick fix was to compile Python with -fmax-type-align=8 when clang
was detected.

But it was a signal that something was wrong in Python on x86-64:
Python didn't respect the x86-64 ABI.
msg361811 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-02-11 13:04
The PyObject structure is part of the stable ABI, but PyGC_Head is not.
https://www.python.org/dev/peps/pep-0384/#structures

I can understand that it's annoying to not be able to use the same wheel package on Python 3.7 <= 3.7.4 and Python Python 3.7 >= 3.7.5, but I don't think that we can fix this issue.

I suggest to close it as "rejected".

One solution is to require to build your C extension on installation (don't ship wheel packages).

If you want to ship wheel package, another compromise would be to require Python 3.7.5 or newer, and check early the Python version.
msg361812 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-02-11 13:05
> This happens on a custom C module (built via Cython) when using including <internal/pystate.h> with -DPy_BUILD_CORE. I'm not sure it'd happen otherwise.

By the way, I don't think that wheel packages are designed to C extensions accessing Python internals.

The internal C API is excluded from the stable ABI.
msg361813 - (view) Author: Julien Danjou (jd) * Date: 2020-02-11 13:10
> If you want to ship wheel package, another compromise would be to require Python 3.7.5 or newer, and check early the Python version.

Yes, this is what I do now to avoid getting a segmentation fault.

I'm fine with closing this as wontfix or reject, your call. As you said, if it's not part of the stable ABI it makes sense.

The sad part if that my code does not access PyGC_Head at all. It's just a side effect when loading the code that makes it crash. :(
msg361814 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-02-11 13:15
> The sad part if that my code does not access PyGC_Head at all. It's just a side effect when loading the code that makes it crash. :(

Python stores pointers to PyObject*. If your code doesn't access PyGC_Head directly, maybe it does acess it indirectly? I don't understand why you get a crash. It would help if you can provide more information about the crash. In which function does it happen? Try to get a gdb trace.
msg361866 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2020-02-12 07:38
[Julien Danjou]

> As you said, if it's not part of the stable ABI it makes sense.

I think this is a misunderstanding of what the stable ABI is for. I'm not sure whether this is spelled out explicitly anywhere, but my understanding was that ABI breakage shouldn't happen *at all* in bugfix releases, whether stable ABI or not.

The stable ABI is about feature releases within the Python 3.x series, not bugfix releases: that is, if your extension module limits itself to using the stable ABI, then a version of that module compiled for Python 3.7.x should still work for Python 3.8.x.

But lots of extension modules don't or can't restrict themselves to the stable ABI. For those modules, it should still be reasonable to expect that bugfix releases don't break the ABI: you should always be able to upgrade from Python 3.7.5 to Python 3.7.6 (for example) without having to recompile your extension modules.

Or am I misunderstanding?
msg361867 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2020-02-12 07:46
https://github.com/python/cpython/blob/95905ce0f41fd42eb1ef60ddb83f057401c3d52f/Include/cpython/objimpl.h#L89

_Py_AS_GC and all APIs to get PyGC_Head from PyObject* are not only unstable, but also private.

I thought PyGC_Head must not be used directly from extensions, so we changed its layout.

Since Julien said "my code does not access PyGC_Head at all", I still think it was not real ABI breakage.

I suppose other serious bug (e.g. dungling pointer, missing refcount increment, etc) caused the segfault.
msg361868 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2020-02-12 08:04
> _Py_AS_GC and all APIs to get PyGC_Head from PyObject* are not only unstable, but also private.

Understood; thanks. I agree that there shouldn't be a guarantee for private undocumented calls.
msg363029 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-02-29 22:49
> Understood; thanks. I agree that there shouldn't be a guarantee for private undocumented calls.

I concur. That's a good summary. We don't provide ABI stability for Julien's use case.

Julien: You cannot use the same wheel package for all Python 3.7.x versions: you need to either always compile at the installation, or ship a different wheel package per micro Python 3.7.x version.
msg363031 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2020-02-29 23:19
Mark:
> Or am I misunderstanding?

No, you are not misunderstanding :)

> ABI breakage shouldn't happen *at all* in bugfix releases, whether stable ABI or not.

Absolutely.

Victor:
> Julien: You cannot use the same wheel package for all Python 3.7.x versions: you need to either always compile at the installation, or ship a different wheel package per micro Python 3.7.x version.

Just to be absolutely clear here, in general you *are* supposed to be able to use the same wheel package for all 3.7.x releases (or one built for any other 3.x.x release series) as long as the extension modules in the wheel follow certain rules and the rules include don't use any Include/internal headers. If the extension module violates that restriction (as is apparently the case here), ABI compatibility across bugfix releases cannot be assured.
msg363033 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-02-29 23:27
> This happens on a custom C module (built via Cython) when using including <internal/pystate.h> with -DPy_BUILD_CORE. I'm not sure it'd happen otherwise.

If you opt-in for the internal C API, you should be prepared to have such issues.
msg363252 - (view) Author: Julien Danjou (jd) * Date: 2020-03-03 10:25
The problem is that AFAIK the wheel system does not allow you to specify and provide a wheel for each minor Python version.

For references, the code that segfaults is:

https://github.com/DataDog/dd-trace-py/blob/master/ddtrace/profile/collector/stack.pyx
msg363253 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-03 10:30
> The problem is that AFAIK the wheel system does not allow you to specify and provide a wheel for each minor Python version.

The alternative is to identify which part of your code depends on PyGC_Head size, and handle the different sizes there.
msg363255 - (view) Author: Julien Danjou (jd) * Date: 2020-03-03 12:14
Right, though if you read the code I linked, the PyGC_Head size is never used. AFAICS the Cython code does not access it either (at least not directly).
The code segfaults at import time.

I'm not even sure it's possible and the problem isn't in something in CPython itself.
msg366065 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-04-09 15:20
See also bpo-40241: "[C API] Make PyGC_Head structure opaque, or even more it to the internal C API".
History
Date User Action Args
2020-04-09 15:20:16vstinnersetmessages: + msg366065
2020-03-03 12:14:03jdsetmessages: + msg363255
2020-03-03 10:30:21vstinnersetmessages: + msg363253
2020-03-03 10:25:41jdsetmessages: + msg363252
2020-02-29 23:27:49vstinnersetmessages: + msg363033
2020-02-29 23:19:29ned.deilysetnosy: + ned.deily
messages: + msg363031
2020-02-29 22:49:21vstinnersetstatus: open -> closed
resolution: not a bug
messages: + msg363029

stage: resolved
2020-02-23 13:07:10mark.dickinsonsetnosy: - mark.dickinson
2020-02-12 08:04:59mark.dickinsonsetmessages: + msg361868
2020-02-12 07:46:54inada.naokisetmessages: + msg361867
2020-02-12 07:38:34mark.dickinsonsetnosy: + mark.dickinson
messages: + msg361866
2020-02-11 13:15:06vstinnersetmessages: + msg361814
2020-02-11 13:10:10jdsetmessages: + msg361813
2020-02-11 13:05:31vstinnersetmessages: + msg361812
2020-02-11 13:04:13vstinnersetmessages: + msg361811
2020-02-10 13:35:36vstinnersettitle: ABI breakage between 3.7.4 and 3.7.5 -> ABI breakage between Python 3.7.4 and 3.7.5: change in PyGC_Head structure
2020-02-10 13:34:57vstinnersetmessages: + msg361696
2020-02-10 13:34:19vstinnersetnosy: + vstinner
messages: + msg361695
2020-02-10 13:33:05vstinnersetnosy: + inada.naoki
2020-02-10 12:54:16jdcreate