Repository navigation
Line numbers for errors off in traces #106290
Description
Activity
Having spent some time trying to debug this, I still have no solution. I do know this: the invariants in the code regarding
next_instr,frame->prev_instr, and the like, are devilish, and I'm not sure what assumptionsENTER_EXECUTORis making.I think that whenever we're in the body of an instruction (i.e. in bytecodes.c):
frame->prev_instrpoints to the word containing the opcode+oparg of the current instructionnext_instrinitially points to the next word; when the instruction exits, it must have skipped over the cache, or jumped to a new instruction
When executing regular bytecode, upon failing a
DEOPT_IFguard, neither of these is changed. Upon failing anERROR_IFcheck,next_instris unused andframe->prev_instris incorporated (astb_lasti) in the traceback.When executing uops, there are at least three reasons to distinguish for leaving
_PyUopExecute()(and that's just assuming we don't switch to a new frame):- We execute the
EXIT_TRACEuop - We fail a
DEOPT_IFguard - We fail an
ERROR_IFcheck
The
ENTER_EXECUTORopcode doesn't distinguish between the first two, and in these cases always resumes whereframe->prev_instrpoints. (When resuming it setsnext_instrto one more thanframe->prev_instr-- this is to restore the invariant of the top two bullets above.)When the executor returns
NULL, the assumption is that (after restoringframefromcframe.current_frame)frame->prev_instrpoints to the instruction that raised the error.But when I put all this together something's off! I generate uop sequences like this:
(more uops) LOAD_FAST_CHECK (n) SET_IP (ip) EXIT_TRACEwhere
ipis the instruction offset of the originalLOAD_FAST_CHECKbytecode, but to make everything (mostly) work theSET_IPopcode must subtract one fromipbefore storing the value inframe->prev_instr(adding the start of the bytecode sequence of course). For example, if I have a bytecodeLOAD_FAST_CHECK 2at byte offset 20, i.e. instruction offset 10, I generateSET_IP 10, but it must storeco_code_adaptive + 9intoframe->prev_instr. This ensures thatDEOPT_IFandEXIT_TRACEwork;ERROR_IFalmost works, but setstb_lastito9and as a result the traceback is subtly wrong. I have attempted various other permutations, but none work.I fear to change the logic in
JUMP_BACKWARDorENTER_EXECUTOR, but maybe that's the real culprit? That would also require changes to the counter-optimizer used to test the infrastructure though.I also believe that if the first uop in a trace fails an
ERROR_IF,frame->prev_instris still pointing at theENTER_EXECUTORinstruction. (And quite possibly the same forDEOPT_IF, but I don't have a way to repro that.)PS. The one thing that's not at fault here is the stack pointer. The logic around setting and restoring that is clear and unimpeachable.
Okay, I think I've got it. In the
EXIT_TRACEuop, we need to decrementframe->prev_instrin order for the rest of the logic to work.
Explanation: When we return from the executor, we
goto resume_frame, where the following code is executed (after expanding the macroSET_LOCALS_FROM_FRAME()):resume_frame: next_instr = frame->prev_instr + 1; stack_pointer = _PyFrame_GetStackPointer(frame); // (Some debug stuff omitted) DISPATCH();The
DISPATCH()macro then uses either computed goto orswitchto execute the instruction atnext_instr.All in all this means that if we have a "destination" opcode indicated by the final
SET_IPuop, we must arrange forframe->prev_instrto point one word before that destination, so that the code at theresume_framelabel correctly setsnext_instrto the destination.When we exit because of a failed
DEOPT_IFguard,frame->prev_instrpoints to the bytecode that needs to be restarted; again, we need to decrement it before returning from the uop executor -- the executor's caller has no special case for deopt, we must make it jump to the bytecode that needs to be re-executed. (This case is handled differently in Tier 2 than in Tier 1 -- in Tier 1, we jump straight to the implementation of the unspecialized bytecode, leaving all other state unchanged; in Tier 2, we actually re-execute the specialized bytecode, counting on it to fail the same guard and then jump to the case for the unspecialized bytecode.OTOH, when we exit because of a failed
ERROR_IF, we must keepframe->prev_instrpointing at the bytecode that failed, since that is the situation expected by the error handling code.I suppose there are two ways to fix this, given my implementation:
- In
SAVE_IP n, makeframe->prev_instrpoint at instruction offsetn - 1; increment it on an error exit - In
SAVE_IP n, makeframe->prev_instrpoint at instruction offsetn; decrement it on regular or deoptimizing exit
In my PR I have used (2), since it emulates the state of the Tier 1 interpreter (during execution of a certain bytecode,
frame->prev_instrpoints to that instruction).We could also incorporate the decrement in the caller of the executor, but beware: there are two places where it's called:
- In the
ENTER_EXECUTORbytecode - In
_PyOptimizer_BackEdge(), when it inserts anENTER_EXECUTORinstruction
- In
When a uop encounters an error, the line number is set to -1 or to the wrong place.
@markshannon
Repro (on latest main):
Run this normally and you get this traceback:
But run with
-Xuops, and you get:Linked PRs