Skip to content
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.

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

gh-105387: Limited C API implements Py_INCREF() as func #105388

Merged
merged 2 commits into from Jun 14, 2023

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jun 6, 2023

In the limited C API version 3.12 and newer, Py_INCREF() and Py_DECREF() functions are now implemented as opaque function calls in the stable ABI to hide implementation details.

@vstinner
Copy link
Member Author

vstinner commented Jun 6, 2023

See also PR #105391: gh-102304: Debug mode no longer supports Limited API 3.9.

In the limited C API version 3.12, Py_INCREF() and Py_DECREF()
functions are now implemented as opaque function calls to hide
implementation details.
@vstinner vstinner marked this pull request as ready for review June 9, 2023 09:45
@vstinner
Copy link
Member Author

vstinner commented Jun 9, 2023

The PR is now ready for a review.

I rebased the PR on the main branch. I fixed the compatibility with Python 3.9 and older in main (commit 7ba0fd9).

@vstinner
Copy link
Member Author

vstinner commented Jun 9, 2023

In Python 3.12, Py_INCREF() and Py_DECREF() implementation become more complicated than PyObject.ob_refcnt++ and PyObject.ob_refcnt-- with the implementation of immortal objects (PEP 683).

There are C compiler issues with the current implementation: issue #105059.

@vstinner
Copy link
Member Author

vstinner commented Jun 9, 2023

This change was written to be backported to Python 3.12: see the issue for the discussion on the version number.

@gpshead gpshead added the needs backport to 3.12 bug and security fixes label Jun 9, 2023
@zooba
Copy link
Member

zooba commented Jun 13, 2023

What happens if I build with 3.12 but set Py_LIMITED_API to 3.11? or to just 1? It looks like I will get the 3.12 non-stable ABI version of Py_INCREF, rather than the 3.11 version of it.

This means that, in practice, it's only safe to set Py_LIMITED_API to at least the version that you're compiling against. I don't recall that ever being how it worked (it's always been the advice, because we know that we mess up, but it's not been a requirement).

If you can't set Py_LIMITED_API to an earlier version than what you're building with, we should make it a compile-time error. However, in this case, I think it's better to have another #else case to use the non-saturating Py_INCREF implementation for users who request a broader stable ABI than the 3.12-specific one.

@vstinner
Copy link
Member Author

What happens if I build with 3.12 but set Py_LIMITED_API to 3.11? or to just 1? It looks like I will get the 3.12 non-stable ABI version of Py_INCREF, rather than the 3.11 version of it.

Limited C API: Python 3.11 (inline)

