bytecode
bytecode copied to clipboard
MatthieuDartiailh
Add support for Python 3.11
Necessary changes an current status:
- [x] encode the exception table at the ConcreteBytecode level
- [x] encode the exception at the Bytecode level done through the use of TryBegin, TryEnd pseudo-instructions
- [x] allow to compute the stack depth in the exception table at the CFG level
- [x] handle the ability for LOAD_GLOBAL to push an extra NULL
- [x] ensure mandatory CACHE opcodes are present
- [x] auto-generate CACHE when going from the Bytecode to the ConcreteBytecode level
- [x] fix the generation of the linestable
- [x] add end line, col_offset and end_col_offset to Instr
- [ ] fix label handling in the presence of jump backward instructions
- [ ] adjust the computation of stack effect for all opcodes
- [ ] adjust the inference of flags
- [ ] fix existing tests
@iritkatriel if you to have a early look at the encoding of the exception table through pseudo-instruction at the Bytecode level feel free to do so.
I tried to implement stack depth computation on the CFG for the exception table but I do not have it right yet it seems.
For the following simple case:
def try_except():
try:
a = 1
except Exception:
return 0
dis display the following:
5 0 RESUME 0
6 2 NOP
7 4 LOAD_CONST 1 (1)
6 STORE_FAST 0 (a)
8 LOAD_CONST 0 (None)
10 RETURN_VALUE
>> 12 PUSH_EXC_INFO
8 14 LOAD_GLOBAL 0 (Exception)
26 CHECK_EXC_MATCH
28 POP_JUMP_FORWARD_IF_FALSE 4 (to 38)
30 POP_TOP
9 32 POP_EXCEPT
34 LOAD_CONST 2 (0)
36 RETURN_VALUE
8 >> 38 RERAISE 0
>> 40 COPY 3
42 POP_EXCEPT
44 RERAISE 1
ExceptionTable:
4 to 6 -> 12 [0]
12 to 30 -> 40 [1] lasti
38 to 38 -> 40 [1] lasti
But after a round trip I get:
5 0 RESUME 0
6 2 NOP
7 4 LOAD_CONST 1 (1)
6 STORE_FAST 0 (a)
8 LOAD_CONST 0 (None)
10 RETURN_VALUE
>> 12 PUSH_EXC_INFO
8 14 LOAD_GLOBAL 0 (Exception)
26 CHECK_EXC_MATCH
28 POP_JUMP_FORWARD_IF_FALSE 4 (to 38)
30 POP_TOP
9 32 POP_EXCEPT
34 LOAD_CONST 2 (0)
36 RETURN_VALUE
8 >> 38 RERAISE 0
>> 40 COPY 3
42 POP_EXCEPT
44 RERAISE 1
ExceptionTable:
4 to 6 -> 12 [0]
12 to 30 -> 40 [2] lasti
38 to 38 -> 40 [2] lasti
What happens is that when computing the stack depth from the second way to enter the block the effect of RERAISE is not taken into effect and as a consequence we enter with a depth of 2.
The strategy we use to compute stack effect is very close to CPython own one. For the exception table each time I see a TryBegin, I push a computation for the target block with the current size. Obviously no exception can be raised before any instruction is executed but I did not see any logic in CPython codebase justifying to delay things. Should RERAISE be treated in a special manner ?
I would welcome any insight in this. @iritkatriel maybe ?
What happens is that when computing the stack depth from the second way to enter the block the effect of RERAISE is not taken into effect and as a consequence we enter with a depth of 2.
I'm trying to understand what you mean by "the second way". This first way goes through the RERAISE, so that would be via a jump from offset 28 (which means the exception did not match the except clause's type, so it was propagated on via the reraise). So the second way would be that the exception did match, but then an exception was raised inside the except block. Is this correct?
The strategy we use to compute stack effect is very close to CPython own one. For the exception table each time I see a TryBegin, I push a computation for the target block with the current size. Obviously no exception can be raised before any instruction is executed but I did not see any logic in CPython codebase justifying to delay things. Should RERAISE be treated in a special manner ?
If I understand correctly, the reraise is irrelevant to the "second way" - the problem is with the stack depth assumed at the time the exception was raised in the except handler. Note that the stack depth is reset by the eval loop to whatever is in the exception table for this line.
Sorry for the lack of clarity @iritkatriel
My point is that the block starting at offset 40 is the target of 2 exception entry:
- the one covering 12 to 30
- the one covering offset 38
As a consequence I have 2 TryBegin pointing to that block and in the calculation of the stacksize, I compute the stacksize once
with the size before the first TryBegin (right before PUSH_EXC_INFO) as input and once with the size before the second one (just before RERAISE). I suspect something is not quite right with this procedure but I cannot figure out what is wrong.
For reference here is the bytecode content (the CACHE are omitted) (the TryBegin do not have a nice repr)
<RESUME arg=0>
<NOP>
<bytecode.instr.TryBegin object at 0x000001AC69AB2F80>
<LOAD_CONST arg=1>
<STORE_FAST arg='a'>
<bytecode.instr.TryEnd object at 0x000001AC6A140370>
<LOAD_CONST arg=None>
<RETURN_VALUE>
<bytecode.instr.Label object at 0x000001AC695CBA00>
<bytecode.instr.TryBegin object at 0x000001AC6AA1B940>
<PUSH_EXC_INFO>
<LOAD_GLOBAL arg=(False, 'Exception')>
<CHECK_EXC_MATCH>
<POP_JUMP_FORWARD_IF_FALSE arg=<bytecode.instr.Label object at 0x000001AC695CB9E0>>
<POP_TOP >
<bytecode.instr.TryEnd object at 0x000001AC6A140610>
<POP_EXCEPT>
<LOAD_CONST arg=0>
<RETURN_VALUE >
<bytecode.instr.Label object at 0x000001AC695CB9E0>
<bytecode.instr.TryBegin object at 0x000001AC6A127940>
<RERAISE arg=0 >
<bytecode.instr.TryEnd object at 0x000001AC6A1401F0>
<bytecode.instr.Label object at 0x000001AC695CBEE0>
<COPY arg=3 >
<POP_EXCEPT >
<RERAISE arg=1 >
As a consequence I have 2
TryBeginpointing to that block and in the calculation of the stacksize, I compute the stacksize once with the size before the first TryBegin (right before PUSH_EXC_INFO) as input and once with the size before the second one (just before RERAISE).
I see - so you are assuming that the stackdepth at the beginning of the except block is equal to the stack depth just before the TryBegin? That wouldn't be correct for the RERAISE path, because RERAISE pops the exception from the stack before reraising it, so depth decreases by 1. Maybe you can special-case RERAISE, but I don't know if you will run into other issues. (You are interpreting one try-except as two try-excepts, right?)
I will try. It is not really a question of interpreting one as two it is just that at the bytecode level I have nothing else than the exception table to go by.
I have a small side question @iritkatriel. According to exception_handling_notes.txt found in cpython repo, exception are pushed as three values. Is this still true ? I am asking because with the RERAISE workaround I and assuming the exception is pushed as a single value I get 2 tests to pass but if it is three nothing work.
Also the RERAISE workaround does not seem to be enough. A try/except/finally test crashes as follow:
def try_except_finally():
try:
a = 1
except Exception:
return 0
finally:
c = 1
CPython
30 0 RESUME 0
31 2 NOP
32 4 LOAD_CONST 1 (1)
6 STORE_FAST 0 (a)
36 8 LOAD_CONST 1 (1)
10 STORE_FAST 1 (b)
12 JUMP_FORWARD 19 (to 52)
>> 14 PUSH_EXC_INFO
33 16 LOAD_GLOBAL 0 (Exception)
28 CHECK_EXC_MATCH
30 POP_JUMP_FORWARD_IF_FALSE 6 (to 44)
32 POP_TOP
34 34 POP_EXCEPT
38 36 LOAD_CONST 1 (1)
38 STORE_FAST 2 (c)
40 LOAD_CONST 2 (0)
42 RETURN_VALUE
33 >> 44 RERAISE 0
>> 46 COPY 3
48 POP_EXCEPT
50 RERAISE 1
36 >> 52 NOP
38 54 LOAD_CONST 1 (1)
56 STORE_FAST 2 (c)
58 LOAD_CONST 0 (None)
60 RETURN_VALUE
>> 62 PUSH_EXC_INFO
64 LOAD_CONST 1 (1)
66 STORE_FAST 2 (c)
68 RERAISE 0
>> 70 COPY 3
72 POP_EXCEPT
74 RERAISE 1
ExceptionTable:
4 to 6 -> 14 [0]
8 to 12 -> 62 [0]
14 to 32 -> 46 [1] lasti
34 to 34 -> 62 [0]
44 to 44 -> 46 [1] lasti
46 to 50 -> 62 [0]
62 to 68 -> 70 [1] lasti
Bytecode roundtrip
30 0 RESUME 0
31 2 NOP
32 4 LOAD_CONST 1 (1)
6 STORE_FAST 0 (a)
36 8 LOAD_CONST 1 (1)
10 STORE_FAST 1 (b)
12 JUMP_FORWARD 19 (to 52)
>> 14 PUSH_EXC_INFO
33 16 LOAD_GLOBAL 0 (Exception)
28 CHECK_EXC_MATCH
30 POP_JUMP_FORWARD_IF_FALSE 6 (to 44)
32 POP_TOP
34 34 POP_EXCEPT
38 36 LOAD_CONST 1 (1)
38 STORE_FAST 2 (c)
40 LOAD_CONST 2 (0)
42 RETURN_VALUE
33 >> 44 RERAISE 0
>> 46 COPY 3
48 POP_EXCEPT
50 RERAISE 1
36 >> 52 NOP
38 54 LOAD_CONST 1 (1)
56 STORE_FAST 2 (c)
58 LOAD_CONST 0 (None)
60 RETURN_VALUE
>> 62 PUSH_EXC_INFO
64 LOAD_CONST 1 (1)
66 STORE_FAST 2 (c)
68 RERAISE 0
>> 70 COPY 3
72 POP_EXCEPT
74 RERAISE 1
ExceptionTable:
4 to 6 -> 14 [0]
8 to 12 -> 62 [3]
14 to 32 -> 46 [1] lasti
34 to 34 -> 62 [3]
44 to 44 -> 46 [1] lasti
46 to 50 -> 62 [3]
62 to 68 -> 70 [4] lasti
It seems to me that block meant to reraise an exception are fully traversed and the resulting size is used as entry size of the exception handling block. I guess my main issue is that I do not fully comprehend yet what the stacksize in the exception table represent. My naive intuition was that it was the stack size at the entrance of the region concerned by an exception table entry such as the interpreter could cancel anything that happened within the block but that does not seem to be true. Could you enlighten me or point to the right resource ?
According to exception_handling_notes.txt found in cpython repo, exception are pushed as three values. Is this still true ?
That's out of date, I'll look into it.
My naive intuition was that it was the stack size at the entrance of the region concerned by an exception table entry such as the interpreter could cancel anything that happened within the block but that does not seem to be true.
I think that is true.
Thanks for your response @iritkatriel I left a small comment on your PR. Also thanks for confirming that I understood correctly what the stack size represent in the exception table.
I am still confused by my first case. Below I try to annotate it with the stack state (after execution) as I understand it at each step which agrees with my implementation but not with CPython.
5 0 RESUME 0 Empty stack []
6 2 NOP Empty stack []
-> the exception entry starting at 4 start with a stack size of 0
7 4 LOAD_CONST 1 (1) Stack = [1]
6 STORE_FAST 0 (a) Stack = []
8 LOAD_CONST 0 (None) Stack = [None]
10 RETURN_VALUE
Entered as part of handling an exception so we push the raised exception, the stack size is 1
Depending if we count that implicit push or not we get 0 or 1 for exception table entry going from 12 to 30
>> 12 PUSH_EXC_INFO Stack = [current exception, raised exception]
8 14 LOAD_GLOBAL 0 (Exception) Stack = [current exception, raised exception, Exception]
26 CHECK_EXC_MATCH Stack = [current exception, raised exception, bool]
28 POP_JUMP_FORWARD_IF_FALSE 4 (to 38) Stack = [current exception, raised exception]
30 POP_TOP Stack = [current exception]
9 32 POP_EXCEPT Stack = []
34 LOAD_CONST 2 (0) Stack = [0]
36 RETURN_VALUE
Entered through the jump from 28 so with a stack of [current exception, raised exception] with size 2 -> according to CPython exception table it should be 1
8 >> 38 RERAISE 0 Stack = [current exception]
Reached through the exception table with lasti of True so we have [previous exception, raised exception, index]
>> 40 COPY 3 Stack = [previous exception, raised exception, index, previous exception]
42 POP_EXCEPT Stack = [previous exception, raised exception, index]
44 RERAISE 1 Stack = [previous exception, raised exception]
ExceptionTable:
4 to 6 -> 12 [0]
12 to 30 -> 40 [1] lasti
38 to 38 -> 40 [1] lasti
Honestly the last block makes no sense to me the COPY, POP_EXCEPT pair looks like a no-op to me. Also it seems I am misunderstanding what happens at 12, 14, 26 and 28 since this is where the discrepancy stems from.
Looking at compile.c I still references to is_block_push which looks at SETUP_ opcodes which do not exist in the final form of the bytecode. So am I right to think that the stack calculation is not done on what dis displays but on some IR ?
EDIT: sorry the formatting looks so bad, the raw version is more readable I believe
Honestly the last block makes no sense to me the COPY, POP_EXCEPT pair looks like a no-op to me.
POP_EXCEPT not only pops the exception from the stack, it also sets the interpreter state's exc_info to this exception. So the purpose of COPY 3, POP_EXCEPT is to restore the exc_info to the third item in the stack. Then the TOS is reraised.
Also it seems I am misunderstanding what happens at 12, 14, 26 and 28 since this is where the discrepancy stems from.
These lines are checking whether the raised exception matches the type of the "except Exception" clause. So it loads the type (Exception), does the match check, which pushes a boolean, then the jump is conditioned on this boolean. If we don't jump (there was a match) then the POP removes the exception from the stack. If we do jump (no match) then we still have the exception on the stack - it will either match another except or be reraised.
Looking at compile.c I still references to
is_block_pushwhich looks at SETUP_ opcodes which do not exist in the final form of the bytecode. So am I right to think that the stack calculation is not done on what dis displays but on some IR ?
Correct. The SETUP_ instructions are included in the stack depth calculation, but are removed around the time that the exception table is constructed, so they are not in the final bytecode. I think this is where the issue is - you are implicitly assuming that each exception table line corresponds to a try (i.e., a SETUP_ opcode) but that is not always the case.
Thanks for the detailed explanation.
It seems I am left with no other option than to reconstruct the setup clause logic from the exception table for the sake of stack depth calculation. I had hoped to avoid that. Out of curiosity are all the blocks already present at the time of the stack depth calculations or are some new ones introduced when the SETUP_ instructions are removed ? I am asking since before part of the reraise exception was not visible so I am wondering at which point in the compiler it gets introduced.
Thanks for the detailed explanation.
It seems I am left with no other option than to reconstruct the setup clause logic from the exception table for the sake of stack depth calculation. I had hoped to avoid that. Out of curiosity are all the blocks already present at the time of the stack depth calculations or are some new ones introduced when the SETUP_ instructions are removed ?
By block you mean the basicblocks? They should pretty much be there. There is a later step where blocks are reordered, and it can cause a block with just a jump instruction to be added, but that shouldn't change anything substantial w.r.t. the exception table.
I am asking since before part of the reraise exception was not visible so I am wondering at which point in the compiler it gets introduced.
What do you mean by "part of the reraise exception was not visible"?
Yes I meant basic blocks.
From the top of my head I do not remember the existence of the equivalent of the COPY/POP_EXCEPT/RERAISE in earlier version bur that may just be bad memory on my side. I will investigate.
There used to be a single opcode that did all of that, called I think POP_EXCEPT_AND_RERAISE. When exceptions became 1 instead of 3 items, we didn’t need it anymore.
For the simple try/except I have been looking at 3.10 dis gives:
2 0 SETUP_FINALLY 5 (to 12)
3 2 LOAD_CONST 1 (1)
4 STORE_FAST 0 (a)
6 POP_BLOCK
8 LOAD_CONST 0 (None)
10 RETURN_VALUE
4 >> 12 DUP_TOP
14 LOAD_GLOBAL 0 (Exception)
16 JUMP_IF_NOT_EXC_MATCH 15 (to 30)
18 POP_TOP
20 POP_TOP
22 POP_TOP
5 24 POP_EXCEPT
26 LOAD_CONST 2 (0)
28 RETURN_VALUE
4 >> 30 RERAISE 0
There is a single reraise rather than the explicit handling of possible exceptions within the exception handler which is what looked simpler to me. I will investigate being smarter about exception table entry corresponding to exception handling code.
Going through compile.c I found this line that is slightly surprising to me: https://github.com/python/cpython/blob/3.11/Python/compile.c#L7448
@iritkatriel could explain to me why the block size is adjusted in this way ?
Going through compile.c I found this line that is slightly surprising to me: https://github.com/python/cpython/blob/3.11/Python/compile.c#L7448
@iritkatriel could explain to me why the block size is adjusted in this way ?
I think it's calculating the stacksize before the exception was raised, so it does -1 for the exception itself, and then another -1 for lasti, but only if this block expects lasti to be in the stack, and this depends on which SETUP_ instruction set up this try block.
So the stack size indicated in the exception table does not account for the extra element that are pushed by the interpreter before the execution of the block start, right ? If so I was already doing that in my calculation so it won't help fixing my issues. Thanks anyway
Hi @iritkatriel
I made some progress by basing my analysis on the hypothesis that the entrance stack effect for a exception handler should be the minimum stack depth seen since it is the only one that can be restored safely. It work surprisingly well on try/except/finally cases (I need to add exception group cases however).
I am now struggling with with statement but it does not seem directly related to exception but more to an issue in stack calculation. I would appreciate it very much if you can give the following a quick look.
def with_no_store():
with contextlib.nullcontext(1):
a = 1
return a
The dis of this function is as follow, with the state of the stack as I understand it.
84 0 RESUME 0
85 2 LOAD_GLOBAL 1 (NULL + contextlib) [NULL, contextlib]
14 LOAD_ATTR 1 (nullcontext) [NULL, nullcontext]
24 LOAD_CONST 1 (1) [NULL, nullcontext, 1]
26 PRECALL 1 [NULL, func]
30 CALL 1 [return value]
40 BEFORE_WITH [exit, return value]
42 POP_TOP [exit]
86 44 LOAD_CONST 1 (1) [exit, 1]
46 STORE_FAST 0 (a) [exit]
85 48 LOAD_CONST 0 (None) [exit, None]
50 LOAD_CONST 0 (None) [exit, None, None]
52 LOAD_CONST 0 (None) [exit, None, None, None]
54 PRECALL 2 [exit, args]
58 CALL 2 [return]
68 POP_TOP []
70 JUMP_FORWARD 11 (to 94)
>> 72 PUSH_EXC_INFO
74 WITH_EXCEPT_START
76 POP_JUMP_FORWARD_IF_TRUE 4 (to 86)
78 RERAISE 2
>> 80 COPY 3
82 POP_EXCEPT
84 RERAISE 1
>> 86 POP_TOP
88 POP_EXCEPT
90 POP_TOP
92 POP_TOP
87 >> 94 LOAD_FAST 0 (a)
96 RETURN_VALUE
ExceptionTable:
42 to 46 -> 72 [1] lasti
72 to 78 -> 80 [3] lasti
86 to 86 -> 80 [3] lasti
My issue comes from the first entry. in the exception table. When we encounter the POP_TOP, I expect to have 2 element on the stack: the context manager __exit__ and the return value of the context manager __enter__. However the stack depth in the exception table is 1. Which kind of make from an execution point of view (we only preserve __exit__) but does not make sense just looking at the stack effect. Am I missing something obvious ? is POP_TOP special cased somehow ?
Through some extra iterations I managed to get with statements working by special casing the impact of BEFORE_WITH and BEFORE_ASYNC_WITH. I expanded the set of test cases and I would love to get feedback while I work on improving and fixing the tests.
There is a difference between SETUP_WITH and other SETUP_*s in terms of stack effect. Since you don't see the SETUP_WITH, I guess you're compensating for that by doing something with the other WITH opcodes?
What I found in an empirical manner was that the exception handling basic block for the instructions following a BEFORE_ instruction did not account for the return value of __enter__. This is reasonable since it either stored immediately or popped and the unwinding cannot promise to preserve it. However since the instruction responsible for this behavior is the first of the exception table entry I had to adapt the logic. I decided to basically track when I see a BEFORE_ instr and for the first TryBegin I encounter I adjust the size by 1.
This was not too hard to get going. The idea to use the minimal entry size took me quite a bit longer to get to. But it seems to work fine.
Digging more I found cases (test_cfg.py:::CFGStacksizeComputationTests::test_stack_size_computation_try_except_finally) where the above heuristic fails due to TryBegin/POP_BLOCK/TryEnd structure or similar. I came to the conclusion that if a TryBegin/TryEnd region sees a net decrease in stack size I need to use the smallest size as entry of the exception handling block since this is the only one safe to restore.
Does this make sense @iritkatriel ?
The cfg tests are now passing !!! after yet another iteration on the stack computation that now badly needs to be refactored. Slowly we are getting there.
@MatthieuDartiailh I just wanted to check if you have a tentative date by which this PR could be completed 🙏
I prefer not to give you a date. My open source time has been limited lately (both for professional and personnal reasons) and even if this is my first priority, I still need to do some minimal work for the other project I maintain.
I refactored the stacksize computation bit I still have to test and push that work. And address the remaining bullet points for the first post. None of them are as involved as the stack size computation but fixing tests can be time consuming. I will let you know when this can be beta tested and reviewed.
I will let you know when this can be beta tested and reviewed.
Sure, totally understandable! If you think there's anything I could have a look at just let me know! Thanks for all your work on this project!
Currently everything should kind of work. But since I want to change the stacksize computation I think it makes sense for you to wait till I land this change.
After things like verification of the validity of the stack, flag inference and proper qualname should be less problematic.
@P403n1x87 I pushed the refactor of the stack size computation. So apart from the points still listed in the first comments, things should work. Feel free to test.
@iritkatriel if you have some time I would appreciate if you could check the logic of the stack size computation.