shithub: rgbds

Info  •  Files  •  Log  •  Branches

Merge pull request #366 from dbrotz/fix-313

Fix signed integer overflow issues

--- a/src/asm/constexpr.c

+++ b/src/asm/constexpr.c

@@ -66,7 +66,7 @@

 		result = value;

 		break;

 	case T_OP_SUB:

-		result = -value;

+		result = -(uint32_t)value;

 		break;

 	case T_OP_NOT:

 		result = ~value;

@@ -155,10 +155,10 @@

 			result = value1 != value2;

 			break;

 		case T_OP_ADD:

-			result = value1 + value2;

+			result = (uint32_t)value1 + (uint32_t)value2;

 			break;

 		case T_OP_SUB:

-			result = value1 - value2;

+			result = (uint32_t)value1 - (uint32_t)value2;

 			break;

 		case T_OP_XOR:

 			result = value1 ^ value2;

@@ -181,7 +181,7 @@

 				fatalerror("Shift by too big value: %d",

 					   value2);

-			result = value1 << value2;

+			result = (uint32_t)value1 << value2;

 			break;

 		case T_OP_SHR:

 			if (value2 < 0)

@@ -194,17 +194,25 @@

 			result = value1 >> value2;

 			break;

 		case T_OP_MUL:

-			result = value1 * value2;

+			result = (uint32_t)value1 * (uint32_t)value2;

 			break;

 		case T_OP_DIV:

 			if (value2 == 0)

 				fatalerror("Division by zero");

-			result = value1 / value2;

+			if (value1 == INT32_MIN && value2 == -1) {

+				warning("Division of min value by -1");

+				result = INT32_MIN;

+			} else {

+				result = value1 / value2;

+			}

 			break;

 		case T_OP_MOD:

 			if (value2 == 0)

 				fatalerror("Division by zero");

-			result = value1 % value2;

+			if (value1 == INT32_MIN && value2 == -1)

+				result = 0;

+			else

+				result = value1 % value2;

 			break;

 		case T_OP_FDIV:

 			result = math_Div(value1, value2);

--- a/src/asm/globlex.c

+++ b/src/asm/globlex.c

@@ -71,8 +71,9 @@

 static int32_t ascii2bin(char *s)

 {

-	int32_t radix = 10;

-	int32_t result = 0;

+	char *start = s;

+	uint32_t radix = 10;

+	uint32_t result = 0;

 	x2bin convertfunc = char2bin;

 	switch (*s) {

@@ -101,6 +102,9 @@

 		break;

 	}

+	const uint32_t max_q = UINT32_MAX / radix;

+	const uint32_t max_r = UINT32_MAX % radix;

+

 	if (*s == '\0') {

 		/*

 		 * There are no digits after the radix prefix

@@ -108,15 +112,39 @@

 		 */

 		yyerror("Invalid integer constant");

 	} else if (radix == 4) {

+		int32_t size = 0;

 		int32_t c;

 		while (*s != '\0') {

 			c = convertfunc(*s++);

 			result = result * 2 + ((c & 2) << 7) + (c & 1);

+			size++;

 		}

+

+		/*

+		 * Extending a graphics constant longer than 8 pixels,

+		 * the Game Boy tile width, produces a nonsensical result.

+		 */

+		if (size > 8) {

+			warning("Graphics constant '%s' is too long",

+				start);

+		}

 	} else {

-		while (*s != '\0')

-			result = result * radix + convertfunc(*s++);

+		bool overflow = false;

+

+		while (*s != '\0') {

+			int32_t digit = convertfunc(*s++);

+

+			if (result > max_q

+			 || (result == max_q && digit > max_r)) {

+				overflow = true;

+			}

+			result = result * radix + digit;

+		}

+

+		if (overflow)

+			warning("Integer constant '%s' is too large",

+				start);

 	}

 	return result;

--- a/src/asm/rpn.c

+++ b/src/asm/rpn.c

@@ -347,7 +347,7 @@

 	     const struct Expression *src2)

 {

 	joinexpr();

-	expr->nVal = (src1->nVal + src2->nVal);

+	expr->nVal = ((uint32_t)src1->nVal + (uint32_t)src2->nVal);

 	pushbyte(expr, RPN_ADD);

 	expr->nRPNPatchSize++;

 }

@@ -356,7 +356,7 @@

 	     const struct Expression *src2)

 {

 	joinexpr();

-	expr->nVal = (src1->nVal - src2->nVal);

+	expr->nVal = ((uint32_t)src1->nVal - (uint32_t)src2->nVal);

 	pushbyte(expr, RPN_SUB);

 	expr->nRPNPatchSize++;

 }

@@ -393,15 +393,18 @@

 {

 	joinexpr();

-	if (src1->nVal < 0)

-		warning("Left shift of negative value: %d", src1->nVal);

+	if (!expr->isReloc) {

+		if (src1->nVal < 0)

+			warning("Left shift of negative value: %d", src1->nVal);

-	if (src2->nVal < 0)

-		fatalerror("Shift by negative value: %d", src2->nVal);

-	else if (src2->nVal >= 32)

-		fatalerror("Shift by too big value: %d", src2->nVal);

+		if (src2->nVal < 0)

+			fatalerror("Shift by negative value: %d", src2->nVal);

+		else if (src2->nVal >= 32)

+			fatalerror("Shift by too big value: %d", src2->nVal);

-	expr->nVal = (src1->nVal << src2->nVal);

+		expr->nVal = ((uint32_t)src1->nVal << src2->nVal);

+	}

+

 	pushbyte(expr, RPN_SHL);

 	expr->nRPNPatchSize++;

 }

