Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deterministic set order in LOAD_CONST #491

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

olii
Copy link

@olii olii commented Mar 18, 2024

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
Copy link
Owner

rocky commented Mar 18, 2024

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
Copy link
Owner

rocky commented Mar 20, 2024

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
Copy link
Author

olii commented Mar 20, 2024

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
Copy link
Owner

rocky commented Mar 20, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants