Issue information

Issue ID
#8566
Status
New
Severity
None
Started
memoryss
Mar 11, 2015 21:07
Last Post
memoryss
Mar 12, 2015 14:38
Confirmation
N/A

memoryss - Mar 11, 2015 21:07

I discover this while doing some bitwise calculation e.g. popcount.
And this issue makes the following script doesn't work properly.[code=auto:0] function script popcount { .@i = getarg(0); .@i -= (.@i >> 1) & 0x55555555; .@i = (.@i & 0x33333333) + ((.@i >> 2) & 0x33333333); .@i = (.@i + (.@i >> 4)) & 0x0F0F0F0F; .@i += .@i >> 8; .@i += .@i >> 16; return .@i & 0x3f; } [/code]

This post has been edited by memoryss on Mar 11, 2015 21:08

Haru - Mar 11, 2015 21:59

Could you provide an input value for which the script doesn't work correctly? (and possibly, the expected output value as well)

memoryss - Mar 11, 2015 22:05

input : -2147483648
expected output : 1

[Warning]: script:op_2num: underflow detected op=C_SUB i1=-2147483648 i2=1073741
824
var being capped to -2147483647, and output becomes 2 but not the expected 1[code=auto:0] - script TEST 952,{ OnInit: debugmes ""+callfunc("popcount", 1<<31); //expected 1 end; } [/code]

This post has been edited by memoryss on Mar 11, 2015 22:32

memoryss - Mar 11, 2015 22:12

an easy way to reproduce this issue
[Warning]: script error in file 'npc/dev/test.txt' line 826 column 11
&nbsp;&nbsp;&nbsp; parse_simpleexpr: overflow detected, capping value to INT_MAX[code=auto:0] - script TEST 952,{ OnInit: .@i = 0x80000000; //-2147483648 } [/code]
0x80000000 or -2147483648 should be equal to INT_MAX
and shouldn't cause overflow

BTW, thanks for Hura's prompt response.

This post has been edited by memoryss on Mar 11, 2015 22:36

memoryss - Mar 11, 2015 23:00

Another example[code=auto:0] prontera,100,100,6 script luckydraw 952,{ mes "Grab a ball" next; .@i |= 1 << rand(31); if( !.@ ) { //never occur mes "You grab nothing?" close; } else if( callfunc("popcount", .@i) > 1 ) { //when we grab ball 31 mes "No cheating! Only grab 1!"; close; } mes "Nice prize right?"; close; } [/code]

This post has been edited by memoryss on Mar 11, 2015 23:01

Haru - Mar 11, 2015 23:06

But 0x80000000 is NOT the same as -2147483648

0x80000000 is a positive value (you're thinking about numeric representation in two's complement machines, but the script engine here doesn't expose the internal representation of numbers)

Assigning 0x80000000 (or any number larger than 0x7fffffff) to a script variable is an illegal operation, as the range is [-2147483648; 2147483647] inclusive. As such, the variable values are clamped to that range on any assignment.

Likewise, the operation 1<<31 goes beyond the valid range. It returns -2147483648 by pure coincidence. It could be any value, since an overflow happened, and we don't guarantee predictable overflow results.

On a side note, even the C language specification, don't guarantee any predictability in overflows of signed integer variables (only on unsigned variables). Probably all current compilers show the same behavior, but it's undocumented, and it may change at any time (any program that relies on that, is buggy by definition)


The actual issue here is that the left shift operation isn't triggering an overflow warning like it should.

memoryss - Mar 12, 2015 14:38

[code=auto:0] void op_2num(struct script_state* st, int op, int i1, int i2) { int ret; int64 x; char overflow; switch( op ) { case C_AND: ret = i1 & i2; break; case C_OR: ret = i1 | i2; break; case C_XOR: ret = i1 ^ i2; break; case C_LAND: ret = (i1 && i2); break; case C_LOR: ret = (i1 || i2); break; case C_EQ: ret = (i1 == i2); break; case C_NE: ret = (i1 != i2); break; case C_GT: ret = (i1 > i2); break; case C_GE: ret = (i1 >= i2); break; case C_LT: ret = (i1 < i2); break; case C_LE: ret = (i1 <= i2); break; case C_R_SHIFT: ret = i1>>i2; break; case C_L_SHIFT: ret = i1<<i2; break; case C_DIV: case C_MOD: if( i2 == 0 ) { ShowError("script:op_2num: division by zero detected op=%s i1=%d i2=%d\n", script->op2name(op), i1, i2); script->reportsrc(st); script_pushnil(st); st->state = END; return; } else if( op == C_DIV ) ret = i1 / i2; else//if( op == C_MOD ) ret = i1 % i2; break; default: switch( op ) {// operators that can overflow/underflow case C_ADD: ret = i1 + i2; overflow = i2 < 1 ? ((INT_MIN - i2 <= i1) ? 0 : 2) : ((INT_MAX - i2 >= i1) ? 0 : 1); break; case C_SUB: ret = i1 - i2; overflow = i2 < 1 ? ((INT_MAX + i2 >= i1) ? 0 : 1) : ((INT_MIN + i2 <= i1) ? 0 : 2); break; case C_MUL: ret = i1 * i2; x = (int64)i1 * i2; overflow = ((x >> 31) && (~x >> 31)) << (x < 0); break; default: ShowError("script:op_2num: unexpected number operator %s i1=%d i2=%d\n", script->op2name(op), i1, i2); script->reportsrc(st); script_pushnil(st); return; } if( overflow & 2 ) { ShowWarning("script:op_2num: underflow detected op=%s i1=%d i2=%d\n", script->op2name(op), i1, i2); script->reportsrc(st); ret = INT_MIN; } else if( overflow & 1 ) { ShowWarning("script:op_2num: overflow detected op=%s i1=%d i2=%d\n", script->op2name(op), i1, i2); script->reportsrc(st); ret = INT_MAX; } } script_pushint(st, ret); } [/code]