Error handling for some instructions is incorrect

[brandtbucher]

In several places, we have goto error; branches in bytecode instructions that occur after modifying the next_instr pointer. This is incorrect, since the error branch will behave as if the error occurred in the new location (most often an adjacent instruction). The result could be as benign as an incorrect location in a traceback, or as problematic as incorrect control flow in or near a try/except block.

I tried for a bit to make the compiler emit code that did the wrong thing here, and I wasn't able to. So this is mostly a theoretical concern (but still worth fixing).

• PR: GH-99298: Don't perform jumps before error handling #99299

• PR: [3.11] GH-99298: Don't perform jumps before error handling #99343

• PR: GH-99298: Clean up attribute specializations #99398

[gvanrossum]

Interesting. There are now some instrs that have JUMPY(<cache size>); CHECK_EVAL_BREAKER(); .Once those instrs get cache effects the JUMPBY() would come from the generator; can it safely be moved after the eval_breaker check, or does the generator now need to generate those checks too (presumably based on some one-of flag in the instr definition)?

[brandtbucher]

The JUMPBY(...); needs to happen before the CHECK_EVAL_BREAKER(); (as it does now) since we need to dispatch to the next instruction after handling eval breakers.

(Note also that error handling for eval breakers is unaffected by this issue, since we can just pretend that any errors raised by eval breaker checks happened as part of the next instruction... basically, we act as if eval breakers are checked just before the next instruction, rather than just after the current one.)

[brandtbucher]

So yeah, we probably need a way to spell this as part of the DSL. But it might be able to wait for now... I think only CALL, CALL_FUNCTION_EX, and JUMP_BACKWARD are affected (RESUME is a weird one... it checks the eval breaker only on certain opargs).

[brandtbucher]

Another option could be to make CHECK_EVAL_BREAKER its own instruction, and put it after every call and before every backward jump. Not sure how expensive that would be, though.

[gvanrossum]

I'd rather not add more opcodes. These can use the legacy format for now; eventually we'll add some silly little flag to the instruction definition format.