Merge pull request #366 from dbrotz/fix-313

Fix signed integer overflow issues

src/asm/constexpr.c

@@ -66,7 +66,7 @@ void constexpr_UnaryOp(struct ConstExpression *expr,
 		result = value;
 		break;
 	case T_OP_SUB:
-		result = -value;
+		result = -(uint32_t)value;
 		break;
 	case T_OP_NOT:
 		result = ~value;
@@ -155,10 +155,10 @@ void constexpr_BinaryOp(struct ConstExpression *expr,
 			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 @@ void constexpr_BinaryOp(struct ConstExpression *expr,
 				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,16 +194,24 @@ void constexpr_BinaryOp(struct ConstExpression *expr,
 			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");
+			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");
+			if (value1 == INT32_MIN && value2 == -1)
+				result = 0;
+			else
 				result = value1 % value2;
 			break;
 		case T_OP_FDIV:

src/asm/globlex.c

@@ -71,8 +71,9 @@ typedef int32_t(*x2bin) (char ch);
 
 static int32_t ascii2bin(char *s)
 {
-	int32_t radix = 10;
+	char *start = s;
-	int32_t result = 0;
+	uint32_t radix = 10;
+	uint32_t result = 0;
 	x2bin convertfunc = char2bin;
 
 	switch (*s) {
@@ -101,6 +102,9 @@ static int32_t ascii2bin(char *s)
 		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 @@ static int32_t ascii2bin(char *s)
 		 */
 		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')
+		bool overflow = false;
-			result = result * radix + convertfunc(*s++);
+
+		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;

src/asm/rpn.c

@@ -347,7 +347,7 @@ void rpn_ADD(struct Expression *expr, const struct Expression *src1,
 	     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 @@ void rpn_SUB(struct Expression *expr, const struct Expression *src1,
 	     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,6 +393,7 @@ void rpn_SHL(struct Expression *expr, const struct Expression *src1,
 {
 	joinexpr();
 
+	if (!expr->isReloc) {
 		if (src1->nVal < 0)
 			warning("Left shift of negative value: %d", src1->nVal);
 
@@ -401,7 +402,9 @@ void rpn_SHL(struct Expression *expr, const struct Expression *src1,
 		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 @@ void rpn_SHR(struct Expression *expr, const struct Expression *src1,
 	     const struct Expression *src2)
 {
 	joinexpr();
+
+	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 @@ void rpn_MUL(struct Expression *expr, const struct Expression *src1,
 	     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 @@ void rpn_DIV(struct Expression *expr, const struct Expression *src1,
 	     const struct Expression *src2)
 {
 	joinexpr();
+
+	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 @@ void rpn_MOD(struct Expression *expr, const struct Expression *src1,
 	     const struct Expression *src2)
 {
 	joinexpr();
+
+	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_MOD(struct Expression *expr, const struct Expression *src1,
 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++;
 }

test/asm/overflow.asm

@@ -0,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

test/asm/overflow.out

@@ -0,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

test/asm/overflow.out.pipe

@@ -0,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