@@ -410,12 +413,16 @@

 	     const struct Expression *src2)

 {

 	joinexpr();

-	if (src2->nVal < 0)

-		fatalerror("Shift by negative value: %d", src2->nVal);

-	else if (src2->nVal >= 32)

-		fatalerror("Shift by too big value: %d", src2->nVal);

-	expr->nVal = (src1->nVal >> src2->nVal);

+	if (!expr->isReloc) {

+		if (src2->nVal < 0)

+			fatalerror("Shift by negative value: %d", src2->nVal);

+		else if (src2->nVal >= 32)

+			fatalerror("Shift by too big value: %d", src2->nVal);

+

+		expr->nVal = (src1->nVal >> src2->nVal);

+	}

+

 	pushbyte(expr, RPN_SHR);

 	expr->nRPNPatchSize++;

 }

@@ -424,7 +431,7 @@

 	     const struct Expression *src2)

 {

 	joinexpr();

-	expr->nVal = (src1->nVal * src2->nVal);

+	expr->nVal = ((uint32_t)src1->nVal * (uint32_t)src2->nVal);

 	pushbyte(expr, RPN_MUL);

 	expr->nRPNPatchSize++;

 }

@@ -433,10 +440,19 @@

 	     const struct Expression *src2)

 {

 	joinexpr();

-	if (src2->nVal == 0)

-		fatalerror("Division by zero");

-	expr->nVal = (src1->nVal / src2->nVal);

+	if (!expr->isReloc) {

+		if (src2->nVal == 0)

+			fatalerror("Division by zero");

+

+		if (src1->nVal == INT32_MIN && src2->nVal == -1) {

+			warning("Division of min value by -1");

+			expr->nVal = INT32_MIN;

+		} else {

+			expr->nVal = (src1->nVal / src2->nVal);

+		}

+	}

+

 	pushbyte(expr, RPN_DIV);

 	expr->nRPNPatchSize++;

 }

@@ -445,10 +461,17 @@

 	     const struct Expression *src2)

 {

 	joinexpr();

-	if (src2->nVal == 0)

-		fatalerror("Division by zero");

-	expr->nVal = (src1->nVal % src2->nVal);

+	if (!expr->isReloc) {

+		if (src2->nVal == 0)

+			fatalerror("Division by zero");

+

+		if (src1->nVal == INT32_MIN && src2->nVal == -1)

+			expr->nVal = 0;

+		else

+			expr->nVal = (src1->nVal % src2->nVal);

+	}

+

 	pushbyte(expr, RPN_MOD);

 	expr->nRPNPatchSize++;

 }

@@ -456,7 +479,7 @@

 void rpn_UNNEG(struct Expression *expr, const struct Expression *src)

 {

 	*expr = *src;

-	expr->nVal = -expr->nVal;

+	expr->nVal = -(uint32_t)expr->nVal;

 	pushbyte(expr, RPN_UNSUB);

 	expr->nRPNPatchSize++;

 }

--- /dev/null

+++ b/test/asm/overflow.asm

@@ -1,0 +1,42 @@

+SECTION "sec", ROM0

+

+print_x: MACRO

+	printv x

+	printt "\n"

+ENDM

+

+x = 2147483647

+x = x + 1

+	dl 2147483647+1

+	print_x

+

+x = -2147483648

+x = x - 1

+	dl -2147483648-1

+	print_x

+

+x = -2147483648

+x = x * -1

+	dl -2147483648 * -1

+	print_x

+

+x = -2147483648

+x = x / -1

+	dl -2147483648 / -1

+	print_x

+

+x = -2147483648

+x = x % -1

+	dl -2147483648 % -1

+	print_x

+

+x = -1

+x = x << 1

+	dl -1 << 1

+	print_x

+

+x = 4294967295

+x = 4294967296

+

+x = `33333333

+x = `333333333

--- /dev/null

+++ b/test/asm/overflow.out

@@ -1,0 +1,18 @@

+warning: overflow.asm(24):

+    Division of min value by -1

+warning: overflow.asm(25):

+    Division of min value by -1

+warning: overflow.asm(34):

+    Left shift of negative value: -1

+warning: overflow.asm(35):

+    Left shift of negative value: -1

+warning: overflow.asm(39):

+    Integer constant '4294967296' is too large

+warning: overflow.asm(42):

+    Graphics constant '`333333333' is too long

+$80000000

+$7FFFFFFF

+$80000000

+$80000000

+$0

+$FFFFFFFE

--- /dev/null

+++ b/test/asm/overflow.out.pipe

@@ -1,0 +1,18 @@

+warning: -(24):

+    Division of min value by -1

+warning: -(25):

+    Division of min value by -1

+warning: -(34):

+    Left shift of negative value: -1

+warning: -(35):

+    Left shift of negative value: -1

+warning: -(39):

+    Integer constant '4294967296' is too large

+warning: -(42):

+    Graphics constant '`333333333' is too long

+$80000000

+$7FFFFFFF

+$80000000

+$80000000

+$0

+$FFFFFFFE