Skip to content
This repository has been archived by the owner on Jan 23, 2024. It is now read-only.

fix: address some cases where jumps were not being updated #83

Merged
merged 2 commits into from
Apr 4, 2023

Conversation

mctavish
Copy link
Contributor

@mctavish mctavish commented Apr 3, 2023

In Python 3.10, due to the fact that jump instruction arguments have changed from byte offsets to instruction offsets, in some cases the jumps will not be updated because it is not noticed that their jump targets are being moved.

Consider the following example:

 39          12 LOAD_FAST                0 (index)
             14 LOAD_CONST               0 (None)
             16 IS_OP                    0
             18 POP_JUMP_IF_FALSE       12 (to 24)

 40          20 LOAD_CONST               1 ('Foo\n')
             22 RETURN_VALUE

 41     >>   24 SETUP_FINALLY           18 (to 62)

Adding a breakpoint on line 40 will insert the breakpoint code between the POP_JUMP_IF_FALSE and its target:

 39          12 LOAD_FAST                0 (index)
             14 LOAD_CONST               0 (None)
             16 IS_OP                    0
             18 POP_JUMP_IF_FALSE       12 (to 24)

 40          20 LOAD_CONST               7 (<built-in method Callback of cdbg_native._Callback object at 0x7f6236913fd0>)
             22 CALL_FUNCTION            0
        >>   24 POP_TOP
             26 LOAD_CONST               1 ('Foo\n')
             28 RETURN_VALUE

 41          30 SETUP_FINALLY           21 (to 74)

Note that the POP_JUMP_IF_FALSE target is not updated, and is pointing to the wrong instruction. This is because the argument is 12, which is incorrectly interpretted as the memory offset and thus is jumping to a code location that has not been moved.

The correct bytecode to generate is as follows:

 39          12 LOAD_FAST                0 (index)
             14 LOAD_CONST               0 (None)
             16 IS_OP                    0
             18 POP_JUMP_IF_FALSE       15 (to 30)

 40          20 LOAD_CONST               7 (<built-in method Callback of cdbg_native._Callback object at 0x7f6236913fd0>)
             22 CALL_FUNCTION            0
             24 POP_TOP
             26 LOAD_CONST               1 ('Foo\n')
             28 RETURN_VALUE

 41     >>   30 SETUP_FINALLY           21 (to 74)

@mctavish mctavish requested a review from jasonborg April 3, 2023 21:24
@mctavish mctavish merged commit ed9d2b9 into GoogleCloudPlatform:main Apr 4, 2023
@mctavish mctavish deleted the bugfix branch April 4, 2023 13:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants