rocky / python-uncompyle6

A cross-version Python bytecode decompiler
GNU General Public License v3.0
3.74k stars 408 forks source link

Deterministic set order in LOAD_CONST #491

Open olii opened 5 months ago

olii commented 5 months ago

This PR attempts to fix the decompilation of simple sets defined as LOAD_CONST. Sets were decompiled correctly but the order of elements was not defined.

Test input

This was my test input compiled by the Python 3.6:

if v in {'element', 5, 'attlist', 'linktype', 'link'}:
    print(v)

Disassembly


 L.   1         0  LOAD_NAME                v
                2  LOAD_CONST               {'attlist', 5, 'linktype', 'link', 'element'}
                4  COMPARE_OP               in
                6  POP_JUMP_IF_FALSE    16  'to 16'

 L.   2         8  LOAD_NAME                print
               10  LOAD_NAME                v
               12  CALL_FUNCTION         1  '1 positional argument'
               14  POP_TOP          
             16_0  COME_FROM             6  '6'
               16  LOAD_CONST               None
               18  RETURN_VALUE  

Decompilation outputs before patch

Attempt n. 1:

if v in {5, 'attlist', 'linktype', 'link', 'element'}:
    print(v)

Attempt n. 2:

if v in {'element', 5, 'linktype', 'link', 'attlist'}:
    print(v)

Decompilation output after patch

if v in {'attlist', 'element', 'link', 'linktype', 5}:
    print(v)
rocky commented 5 months ago

This may improve this situation but make other situations worse. So it may be that tests on both the bytecode version and Python version may need to be done.

Also, we might want to jigger things in xdis rather than here.

Let me think about this one.

rocky commented 5 months ago

In thinking more about this, I think the change is more appropriately made in xdis, not here.

The order that set items should appear is the order in which they items appear in marshal data.

Since Python 3.1, there is OrderedDict. And that is what should be used for those versions of Python that support this.

olii commented 5 months ago

The order that set items should appear is the order in which they items appear in marshal data.

I agree, however, is there a way to parse the value in xdis and store it like some other type, in this case OrderedDict? Is there anything similar already?

rocky commented 5 months ago

The order that set items should appear is the order in which they items appear in marshal data.

I agree, however, is there a way to parse the value in xdis and store it like some other type, in this case OrderedDict? Is there anything similar already?

Yes, this would all be done inside xdis.