Skip to content

CALL_LIST_APPEND specialization can make no progress #141367

Closed
Closed
CALL_LIST_APPEND specialization can make no progress#141367
Assignees
efimov-mikhail
Labels
3.15bugs and security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Description

@Fidget-Spinner
Fidget-Spinner
opened on Nov 10, 2025

Bug report

Bug description:

CALL_LIST_APPEND requires self to be a PyList_CheckExact in bytecodes.c. However, the specialize.c doesn't check that. This means there's a possible discrepancy between what we specialize for and what we deopt.

The fix is to check that self is a list. It should be as simple as

diff --git a/Python/specialize.c b/Python/specialize.c
index a1c5dedd615..5b44432b8b7 100644
--- a/Python/specialize.c
+++ b/Python/specialize.c
@@ -2041,8 +2041,12 @@ specialize_method_descriptor(PyMethodDescrObject *descr, _Py_CODEUNIT *instr,
             bool pop = (next.op.code == POP_TOP);
             int oparg = instr->op.arg;
             if ((PyObject *)descr == list_append && oparg == 1 && pop) {
-                specialize(instr, CALL_LIST_APPEND);
-                return 0;
+                PyThreadState *tstate = PyThreadState_GET();
+                PyObject *self = PyStackRef_AsPyObjectBorrow(tstate->current_frame->stackpointer[-2]);
+                if (PyList_CheckExact(self)) {
+                    specialize(instr, CALL_LIST_APPEND);
+                    return 0;
+                }
             }
             specialize(instr, CALL_METHOD_DESCRIPTOR_O);
             return 0;

The test case should be in test_opcache.py, and test something like

class MyList(list): pass
x = MyList()
for _ in range(spec_counter):
    x.append(1)

and ensure that the append is not specialized to CALL_LIST_APPEND.

@efimov-mikhail can I let you take up this issue please? I'm pretty sure 3.14 is affected too.

CPython versions tested on:

CPython main branch, 3.15, 3.14

Operating systems tested on:

No response

Linked PRs

Activity

  1. Fidget-Spinner
    added
    type-bugAn unexpected behavior, bug, or error
    on Nov 10, 2025
  2. StanFromIreland
    added
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    on Nov 10, 2025
  3. efimov-mikhail
    self-assigned this
    on Nov 10, 2025

  4. efimov-mikhail commented on Nov 10, 2025

    @efimov-mikhail
    efimov-mikhail
    on Nov 10, 2025
    Member

    @Fidget-Spinner , of course, I will take this, thanks.

  5. efimov-mikhail commented on Nov 11, 2025

    @efimov-mikhail
    efimov-mikhail
    on Nov 11, 2025
    Member

    I'm pretty sure 3.14 is affected too.

    Actually, it isn't affected.

    You can see your own comment on PR with this change:
    #134240 (comment)

    So, basically, we should fix specialization on main branch only.
    I'll create a PR soon.

  6. efimov-mikhail
    added
    3.15bugs and security fixes
    on Nov 11, 2025

  7. Fidget-Spinner commented on Nov 11, 2025

    @Fidget-Spinner
    Fidget-Spinner
    on Nov 11, 2025
    MemberAuthor

    Great observation, thanks!

  8. efimov-mikhail commented on Nov 11, 2025

    @efimov-mikhail
    efimov-mikhail
    on Nov 11, 2025
    Member

    @Fidget-Spinner , I've created a PR with your proposed solution, CI is green.
    I've used _PyThreadState_GET instead of PyThreadState_GET because it's sligtly faster, IIUC.
    And make test for both list and MyList append specialization.

  9. markshannon commented on Nov 11, 2025

    @markshannon
    markshannon
    on Nov 11, 2025
    Member

    Rather than probing the stack tstate->current_frame->stackpointer[-2], we should pass the arguments to _Py_Specialize_Call.
    We also need to add an additional guard in _CALL_LIST_APPEND that callable == interp->callable_cache.list_append.

  10. efimov-mikhail commented on Nov 11, 2025

    @efimov-mikhail
    efimov-mikhail
    on Nov 11, 2025
    Member

    Rather than probing the stack tstate->current_frame->stackpointer[-2], we should pass the arguments to _Py_Specialize_Call.

    Ok, I'll investigate this.

    We also need to add an additional guard in _CALL_LIST_APPEND that callable == interp->callable_cache.list_append.

    Do we really need this since _GUARD_CALLABLE_LIST_APPEND is present?

    Maybe we should remove checking DEOPT_IF(!PyList_CheckExact(self_o)) in _GUARD_CALLABLE_LIST_APPEND because _GUARD_NOS_LIST is already used in CALL_LIST_APPEND macro?

  11. markshannon commented on Nov 11, 2025

    @markshannon
    markshannon
    on Nov 11, 2025
    Member

    Do we really need this since _GUARD_CALLABLE_LIST_APPEND is present?

    No we don't. I was just looking at the uop, not the whole instruction.

    Maybe we should remove checking DEOPT_IF(!PyList_CheckExact(self_o)) in _GUARD_CALLABLE_LIST_APPEND because _GUARD_NOS_LIST is already used in CALL_LIST_APPEND macro?

    Yes, that makes sense.

  12. efimov-mikhail commented on Nov 11, 2025

    @efimov-mikhail
    efimov-mikhail
    on Nov 11, 2025
    Member

    While working on this issue I've found this PR:
    https://github.com/python/cpython/pull/132018/files#diff-af5c4ee62cca27529efe0febc3babfd5745cc1ae48a280a58605a8c85ee38942R335

    Maybe we should change this test a little to use actual SPECIALIZATION_THRESHOLD instead of constant 3?

  13. Fidget-Spinner commented on Nov 11, 2025

    @Fidget-Spinner
    Fidget-Spinner
    on Nov 11, 2025
    MemberAuthor

    Maybe not for this PR, but if you open another PR to fix it I'd be open to merging.

  14. Fidget-Spinner
    added 2 commits that reference this issue on Nov 11, 2025

    gh-141367: Use actual SPECIALIZATION_THRESHOLD value in specializatio…

    2fb2b82

    gh-141367: Use CALL_LIST_APPEND instruction only for lists, not for l…

    1281be1
  15. Fidget-Spinner
    closed this as completedon Nov 14, 2025
  16. StanFromIreland
    added 2 commits that reference this issue on Dec 6, 2025

    pythongh-141367: Use actual SPECIALIZATION_THRESHOLD value in special…

    39a8612

    pythongh-141367: Use CALL_LIST_APPEND instruction only for lists, not…

    dd905f2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

3.15bugs and security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions