fails to load very big constant numbers

Question

fails to load very big constant numbers

Closed this issue 6 years ago · 11 comments

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?

Answer 1 · 2018-10-16T02:36:03.000Z

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.

Answer 2 · 2018-10-16T06:31:57.000Z

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

Answer 3 · 2018-10-16T08:41:45.000Z

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

Answer 4 · 2018-10-16T13:12:05.000Z

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);

Answer 5 · 2018-10-16T13:55:19.000Z

@serhiy-storchaka This 0xFFFFFFFF...

Answer 6 · 2018-10-16T13:57:19.000Z

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

Answer 7 · 2018-10-16T15:37:49.000Z

@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.

Answer 8 · 2018-10-19T08:44:36.000Z

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?

Answer 9 · 2018-10-19T09:07:52.000Z

#41 addresses this.

Answer 10 · 2018-10-19T13:59:22.000Z

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.

Answer 11 · 2018-10-19T14:27:30.000Z

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