shithub: rgbds

Download patch

ref: b8d5dd18240cd12ed3cb5294a8c2c239d13fb145
parent: 88b66f2941ccb25d83c346f10c86d2a2eb5c9eeb
parent: ca6149abcfd089393388ec7b83029d98b82966fc
author: Antonio Niño Díaz <[email protected]>
date: Sat Aug 17 12:09:09 EDT 2019 

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