Add support for Python 3.13

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 99.37500% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.79%. Comparing base (c2d97b6) to head (b50cc68). Report is 59 commits behind head on main.

Files with missing lines	Patch %	Lines
src/bytecode/instr.py	97.61%	0 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #146 +/- ## ========================================== + Coverage 95.63% 95.79% +0.15% ========================================== Files 6 7 +1 Lines 1971 2044 +73 Branches 475 464 -11 ========================================== + Hits 1885 1958 +73 Misses 52 52 Partials 34 34 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[MatthieuDartiailh]

The framework test issue does not clearly look related.

[P403n1x87]

I've merged this branch into #143 to see what we get with the better round-tripping 🤞

[P403n1x87]

The framework test issue does not clearly look related.

I've run this locally on a Mac with 3.13.0 and couldn't reproduce the failure. OTOH the better framework tests are failing with recompilation issues https://github.com/MatthieuDartiailh/bytecode/actions/runs/11343896774/job/31547476578?pr=143

[MatthieuDartiailh]

Thanks @P403n1x87 I will try to have a look at the recompilation issue.

[MatthieuDartiailh]

I tried but cannot reproduce the recompilation issue. Can you ?

[P403n1x87]

I tried but cannot reproduce the recompilation issue. Can you ?

Nope 🤔

[MatthieuDartiailh]

So that's really weird. Can you reproduce on 3.11 ?

[MatthieuDartiailh]

I tried re-running and I got the same failure on 3.13 but I do not understand why it does not reproduce locally

[P403n1x87]

So that's really weird. Can you reproduce on 3.11 ?

Hmm no, it seems that at least collection works fine on 3.11 too (locally for 3.11.4 on MacOS M1) 🤔 very weird

Ah wait! I've been super-silly, I forgot that I needed to set the PYTHONPATH (sorry it's been long since I opened that PR 😅 ). I can repro on 3.11 and 3.13 with

PYTHONPATH=$(pwd)/tests/frameworks/ bash scripts/frameworks/boto3/run.sh /tmp/boto3 3.11

[MatthieuDartiailh]

Thanks for confirming. When I tried for 3.13 I did not use the full harness just tried disassembling the offending classmethod. I guess I will have to do the full thing but that will make debugging harder.

[P403n1x87]

I'm improving the output from the framework tests. I think I've got a lead for the 3.13 failure