You can look at xxlimited (Modules/xxlimited.c) which builds a C extension for the limited C API of Python 3.11 (#define Py_LIMITED_API 0x030b0000).

Preprocessor output (gcc -E):

static inline __attribute__((always_inline)) void Py_INCREF(PyObject *op)
{
    uint32_t cur_refcnt = op->ob_refcnt_split[0];
    uint32_t new_refcnt = cur_refcnt + 1;
    if (new_refcnt == 0) {
        return;
    }
    op->ob_refcnt_split[0] = new_refcnt;
    ((void)0);
}

Extract of Xxo_getattro() x86-64 assembly code:

# C code
        PyObject *v = PyDict_GetItemWithError(self->x_attr, name);
        if (v != NULL) {
            return Py_NewRef(v);

# Assembly code
   # v = PyDict_GetItemWithError()
   0x00007ffff7c7f533 <+19>:	call   0x7ffff7c7f1b0 <PyDict_GetItemWithError@plt>
   # rbp = v
   0x00007ffff7c7f538 <+24>:	mov    rbp,rax

   # if (v == NULL) ...
   0x00007ffff7c7f53b <+27>:	test   rax,rax
   0x00007ffff7c7f53e <+30>:	je     0x7ffff7c7f558 <Xxo_getattro+56>

    # Py_NewRef(v) => Py_INCREF(v)
   0x00007ffff7c7f540 <+32>:	mov    eax,DWORD PTR [rax]
   0x00007ffff7c7f542 <+34>:	add    eax,0x1
   0x00007ffff7c7f545 <+37>:	je     0x7ffff7c7f54a <Xxo_getattro+42>  # immutable refcnt?
   0x00007ffff7c7f547 <+39>:	mov    DWORD PTR [rbp+0x0],eax

   # return v => return rbp
   0x00007ffff7c7f54a <+42>:	mov    rax,rbp
   0x00007ffff7c7f54d <+45>:	pop    rbx
   0x00007ffff7c7f54e <+46>:	pop    rbp
   0x00007ffff7c7f54f <+47>:	pop    r12
   0x00007ffff7c7f551 <+49>:	ret

=> with the limited C API version 3.11, Py_INCREF() is inlined code (refcnt+1 and test for immutable refcnt) at the ABI level.

Limited C API: Python 3.12 (function call)

I modified xxlimited (Modules/xxlimited.c) to build the C extension for the limited C API of Python 3.12 (#define Py_LIMITED_API 0x030c0000).

Preprocessor output (gcc -E):

static inline __attribute__((always_inline)) void Py_INCREF(PyObject *op)
{
    _Py_IncRef(op);
}

Extract of Xxo_getattro() x86-64 assembly code:

# C code
        PyObject *v = PyDict_GetItemWithError(self->x_attr, name);
        if (v != NULL) {
            return Py_NewRef(v);

# Assembly code
   # v = PyDict_GetItemWithError()
   0x00007ffff7c7f4e3 <+19>:	call   0x7ffff7c7f1c0 <PyDict_GetItemWithError@plt>
   # r12 = v
   0x00007ffff7c7f4e8 <+24>:	mov    r12,rax

   # if (v == NULL) ...
   0x00007ffff7c7f4eb <+27>:	test   rax,rax
   0x00007ffff7c7f4ee <+30>:	je     0x7ffff7c7f500 <Xxo_getattro+48>

   # Py_INCREF(v) => _Py_IncRef(v)
   0x00007ffff7c7f4f0 <+32>:	mov    rdi,rax
   0x00007ffff7c7f4f3 <+35>:	call   0x7ffff7c7f030 <_Py_IncRef@plt>

   # return v => return r12
   0x00007ffff7c7f4f8 <+40>:	mov    rax,r12
   0x00007ffff7c7f4fb <+43>:	pop    rbx
   0x00007ffff7c7f4fc <+44>:	pop    rbp
   0x00007ffff7c7f4fd <+45>:	pop    r12
   0x00007ffff7c7f4ff <+47>:	ret

=> with the limited C API version 3.12, Py_INCREF() becomes a function call to _Py_IncRef() at the ABI level.

Py_LIMITED_API=1 (inline)

I modified Modules/xxlimited.c to use #define Py_LIMITED_API 1.

Preprocessor output (gcc -E):

static inline __attribute__((always_inline)) void Py_INCREF(PyObject *op)
{
    uint32_t cur_refcnt = op->ob_refcnt_split[0];
    uint32_t new_refcnt = cur_refcnt + 1;
    if (new_refcnt == 0) {
        return;
    }
    op->ob_refcnt_split[0] = new_refcnt;
    ((void)0);
}

=> With #define Py_LIMITED_API 1, I get the same Py_INCREF() inlined code than limited C API version 3.11 (#define Py_LIMITED_API 0x030b0000) at the ABI level.

#define Py_LIMITED_API 1 asks for the limited C API version 3.2: maximum backward compatibility.

The trick is to convert Py_LIMITED_API to an integer with Py_LIMITED_API+0 and uses it to compare the version:

#if defined(Py_LIMITED_API) && (Py_LIMITED_API+0 >= 0x030c0000 || defined(Py_REF_DEBUG))

It's a common pattern in the limited C API: look for Py_LIMITED_API usage in Include/*.h files.

@vstinner
Copy link
Member Author

It looks like I will get the 3.12 non-stable ABI version of Py_INCREF, rather than the 3.11 version of it. This means that, in practice, it's only safe to set Py_LIMITED_API to at least the version that you're compiling against.

If you build a C extension with Python 3.12 and target the limited C API 3.11, you get Python 3.12 Py_INCREF() inlined code (refcnt+1, test for immutable refcnt) which is backward compatible: the binary built with new Python 3.12 header files can be run on old Python 3.11 and older, see PEP 683: Stable ABI.

If you consider that it's wrong, please revisit the whole PEP 683, but I don't see how this question is related to my change. Your question is about the code path unaffected by this PR.

If you can't set Py_LIMITED_API to an earlier version

You can set Py_LIMITED_API to an earlier version and the built binary works on old Python versions (it remains ABI compatible). Would you mind to elaborate why do you think that it does not work?

@vstinner
Copy link
Member Author

vstinner commented Jun 13, 2023

If I understood correctly @zooba rationale, he wants to build a C extension with (the limited C API of) the new Python 3.(N+1), run it with an old Python 3.N and have the exact same behavior as if the C extension was built with an old Python 3.N. The current implementation of the limited C API DOES NOT provide such ABI warranty. There are many corner cases like the function being discussed here: Py_INCREF().

There are around 315 functions in Python 3.12 which are either implemented as a macro or a static inline function and so is likely to access structure members or relying on another implementation details: https://pythoncapi.readthedocs.io/stats.html#functions-defined-as-macros-and-static-inline-functions

The intent of moving the limited C API towards "opaque function calls" is to build a C extension with Python 3.N and have exactly the Python 3.x behavior on Python 3.x since the executed code is no longer hardcoded in the extension binary, but instead we call call in the Python executable (libpython).

For example, if this change lands in Python 3.12 beta3, a C extension is built with Python 3.12 beta3, but Python 3.12 beta4 changes again the Py_INCREF() implementation: the C extension will execute the exact new beta4 implementation, rather than using the copy of old beta3 implementation.

Without opaque function calls, the promise of a "stable ABI" is weak. This change moves the limited C API / stable ABI one step towards a better (more stable) stable ABI.

@zooba rationale is that the implementation of the limited C API and the stable ABI is broken is should be abandoned. I disagree here. IMO not only the current half-baken implementation is useful to many people, but also it is fixable in an increment way. Moreover, my long term goal is to bend the whole (regular) Python C API towards to limited C API / stable ABI.

@vstinner vstinner merged commit b542972 into python:main Jun 14, 2023
24 checks passed
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@vstinner vstinner deleted the limited_incref_func branch June 14, 2023 00:33
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 14, 2023
…GH-105388)

In the limited C API version 3.12, Py_INCREF() and Py_DECREF()
functions are now implemented as opaque function calls to hide
implementation details.
(cherry picked from commit b542972)

Co-authored-by: Victor Stinner <vstinner@python.org>
@bedevere-bot
Copy link

GH-105763 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 bug and security fixes label Jun 14, 2023
vstinner added a commit that referenced this pull request Jun 14, 2023
…5388) (#105763)

gh-105387: Limited C API implements Py_INCREF() as func (GH-105388)

In the limited C API version 3.12, Py_INCREF() and Py_DECREF()
functions are now implemented as opaque function calls to hide
implementation details.
(cherry picked from commit b542972)

Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants