[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Irit Katriel]

Irit Katriel  added the comment:


New changeset c5bfb88eb6f82111bb1603ae9d78d0476b552d66 by Irit Katriel in 
branch '3.10':
[3.10] bpo-45757: Fix bug where dis produced an incorrect oparg on EXTENDED_ARG 
before a no-arg opcode (GH-29480) (GH-29506)
https://github.com/python/cpython/commit/c5bfb88eb6f82111bb1603ae9d78d0476b552d66


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Irit Katriel]

Change by Irit Katriel :


--
pull_requests: +27757
pull_request: https://github.com/python/cpython/pull/29506

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Irit Katriel]

Irit Katriel  added the comment:

The 3.9 backport had a conflict, but I think it's not worth worrying about 
because this scenario only showed up in 3.10.

--
versions:  -Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 5.0 -> 6.0
pull_requests: +27752
pull_request: https://github.com/python/cpython/pull/29502

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Irit Katriel]

Irit Katriel  added the comment:


New changeset cb414cf0e207668300c4fe3f310c0bd249153273 by Irit Katriel in 
branch 'main':
bpo-45757: Fix bug where dis produced an incorrect oparg on EXTENDED_ARG before 
a no-arg opcode (GH-29480)
https://github.com/python/cpython/commit/cb414cf0e207668300c4fe3f310c0bd249153273


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Brandt Bucher]

Brandt Bucher  added the comment:

I don't think that it does, since oparg gets totally reassigned each time we 
load a new instruction. EXTENDED_ARG actually needs to hack around this by 
advancing the instruction itself, updating oparg, and jumping straight into the 
next opcode.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Irit Katriel]

Irit Katriel  added the comment:

We should also verify that the interpreter doesn’t have its own version of the 
dis bug.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Brandt Bucher]

Brandt Bucher  added the comment:

I have a hunch that this is caused in optimize_basic_block. In general, we 
don't bother clearing the oparg when peepholing instructions into NOPs.

We could either start becoming more principled about that and fix all of the 
existing NOPs, or just update instrsize and write_op_arg to ignore args for 
instructions that don't use them.

The latter seems easier and less error-prone.

--
nosy: +brandtbucher

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Rok Mandeljc]

Rok Mandeljc  added the comment:

The EXTENDED_ARG + NOP sequence seems to be specific to python 3.10; we could 
not reproduce the original bug (dis failing on telegram.message) with earlier 
python versions. The combination makes little sense, so perhaps it is a bug on 
its own?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Irit Katriel]

Irit Katriel  added the comment:

+ Mark for FYI. 

The behaviour of dis is not new, I wonder if generating code with EXTENDED_ARG 
+ NOP is new?

--
nosy: +Mark.Shannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Irit Katriel]

Change by Irit Katriel :


--
keywords: +patch
pull_requests: +27731
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/29480

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Irit Katriel]

Change by Irit Katriel :


--
versions: +Python 3.11 -Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Irit Katriel]

Irit Katriel  added the comment:

Thanks for the report. This should fix it, I'll create a patch:

@@ -428,6 +428,7 @@ def _unpack_opargs(code):
 extended_arg = (arg << 8) if op == EXTENDED_ARG else 0
 else:
 arg = None
+extended_arg = 0
 yield (i, op, arg)

--
nosy: +iritkatriel, serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45757] dis module incorrectly handles EXTENDED_ARG + NOP sequence

[Rok Mandeljc]

New submission from Rok Mandeljc :

dis module incorrectly handles the instruction sequence where EXTENDED_ARG is 
followed by NOP, e.g.,:

EXTENDED_ARG 0x01
NOP
EXTENDED_ARG 0x01
LOAD_CONST 0x29

The above sequence loads the constant from index 0x0129, whereas dis attempts 
to load it from index 0x010129, resulting in "IndexError: tuple index out of 
range error" when attempting to iterate over instructions returned by 
dis.get_instructions().

Complete example:

# *** example.py begin ***
import types
import dis

# Create a test code object...
constants = [None] * (0x000129 + 1)
constants[0x000129] = "Hello world!"

code = types.CodeType(
0,  # argcount
0,  # posonlyargcount
0,  # kwonlyargcount
0,  # nlocals
1,  # stacksize
64,  # flags
bytes([
0x90, 0x01,  # EXTENDED_ARG 0x01
0x09, 0xFF,  # NOP 0xFF
0x90, 0x01,  # EXTENDED_ARG 0x01
0x64, 0x29,  # LOAD_CONST 0x29
0x53, 0x00,  # RETURN_VALUE 0x00
]),  # codestring=
tuple(constants),  # constants
(),  # names
(),  # varnames
'',  # filename
'code',  # name
1,  # firstlineno
b''  # linetable
)

# ... and eval it to show that NOP resets EXTENDED_ARG
print("Output:", eval(code))

# Try to list all instructions via dis
print(list(dis.get_instructions(code)))

# *** example.py end ***

Running the example gives us:

Output: Hello world!
Traceback (most recent call last):
  File "/home/rok/example.py", line 35, in 
print(list(dis.get_instructions(code)))
  File "/usr/lib64/python3.10/dis.py", line 338, in _get_instructions_bytes
argval, argrepr = _get_const_info(arg, constants)
  File "/usr/lib64/python3.10/dis.py", line 292, in _get_const_info
argval = const_list[const_index]
IndexError: tuple index out of range


To fix the problem on dis side, the extended_arg in dis._unpack_opargs should 
be reset to 0 when NOP (or perhaps any opcode without argument) is encountered. 
I.e., by adding "extended_arg = 0" here:
https://github.com/python/cpython/blob/91275207296c39e495fe118019a757c4ddefede8/Lib/dis.py#L525



The problematic behavior was reported by users of PyInstaller under python 
3.10; it seems that python 3.10 generates bytecode involving EXTENDED_ARG + NOP 
for telegram.message.py:
https://raw.githubusercontent.com/python-telegram-bot/python-telegram-bot/v13.7/telegram/message.py

The original PyInstaller issue with preliminary analysis is here: 
https://github.com/pyinstaller/pyinstaller/issues/6301

--
components: Library (Lib)
messages: 405985
nosy: rok.mandeljc
priority: normal
severity: normal
status: open
title: dis module incorrectly handles EXTENDED_ARG + NOP sequence
type: behavior
versions: Python 3.10, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com