frozenset not unpickled correctly

[Gouvernathor]

Last version of unrpyc Last (nightly) version of renpy Innocent Witches (yes, I know, but the issue mayuncover something more serious)

Trying to decompile this file (remove the .txt extension ofc), it fails displayables.rpyc.txt When using the simple version, it raises the following error :

Traceback (most recent call last):
  File "C:\Users\Armise\Desktop\git temp\unrpyc\unrpyc.py", line 241, in worker
    tag_outside_block=args.tag_outside_block, init_offset=args.init_offset, try_harder=args.try_harder)
  File "C:\Users\Armise\Desktop\git temp\unrpyc\unrpyc.py", line 204, in decompile_rpyc
    ast = read_ast_from_file(in_file)
  File "C:\Users\Armise\Desktop\git temp\unrpyc\unrpyc.py", line 177, in read_ast_from_file
    data, stmts = magic.safe_loads(raw_contents, class_factory, {"_ast", "collections"})
  File "C:\Users\Armise\Desktop\git temp\unrpyc\decompiler\magic.py", line 599, in safe_loads
    encoding=encoding, errors=errors).load()
  File "C:\Program Files\Python27\lib\pickle.py", line 864, in load
    dispatch[key](self)
  File "C:\Program Files\Python27\lib\pickle.py", line 1139, in load_reduce
    value = func(*args)
  File "C:\Users\Armise\Desktop\git temp\unrpyc\decompiler\magic.py", line 113, in __new__
    raise FakeUnpicklingError("{0} was instantiated with unexpected arguments {1}, {2}".format(cls, args, kwargs))
FakeUnpicklingError: <class 'frozenset'> was instantiated with unexpected arguments ([u'panties'],), {}

Decompilation of 1 file failed

And what's interesting (?) is that : 1) it doesn't show this error when --try-harder, 2) frozenset is a builtin python type and 3) frozenset(['somestring']) is valid python !

Is it possible, since that type is also not popular, that the type has just been forgotten about and that no decompiled renpy game used it before ?

[Gouvernathor]

Actually, the unpickling works when just editing https://github.com/Gouvernathor/unrpyc/blob/25f4470ea1612ecacec578efcecc3054a59098c8/unrpyc.py#L150 Turning the FakeStrict into a FakeWarning. I'm not closing the issue because I had to edit the file to make it work, and also becausea warning about a built-in type should arguably never happen, so there's got to be a deeper problem. Anyway, the game boots just fine afterwards.

[georgiehen]

@Gouvernathor I downloaded your update for unrpyc, put it in my own local program, but still couldn't decompile displayables.rpyc. Then I tried downloading your entire repo for unrpyc, but it still didn't work. I'm still getting the same error you started with. Any advice?

[Gouvernathor]

Look at my second message : on the line I linked, at the very end, change FakeStrict into FakeWarning

[georgiehen]

@Gouvernathor THANK YOU!

[CensoredUsername]

@Gouvernathor so basically frozenset never ends up in .rpyc files as the normal ren'py ast doesn't contain any frozensets. However this game adds some ast nodes at runtime which then include all kind of Revertable datastructures that normally should never end up in the AST. In fact, renpy normally changes all uses of a frozenset by RevertableSet so I'm kinda curious what's ending up in the actual AST.

[Gouvernathor]

Maybe I didn't understand what you're saying, but doesn't this show that renpy actually uses the real frozenset, and not the revertableset ? (in the console of the last stable release)

EDIT : This seems even more improbable (unless, again, I misunderstood), because frozensets have no special recognition in ast, since there are no frozenset literals (as opposed to list, dict, tuple and set literals). As with a set(...) call, a frozenset(...) is ast-ed as a function call, as shown below (from py 3.9, admittedly) : So renpy can't possibly change frozensets using ast (which it does use, for literals of the above-mentioned types, and to make class-definitions inherit the RevertableObject). And if it did, it would show up in the console.

Now, the other way renpy does such things is by shadowing the name of the callables, but as shown in my first screen and below (and by checking in the code), frozenset is not shadowed, since like tuple it is immutable and doesn't need to be revertable. So, the problem may very well come from SadCrab's shenanigans, but I really think you're wrong about the renpy part.

[CensoredUsername]

huh, guess it's just inconsistent then, I was going off code in renpy.python where it seems to change both sets and frozensets into RevertableSets when returned by methods on set, but it might just be much simpler then. Since frozenset isn't an abusable data type the problem should just be solvable by allowing it to be loaded during the safe unpickling process.

[Gouvernathor]

Indeed, I didn't see that line. But that line of code triggers only when calling operators between sets and other data, and when the data which would be returned is a frozenset. I doubt it's correct to turn an immutable in a mutable, so that line may disappear in a nearby nightly 🙃, but all in all I don't think this is related to the problem, given that the error seems to trigger when the constructor is called.

EDIT : after checking python's doc, "Binary operations that mix set instances with frozenset return the type of the first operand. For example: frozenset('ab') | set('bc') returns an instance of frozenset." So that instancecheck is never actually confronted with a frozenset instance, only with pure sets.

[CensoredUsername]

Decided to dig into this fully to figure out what the hell this game is doing. So first of all:

It uses a lot of custom UserStatement nodes. And each node contains a reference to the full Lexer context. This makes AST dumps very fun to browse (200MB AST dumps yay..).

It doesn't use the special handling for frozenset that python pickle normally does (this might be a protocol thing, not sure when it was added to the pickle protocol).

And the frozenset somehow ends up in ren'py's AST through the parse output of the UserStatement (I'm not exactly sure where though? I see it in the pickle dump but not in the actual AST. It might just be used to construct a wrapper class).

Anyway I added a fix to master so it loads anything containing a frozenset properly as it's a bit weird it doesn't currently. It's a single word change anyway.