Check for overflow in fixed-point precision suffix (#1918)

Fixes #1917

src/asm/lexer.cpp

@@ -20,6 +20,7 @@
 #include <string.h>
 #include <string>
 #include <string_view>
+#include <tuple>
 #include <unordered_map>
 #include <utility>
 #include <variant>
@@ -951,7 +952,8 @@ static void
 	}
 }
 
-static bool readFractionDigits(uint32_t &dividend, uint32_t &divisor) {
+static std::tuple<uint32_t, uint32_t, bool> readFractionDigits() {
+	uint32_t dividend = 0, divisor = 1;
 	bool prevWasSeparator = false;
 
 	int c = peek();
@@ -966,11 +968,13 @@ static bool readFractionDigits(uint32_t &dividend, uint32_t &divisor) {
 		} else if (isDigit(c)) {
 			prevWasSeparator = false;
 			c -= '0';
-			if (divisor > (UINT32_MAX - c) / 10) {
+			if (dividend > (UINT32_MAX - c) / 10 || divisor > UINT32_MAX / 10) {
-				warning(WARNING_LARGE_CONSTANT, "Precision of fixed-point constant is too large");
+				warning(
+				    WARNING_LARGE_CONSTANT, "Fixed-point constant has too many fractional digits"
+				);
 				// Discard any additional digits
 				for (int d = peek(); isDigit(d) || d == '_'; c = d, d = nextChar()) {}
-				return c == '_';
+				return {dividend, divisor, c == '_'};
 			}
 			dividend = dividend * 10 + c;
 			divisor *= 10;
@@ -979,7 +983,7 @@ static bool readFractionDigits(uint32_t &dividend, uint32_t &divisor) {
 		}
 	}
 
-	return prevWasSeparator;
+	return {dividend, divisor, prevWasSeparator};
 }
 
 static uint8_t readPrecisionSuffix() {
@@ -993,7 +997,15 @@ static uint8_t readPrecisionSuffix() {
 	// '_' is not allowed after 'q'/'Q'
 	for (int c = peek(); isDigit(c); c = nextChar()) {
 		empty = false;
-		precision = precision * 10 + (c - '0');
+		c -= '0';
+		if (precision > (UINT8_MAX - c) / 10) {
+			// Discard any additional digits
+			skipChars(isDigit);
+			// Return an invalid precision to cause a subsequent error, which is checked afterwards
+			// to cover the default `options.fixPrecision` as well, just in case
+			return UINT8_MAX;
+		}
+		precision = precision * 10 + c;
 	}
 
 	if (empty) {
@@ -1005,10 +1017,8 @@ static uint8_t readPrecisionSuffix() {
 }
 
 static uint32_t finishReadingFixedPoint(uint32_t integer) {
-	uint32_t dividend = 0, divisor = 1;
+	auto [dividend, divisor, prevWasSeparator] = readFractionDigits();
 	uint8_t precision = options.fixPrecision;
-
-	bool prevWasSeparator = readFractionDigits(dividend, divisor);
 	if (int c = peek(); c == 'q' || c == 'Q') {
 		// '_' is allowed before 'q'/'Q', so do not call `checkDigitsEnding`
 		shiftChar();
@@ -1248,7 +1258,8 @@ static uint32_t readGfxConstant() {
 	checkDigitsEnding(width == 0, "'`'", prevWasSeparator, "graphics");
 	if (width == 9) {
 		warning(
-		    WARNING_LARGE_CONSTANT, "Graphics constant is too large; only first 8 pixels considered"
+		    WARNING_LARGE_CONSTANT,
+		    "Graphics constant has too many digits; only first 8 pixels considered"
 		);
 	}
 

test/asm/fixed-point-precision.asm

@@ -21,3 +21,5 @@ def q24 = 1.25q.24
 
 def qerr = 1.25q32
 	println "{q0f:qerr}"
+def qerr = 1.25q264
+	println "{q999f:qerr}"

test/asm/fixed-point-precision.err

@@ -2,4 +2,8 @@ error: Fixed-point constant precision must be between 1 and 31
     at fixed-point-precision.asm(22)
 error: Fixed-point constant precision 0 invalid, defaulting to 16
     at fixed-point-precision.asm(23)
-Assembly aborted with 2 errors
+error: Fixed-point constant precision must be between 1 and 31
+    at fixed-point-precision.asm(24)
+error: Fixed-point constant precision 999 invalid, defaulting to 16
+    at fixed-point-precision.asm(25)
+Assembly aborted with 4 errors

test/asm/fixed-point-precision.out

@@ -7,3 +7,4 @@
 0.00488q16 1.25000q16 320.00000q16
 Q8 $140 Q16 $14000 Q24 $1400000
 1.25000
+1.25000

test/asm/fixed-point-syntax.asm

@@ -6,6 +6,12 @@ println 12.34_q.5
 println 1_2.3_4_q15
 println 1.q2
 
+; warning
+println 1.000_000_000_000_001
+println 1.999_999_999_999_999
+println 1.000_000_000_000_001q16
+println 1.999_999_999_999_999q.16
+
 ; bad
 println 12.34q0
 println 12.34q_15 ; lexes as `12.34q` (invalid) then symbol `_15`
@@ -14,6 +20,3 @@ println 1_.2
 println 1._2
 println 1.__2
 println 1.2q
-println 1.999_999_999_999_999
-println 1.999_999_999_999_999q16
-println 1.999_999_999_999_999q.16

test/asm/fixed-point-syntax.err

@@ -1,25 +1,27 @@
-error: Fixed-point constant precision must be between 1 and 31
+warning: Fixed-point constant has too many fractional digits [-Wlarge-constant]
     at fixed-point-syntax.asm(10)
-error: Invalid fixed-point constant, no digits after 'q'
+warning: Fixed-point constant has too many fractional digits [-Wlarge-constant]
     at fixed-point-syntax.asm(11)
-error: syntax error, unexpected symbol
+warning: Fixed-point constant has too many fractional digits [-Wlarge-constant]
-    at fixed-point-syntax.asm(11)
-error: syntax error, unexpected symbol
     at fixed-point-syntax.asm(12)
-error: Invalid integer constant, trailing '_'
+warning: Fixed-point constant has too many fractional digits [-Wlarge-constant]
     at fixed-point-syntax.asm(13)
-error: Invalid fixed-point constant, '_' after '.'
+error: Fixed-point constant precision must be between 1 and 31
-    at fixed-point-syntax.asm(14)
-error: Invalid fixed-point constant, '_' after '.'
-    at fixed-point-syntax.asm(15)
-error: Invalid fixed-point constant, '_' after another '_'
-    at fixed-point-syntax.asm(15)
-error: Invalid fixed-point constant, no digits after 'q'
     at fixed-point-syntax.asm(16)
-warning: Precision of fixed-point constant is too large [-Wlarge-constant]
+error: Invalid fixed-point constant, no digits after 'q'
     at fixed-point-syntax.asm(17)
-warning: Precision of fixed-point constant is too large [-Wlarge-constant]
+error: syntax error, unexpected symbol
+    at fixed-point-syntax.asm(17)
+error: syntax error, unexpected symbol
     at fixed-point-syntax.asm(18)
-warning: Precision of fixed-point constant is too large [-Wlarge-constant]
+error: Invalid integer constant, trailing '_'
     at fixed-point-syntax.asm(19)
+error: Invalid fixed-point constant, '_' after '.'
+    at fixed-point-syntax.asm(20)
+error: Invalid fixed-point constant, '_' after '.'
+    at fixed-point-syntax.asm(21)
+error: Invalid fixed-point constant, '_' after another '_'
+    at fixed-point-syntax.asm(21)
+error: Invalid fixed-point constant, no digits after 'q'
+    at fixed-point-syntax.asm(22)
 Assembly aborted with 9 errors

test/asm/fixed-point-syntax.out

@@ -4,11 +4,12 @@ $4D2915B
 $18B
 $62B85
 $4
+$10000
+$10000
+$10000
+$10000
 $C570A
 $13333
 $13333
 $13333
 $13333
-$10000
-$10000
-$10000

test/asm/interpolation-overflow.err

@@ -1,6 +1,6 @@
 error: Fractional width 99999999 too long, limiting to 255
     at interpolation-overflow.asm(4)
-warning: Precision of fixed-point constant is too large [-Wlarge-constant]
+warning: Fixed-point constant has too many fractional digits [-Wlarge-constant]
     at interpolation-overflow.asm(4)
     while expanding symbol `x`
 error: `\1` cannot be used outside of a macro

test/asm/invalid-numbers.err

@@ -16,7 +16,7 @@ warning: Integer constant is too large [-Wlarge-constant]
     at invalid-numbers.asm::try(2) <- invalid-numbers.asm(20)
 warning: Integer constant is too large [-Wlarge-constant]
     at invalid-numbers.asm::try(2) <- invalid-numbers.asm(21)
-warning: Graphics constant is too large; only first 8 pixels considered [-Wlarge-constant]
+warning: Graphics constant has too many digits; only first 8 pixels considered [-Wlarge-constant]
     at invalid-numbers.asm::try(2) <- invalid-numbers.asm(22)
 warning: Magnitude of fixed-point constant is too large [-Wlarge-constant]
     at invalid-numbers.asm::try(2) <- invalid-numbers.asm(23)

test/asm/overflow.err

@@ -4,5 +4,5 @@ warning: Division of -2147483648 by -1 yields -2147483648 [-Wdiv]
     at overflow.asm(24)
 warning: Integer constant is too large [-Wlarge-constant]
     at overflow.asm(44)
-warning: Graphics constant is too large; only first 8 pixels considered [-Wlarge-constant]
+warning: Graphics constant has too many digits; only first 8 pixels considered [-Wlarge-constant]
     at overflow.asm(47)