Description

tom-pytel

opened on Aug 17, 2022

Issue body actions

Bug report

Opcodes which have caches (specifically adaptive calls) leave frame->prev_instr pointing to the last byte of the cache instead of the actual last instruction.

Your environment

v3.11.0rc1
Linux tom-VirtualBox 5.15.0-46-generic #49~20.04.1-Ubuntu SMP Thu Aug 4 19:15:44 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Activity

1. tom-pytel

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Aug 17, 2022
2. tom-pytel

   added a commit that references this issue on Aug 17, 2022

   [3.11] pythongh-96049: frame->prev_instr incorrect state

   f513a07
3. brandtbucher

   self-assigned this

   on Aug 17, 2022
4. brandtbucher

   added

   3.11only security fixesonly security fixes

   3.12only security fixesonly security fixes

   on Aug 17, 2022
5. brandtbucher commented on Aug 17, 2022

   

   brandtbucher

   on Aug 17, 2022

   Member

   More actions

   This is intentional. See this comment in the code:

   cpython/Include/internal/pycore_frame.h

   Lines 58 to 62 in d8c07f8

   	// NOTE: This is not necessarily the last instruction started in the given
   	// frame. Rather, it is the code unit *prior to* the *next* instruction. For
   	// example, it may be an inline CACHE entry, an instruction we just jumped
   	// over, or (in the case of a newly-created frame) a totally invalid value:
   	_Py_CODEUNIT *prev_instr;

   For my own understanding, how has this negatively affected you?

   CC @markshannon
6. tom-pytel commented on Aug 17, 2022

   

   tom-pytel

   on Aug 17, 2022

   ContributorAuthor

   More actions

   For my own understanding, how has this negatively affected you?

   CC @markshannon

   Ok, I missed that quote, just figured since all other opcodes left the pointer at the last instruction these should as well since otherwise the location of the original instruction is unrecoverable (due to random data in cache area).

   This is problematic for us because we have an error reporting fork of the cpython interpreter as a product and after an exception we want to get the instruction which caused it. This particular patch leaves the last instruction pointer always valid but if it is not an issue and not desirable for the base cpython implementation then I will close the PR and just keep the change on our branch.
7. brandtbucher commented on Aug 17, 2022

   

   brandtbucher

   on Aug 17, 2022

   Member

   More actions

   Ok, I missed that quote, just figured since all other opcodes left the pointer at the last instruction these should as well since otherwise the location of the original instruction is unrecoverable (due to random data in cache area).

   It is recoverable, just with a bit of effort. A call to PyCode_GetCode (or accessing the co_code member in the Python layer) will give you a copy of the bytecode with the caches cleared. It's safe to scan backwards over this to find the last "real" instruction.
8. brandtbucher

   closed this as not plannedon Aug 17, 2022

Issue actions