Traceback (most recent call last):
  File "/Users/gabriele.tornetta/p403n1x87/bytecode/tests/frameworks/sitecustomize.py", line 50, in transform
    abstract_code = Bytecode.from_code(code)
  File "/Users/gabriele.tornetta/p403n1x87/bytecode/src/bytecode/bytecode.py", line 282, in from_code
    return concrete.to_bytecode(
           ~~~~~~~~~~~~~~~~~~~~^
        prune_caches=prune_caches,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^
        conserve_exception_block_stackdepth=conserve_exception_block_stackdepth,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/Users/gabriele.tornetta/p403n1x87/bytecode/src/bytecode/concrete.py", line 1053, in to_bytecode
    arg = locals_lookup[c_arg]
          ~~~~~~~~~~~~~^^^^^^^
IndexError: list index out of range

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/gabriele.tornetta/p403n1x87/bytecode/tests/frameworks/sitecustomize.py", line 65, in transform
    instr.arg = self.transform(instr.arg, _module, root=False)
                ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gabriele.tornetta/p403n1x87/bytecode/tests/frameworks/sitecustomize.py", line 59, in transform
    raise RuntimeError(stream.getvalue()) from e
RuntimeError: Failed to convert <code object PytestPdbWrapper at 0x1018874b0, file "/tmp/boto3-3.13/.venv/lib/python3.13/site-packages/_pytest/debugging.py", line 159> from <module '_pytest.debugging' from '/tmp/boto3-3.13/.venv/lib/python3.13/site-packages/_pytest/debugging.py'> into abstract code
==== Locals from last frame ====
                                 self = <ConcreteBytecode instr#=62>
                         prune_caches = True
  conserve_exception_block_stackdepth = False
                       c_instructions = <ConcreteBytecode instr#=62>
                         jump_targets = set()
                               offset = 16
                              c_instr = <LOAD_FAST arg=3 location=InstrLocation(lineno=163, end_lineno=167, col_offset=12, end_col_offset=26)>
                               target = None
                             ex_start = {}
                               ex_end = {}
                                jumps = []
                         instructions = [<COPY_FREE_VARS arg=3 location=InstrLocation(lineno=None, end_lineno=None, col_offset=None, end_col_offset=None)>, <MAKE_CELL arg=<CellVar '__class__'> location=InstrLocation(lineno=None, end_lineno=None, col_offset=None, end_col_offset=None)>, <RESUME arg=0 location=InstrLocation(lineno=159, end_lineno=159, col_offset=0, end_col_offset=0)>, <LOAD_NAME arg='__name__' location=InstrLocation(lineno=159, end_lineno=159, col_offset=0, end_col_offset=0)>, <STORE_NAME arg='__module__' location=InstrLocation(lineno=159, end_lineno=159, col_offset=0, end_col_offset=0)>, <LOAD_CONST arg='pytestPDB._get_pdb_wrapper_class.<locals>.PytestPdbWrapper' location=InstrLocation(lineno=159, end_lineno=159, col_offset=0, end_col_offset=0)>, <STORE_NAME arg='__qualname__' location=InstrLocation(lineno=159, end_lineno=159, col_offset=0, end_col_offset=0)>, <LOAD_CONST arg=159 location=InstrLocation(lineno=159, end_lineno=159, col_offset=0, end_col_offset=0)>, <STORE_NAME arg='__firstlineno__' location=InstrLocation(lineno=159, end_lineno=159, col_offset=0, end_col_offset=0)>, <LOAD_LOCALS location=InstrLocation(lineno=160, end_lineno=160, col_offset=29, end_col_offset=35)>, <LOAD_FROM_DICT_OR_DEREF arg=<FreeVar 'capman'> location=InstrLocation(lineno=160, end_lineno=160, col_offset=29, end_col_offset=35)>, <STORE_NAME arg='_pytest_capman' location=InstrLocation(lineno=160, end_lineno=160, col_offset=12, end_col_offset=26)>, <LOAD_CONST arg=False location=InstrLocation(lineno=161, end_lineno=161, col_offset=25, end_col_offset=30)>, <STORE_NAME arg='_continued' location=InstrLocation(lineno=161, end_lineno=161, col_offset=12, end_col_offset=22)>, <LOAD_FAST arg=<CellVar '__class__'> location=InstrLocation(lineno=163, end_lineno=167, col_offset=12, end_col_offset=26)>]
                               labels = {}
                            tb_instrs = {}
                         cells_lookup = [<CellVar '__class__'>]
                               ncells = 1
                        locals_lookup = [<CellVar '__class__'>]
                               lineno = 163
                          jump_target = None
                                 size = 2
                 current_instr_offset = 15
                               opcode = 85
                                c_arg = 3
                                  arg = __class__
                            n_or_cell = __class__
                                 name = capman
                             location = InstrLocation(lineno=163, end_lineno=167, col_offset=12, end_col_offset=26)
================================
  --           COPY_FREE_VARS           3
               MAKE_CELL                0 (__class__)

 159           RESUME                   0
               LOAD_NAME                0 (__name__)
               STORE_NAME               1 (__module__)
               LOAD_CONST               0 ('pytestPDB._get_pdb_wrapper_class.<locals>.PytestPdbWrapper')
               STORE_NAME               2 (__qualname__)
               LOAD_CONST               1 (159)
               STORE_NAME               3 (__firstlineno__)

 160           LOAD_LOCALS
               LOAD_FROM_DICT_OR_DEREF  2 (capman)
               STORE_NAME               4 (_pytest_capman)

 161           LOAD_CONST               2 (False)
               STORE_NAME               5 (_continued)

 163           LOAD_FAST                0 (__class__)
               LOAD_FAST                3 (cls)

My hunch is that locals_lookup should probably include the free vars (I haven't checked the CPython source yet). The last instruction is expecting to find the cell value at index 3, but before the cell is created we have COPY_FREE_VARS 3

[MatthieuDartiailh]

So I can finally reproduce using:

import dis
import io
from types import CodeType

from _pytest import debugging

from bytecode import Bytecode, ControlFlowGraph

def transform(code):
    # Round-trip the code object through the library
    try:
        abstract_code = Bytecode.from_code(code)

        for instr in abstract_code:
            try:
                if isinstance(instr.arg, CodeType):
                    instr.arg = transform(instr.arg)
            except AttributeError:
                pass

        cfg = ControlFlowGraph.from_bytecode(abstract_code)

        recompiled_code = cfg.to_code()

        # Check we can still disassemble the code
        dis.dis(recompiled_code, file=io.StringIO())

        return recompiled_code
    except Exception:
        print(f"Failed to recompile {code}")
        dis.dis(code)
        raise

transform(debugging.__loader__.get_code(debugging.__name__))

Your fix allow to find a name but LOAD_FAST currently reject FreeVar object... I need to understand what changed here since 3.12.

[MatthieuDartiailh]

I have a fix that work locally on the misbehaving case. Let's see how much it helps. Ideally we need a dedicated test to catch such issue faster in the future.

[P403n1x87]

Ideally we need a dedicated test to catch such issue faster in the future.

Hopefully the new framework test output will help. I was able to narrow it down thanks to the local variable dump, which pointed to the "offending" instruction.

[P403n1x87]

The offending code was

    @classmethod
    def _get_pdb_wrapper_class(cls, pdb_cls, capman: Optional["CaptureManager"]):
        import _pytest.config

        # Type ignored because mypy doesn't support "dynamic"
        # inheritance like this.
        class PytestPdbWrapper(pdb_cls):  # type: ignore[valid-type,misc]
            _pytest_capman = capman
            _continued = False

            def do_debug(self, arg):
                cls._recursive_debug += 1
                ret = super().do_debug(arg)
                cls._recursive_debug -= 1
                return ret

I think a minimal reproducer would be

class Foo:
    r = 0

    @classmethod
    def bar(cls, k):
        class Snafu(k):
            def do_debug(self, arg):
                cls.r += 1
                return super().d(arg)

        return Snafu

[P403n1x87]

btw the fix seems to work https://github.com/MatthieuDartiailh/bytecode/actions/runs/11383719496/job/31669942536?pr=143

[MatthieuDartiailh]

So we still have the weird test failure on 3.13 that I do not understand. And 3.11 passed here and failed on your other branch, this one I may ignore for some time to get a release with 3.13 support faster.

[P403n1x87]

And 3.11 passed here and failed on your other branch, this one I may ignore for some time to get a release with 3.13 support faster.

The other branch does much more work as it re-compiles many more code objects. We should try to get that PR merged if possible because it would help catch many more issues.

[MatthieuDartiailh]

Thanks for the reproducer I was able to add a minimal test.

[P403n1x87]

I'm looking into the current boto3 failure and I'm comparing code objects against their recompiled versions. I've noticed that there some occasional differences in the co_consts attribute

Attribute co_consts differs:
  Original: ('CalledProcessError', 132, 'Raised when run() is called with check=True and the process\nreturns a non-zero exit status.\n\nAttributes:\n  cmd, returncode, stdout, stderr, output\n', None, <code object __init__ at 0x105075830, file "lib/python3.13/subprocess.py", line 139>, <code object __str__ at 0x105e896f0, file "lib/python3.13/subprocess.py", line 145>, <code object stdout at 0x107040120, file "lib/python3.13/subprocess.py", line 157>, <code object stdout at 0x107086cd0, file "lib/python3.13/subprocess.py", line 162>, ('cmd', 'output', 'returncode', 'stderr'), (None, None))
  Recompiled: ('CalledProcessError', 132, 'Raised when run() is called with check=True and the process\nreturns a non-zero exit status.\n\nAttributes:\n  cmd, returncode, stdout, stderr, output\n', (None, None), <code object __init__ at 0x107046730, file "lib/python3.13/subprocess.py", line 139>, <code object __str__ at 0x10de789d0, file "lib/python3.13/subprocess.py", line 145>, <code object stdout at 0x1070d4990, file "lib/python3.13/subprocess.py", line 157>, <code object stdout at 0x107086f70, file "lib/python3.13/subprocess.py", line 162>, ('cmd', 'output', 'returncode', 'stderr'), None)

In this case it looks like two constants have been swapped in the recompilation

[MatthieuDartiailh]

This is something I am aware. When rebuilding from the abstract version we put things in order while the Python compiler sometimes produce a different order. So far it did not cause any issue.

[P403n1x87]

Just spotted this around the line that causes the problem

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../boto3/session.py:402: in resource
    raise ResourceNotExistsError(

So in this case it looks like the wrong values are being loaded

[MatthieuDartiailh]

Looking at the full diff before and after decompilation/recompilation It is interesting to see a single instruction is affected while other instructions loading the same object are fine. I will need a deeper dive but I won't have time today.

Anyway thanks for your amazing investigations that gave me very clear reproducer to play with.

[P403n1x87]

Happy to help 🙂 . Looking at this bit of code https://github.com/P403n1x87/bytecode/blob/4892a96180c6c5a12c65f06ef489413262d2d1f6/src/bytecode/concrete.py#L1281-L1283 I wonder if the lookup should include cell and var names 🤔

[MatthieuDartiailh]

Happy to help 🙂 . Looking at this bit of code https://github.com/P403n1x87/bytecode/blob/4892a96180c6c5a12c65f06ef489413262d2d1f6/src/bytecode/concrete.py#L1281-L1283 I wonder if the lookup should include cell and var names 🤔

That's a good point. I do not know if the LOAD_FAST_LOAD_FAST support cell vars, definitively worth checking at least.

[P403n1x87]

Adding some more facts as I come across them. I think there is something wrong with reconstructing the value of the dual arg opcodes

DUAL_ARG_OPCODES 19  =>  ('service_name', 'api_version')
DUAL_ARG_OPCODES 16  =>  ('service_name', 'self')
DUAL_ARG_OPCODES 28  =>  ('service_name', 'available')
DUAL_ARG_OPCODES 19  =>  ('service_name', 'api_version')
DUAL_ARG_OPCODES 19  <=  ('service_name', 'api_version')
DUAL_ARG_OPCODES 16  <=  ('service_name', 'self')
DUAL_ARG_OPCODES 16  <=  ('service_name', 'available')
DUAL_ARG_OPCODES 19  <=  ('service_name', 'api_version')

The penultimate line should not give 16

[P403n1x87]

Decomposing the indices returned by add we get

DUAL_ARG_OPCODES 16 = (1, 16)  <=  ('service_name', 'available') ['self', 'service_name', 'region_name', 'api_version', 'use_ssl', 'verify', 'endpoint_url', 'aws_access_key_id', 'aws_secret_access_key', 'aws_session_token', 'config', 'resource_model', 'client', 'service_model', 'service_context', 'cls', 'available', 'has_low_level_client']

Indeed 16 = (1 << 4) | (16 & 15), so I think the problem is that one should make sure that the args of dual arg opcodes fit within the first 16 varnames

[P403n1x87]

This patch seems to fix the problem

diff --git a/src/bytecode/concrete.py b/src/bytecode/concrete.py
index 8b1e6f1..bfa27a9 100644
--- a/src/bytecode/concrete.py
+++ b/src/bytecode/concrete.py
@@ -1193,6 +1194,10 @@ class _ConvertBytecodeToConcrete:

         # We use None as a sentinel to ensure caches for the last instruction are
         # properly generated.
+        for instr in self.bytecode:
+            if isinstance(instr, Instr) and instr._opcode in DUAL_ARG_OPCODES:
+                for arg in instr.arg:
+                    self.add(self.varnames, arg)
         for instr in itertools.chain(self.bytecode, [None]):
             # Enforce proper use of CACHE opcode on Python 3.11+ by checking we get the
             # number we expect or directly generate the needed ones.
@@ -1278,8 +1283,8 @@ class _ConvertBytecodeToConcrete:
                         and isinstance(arg[0], str)
                         and isinstance(arg[1], str)
                     )
-                    arg = (self.add(self.varnames, arg[0]) << 4) + self.add(
-                        self.varnames, arg[1]
+                    arg = (self.add(self.varnames, arg[0]) << 4) | (
+                        self.add(self.varnames, arg[1]) & 15
                     )
                 elif PY313 and isinstance(arg, CellVar):
                     cell_instrs.append(len(self.instructions))

We basically make sure that we add those args to varnames asap to increase the odds we are among the first 16 entires. However, I believe varnames is initialised with the argument names, and this might be a problem for functions with a lot of arguments?

[MatthieuDartiailh]

In addition to your changeset, we should add a fallback to using 2 LOAD_FAST if one of the args overflows. This way, people can attempt to use the optimal solution safely.

[MatthieuDartiailh]

I pushed a fix but I will have to add tests also. If you can get me a reproducer for the CFG issue we can hopefully converge quickly.

[MatthieuDartiailh]

I addressed your comment and added a test. I will merge as is to rebase and merge the CI improvements and see if I can get to the bottom of the 3.11 issue.