Jump to content

  •  

Bug Tracker Migration

June 3rd
Good news everyone! The staff has decided that it is time to slowly kill off this Bug Tracker. We will begin the process of slowly migrating from this Bug Tracker over to our Github Issues which can be found here: https://github.com/HerculesWS/Hercules/issues

Over the next couple of days, I will be closing off any opportunity to create new reports. However, I still will keep the opportunity to reply to existing Bug Reports. Doing this will allow us to slowly fix any bug reports we have listed here so that we can easily migrate over to our Issue Tracker.

Update - June 7th 2015: Creating new bug posts has been disabled. Please use our https://github.com/HerculesWS/Hercules/issues tracker to post bugs. Users are still able to reply to existing bug posts.

- Administration

Issue Information

  • #008566

  • 0 - None Assigned

  • New

Issue Confirmations

  • Yes (0)No (0)
Photo

script variables can't reach INT_MIN

Posted by memoryss on 11 March 2015 - 09:07 PM

I discover this while doing some bitwise calculation e.g. popcount.
And this issue makes the following script doesn't work properly.
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;
}

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

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
-	script	TEST	952,{
OnInit:
	debugmes ""+callfunc("popcount", 1<<31); //expected 1
	end;
}

Edited by memoryss, 11 March 2015 - 10:32 PM.


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
-    script    TEST    952,{
OnInit:
    .@i = 0x80000000; //-2147483648
}
0x80000000 or -2147483648 should be equal to INT_MAX
and shouldn't cause overflow

BTW, thanks for Hura's prompt response.

Edited by memoryss, 11 March 2015 - 10:36 PM.


Another example
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;
}

Edited by memoryss, 11 March 2015 - 11:01 PM.


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.

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