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

fails to load very big constant numbers #38

Closed
thautwarm opened this issue Oct 15, 2018 · 11 comments · Fixed by #44
Closed

fails to load very big constant numbers #38

thautwarm opened this issue Oct 15, 2018 · 11 comments · Fixed by #44

Comments

@thautwarm
Copy link
Contributor

thautwarm commented Oct 15, 2018

Hey, I come back again and ask for your help!

When I was loading very big numbers, I got an OverflowError:

bc = Bytecode([Instr('LOAD_CONST', 0xFFFFFFFF), Instr('RETURN_VALUE')])
bc.to_code()
>> OverflowError: Python int too large to convert to C long

Let's avoid performing dis.stack_effect when the instruction is something like LOAD_CONST?

@MatthieuDartiailh
Copy link
Owner

Thanks for the report. Could you make a PR adding LOAD_CONST to _stack_effects so that we avoid the offending call ? Add a test and an entry in the changlog.

@thautwarm
Copy link
Contributor Author

Very glad to do that. I'll make it done.

@serhiy-storchaka
Copy link
Contributor

I can't reproduce the error. What python version do you use?

@MatthieuDartiailh
Copy link
Owner

I did not have time to try to reproduce but the line 34 in _opcode.c seemed like a possible reason for the error:

oparg_int = (int)PyLong_AsLong(oparg);

@thautwarm
Copy link
Contributor Author

@serhiy-storchaka This 0xFFFFFFFF...

@thautwarm
Copy link
Contributor Author

And I'm using Python 3.6.5 in Windows 10 x86_64.

@thautwarm
Copy link
Contributor Author

@MatthieuDartiailh Yes, you're right! (int)PyLong_AsLong(oparg) causes this, but note that standard compiler could even compile 0xFFFFFFFFFF..

I've found the way to solve this problem, a few minutes.

@serhiy-storchaka
Copy link
Contributor

I am not sure that specifying a whitelist for opcodes whose stack effect depends on its argument is a good approach. The list is not complete. It lacks following opcodes: BUILD_LIST, BUILD_LIST_UNPACK, BUILD_MAP_UNPACK, BUILD_SET, BUILD_SET_UNPACK, BUILD_TUPLE, BUILD_TUPLE_UNPACK, BUILD_TUPLE_UNPACK_WITH_CALL, CALL_FUNCTION_VAR, CALL_FUNCTION_VAR_KW, MAKE_CLOSURE. Also in new Python versions new opcodes can be added, and the meaning of old opcodes can be changed.

Are there other opcodes that need a workaround besides LOAD_CONST?

@serhiy-storchaka
Copy link
Contributor

#41 addresses this.

@MatthieuDartiailh
Copy link
Owner

Thanks for the ping @serhiy-storchaka . Actually I probably was in too much of a rush when reviewing the #43.
Looking through Python opcode and our implementation, I believe you are correct that only LOAD_CONST can manipulate arbitrary objects. And limiting the workaround to this one seems safer. I will review your PR sometimes next week. I want in particular to understand the need for the modified tests.
Thanks again for catching this.

@serhiy-storchaka
Copy link
Contributor

I have extracted the fix for this issue into separate PR #44.

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 a pull request may close this issue.

3 participants