Restrict custom binary and graphics digits (#1693)

* Restrict custom binary and graphics digits

* Update documentation

* Fix build error

include/asm/lexer.hpp

@@ -118,17 +118,8 @@ struct LexerState {
 extern char binDigits[2];
 extern char gfxDigits[4];
 
-static inline void lexer_SetBinDigits(char const digits[2]) {
+void lexer_SetBinDigits(char const digits[2]);
-	binDigits[0] = digits[0];
+void lexer_SetGfxDigits(char const digits[4]);
-	binDigits[1] = digits[1];
-}
-
-static inline void lexer_SetGfxDigits(char const digits[4]) {
-	gfxDigits[0] = digits[0];
-	gfxDigits[1] = digits[1];
-	gfxDigits[2] = digits[2];
-	gfxDigits[3] = digits[3];
-}
 
 bool lexer_AtTopLevel();
 void lexer_RestartRept(uint32_t lineNo);

man/rgbasm.1

@@ -51,8 +51,19 @@ is invalid because it could also be
 The arguments are as follows:
 .Bl -tag -width Ds
 .It Fl b Ar chars , Fl \-binary-digits Ar chars
-Change the two characters used for binary constants.
+Allow two characters to be used for binary constants in addition to the default
-The defaults are 01.
+.Sq 0
+and
+.Sq 1 .
+Valid characters are numbers other than
+.Sq 0
+and
+.Sq 1 ,
+letters,
+.Sq \&. ,
+.Sq # ,
+or
+.Sq @ .
 .It Fl D Ar name Ns Oo = Ns Ar value Oc , Fl \-define Ar name Ns Oo = Ns Ar value Oc
 Add a string symbol to the compiled source code.
 This is equivalent to
@@ -65,7 +76,21 @@ is not specified.
 .It Fl E , Fl \-export-all
 Export all labels, including unreferenced and local labels.
 .It Fl g Ar chars , Fl \-gfx-chars Ar chars
-Change the four characters used for gfx constants.
+Allow four characters to be used for graphics constants in addition to the default
+.Sq 0 ,
+.Sq 1 ,
+.Sq 2 ,
+and
+.Sq 3 .
+Valid characters are numbers other than
+.Sq 0
+to
+.Sq 3 ,
+letters,
+.Sq \&. ,
+.Sq # ,
+or
+.Sq @ .
 The defaults are 0123.
 .It Fl h , Fl \-help
 Print help text for the program and exit.

src/asm/lexer.cpp

@@ -610,7 +610,7 @@ static bool isMacroChar(char c) {
 // forward declarations for readBracketedMacroArgNum
 static int peek();
 static void shiftChar();
-static uint32_t readNumber(int radix, uint32_t baseValue);
+static uint32_t readDecimalNumber(int initial);
 
 static uint32_t readBracketedMacroArgNum() {
 	bool disableMacroArgs = lexerState->disableMacroArgs;
@@ -634,7 +634,7 @@ static uint32_t readBracketedMacroArgNum() {
 	}
 
 	if (c >= '0' && c <= '9') {
-		uint32_t n = readNumber(10, 0);
+		uint32_t n = readDecimalNumber(0);
 		if (n > INT32_MAX) {
 			error("Number in bracketed macro argument is too large\n");
 			return 0;
@@ -1018,26 +1018,6 @@ static std::string readAnonLabelRef(char c) {
 	return sym_MakeAnonLabelName(n, c == '-');
 }
 
-static uint32_t readNumber(int radix, uint32_t baseValue) {
-	uint32_t value = baseValue;
-
-	for (;; shiftChar()) {
-		int c = peek();
-
-		if (c == '_') {
-			continue;
-		} else if (c < '0' || c > '0' + radix - 1) {
-			break;
-		}
-		if (value > (UINT32_MAX - (c - '0')) / radix) {
-			warning(WARNING_LARGE_CONSTANT, "Integer constant is too large\n");
-		}
-		value = value * radix + (c - '0');
-	}
-
-	return value;
-}
-
 static uint32_t readFractionalPart(uint32_t integer) {
 	uint32_t value = 0, divisor = 1;
 	uint8_t precision = 0;
@@ -1103,21 +1083,64 @@ static uint32_t readFractionalPart(uint32_t integer) {
 }
 
 char binDigits[2];
+char gfxDigits[4];
+
+static bool isValidDigit(char c) {
+	return (c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '.'
+	       || c == '#' || c == '@';
+}
+
+static bool checkDigitErrors(char const *digits, size_t n, char const *type) {
+	for (size_t i = 0; i < n; i++) {
+		char c = digits[i];
+
+		if (!isValidDigit(c)) {
+			error("Invalid digit for %s constant %s\n", type, printChar(c));
+			return false;
+		}
+
+		if (c >= '0' && c < static_cast<char>(n + '0') && c != static_cast<char>(i + '0')) {
+			error("Changed digit for %s constant %s\n", type, printChar(c));
+			return false;
+		}
+
+		for (size_t j = i + 1; j < n; j++) {
+			if (c == digits[j]) {
+				error("Repeated digit for %s constant %s\n", type, printChar(c));
+				return false;
+			}
+		}
+	}
+
+	return true;
+}
+
+void lexer_SetBinDigits(char const digits[2]) {
+	if (size_t n = std::size(binDigits); checkDigitErrors(digits, n, "binary")) {
+		memcpy(binDigits, digits, n);
+	}
+}
+
+void lexer_SetGfxDigits(char const digits[4]) {
+	if (size_t n = std::size(gfxDigits); checkDigitErrors(digits, n, "graphics")) {
+		memcpy(gfxDigits, digits, n);
+	}
+}
 
 static uint32_t readBinaryNumber() {
 	uint32_t value = 0;
+	bool empty = true;
 
 	for (;; shiftChar()) {
 		int c = peek();
 		int bit;
 
-		// Check for '_' after digits in case one of the digits is '_'
+		if (c == '_' && !empty) {
-		if (c == binDigits[0]) {
-			bit = 0;
-		} else if (c == binDigits[1]) {
-			bit = 1;
-		} else if (c == '_') {
 			continue;
+		} else if (c == '0' || c == binDigits[0]) {
+			bit = 0;
+		} else if (c == '1' || c == binDigits[1]) {
+			bit = 1;
 		} else {
 			break;
 		}
@@ -1125,6 +1148,72 @@ static uint32_t readBinaryNumber() {
 			warning(WARNING_LARGE_CONSTANT, "Integer constant is too large\n");
 		}
 		value = value * 2 + bit;
+
+		empty = false;
+	}
+
+	if (empty) {
+		error("Invalid integer constant, no digits after '%%'\n");
+	}
+
+	return value;
+}
+
+static uint32_t readOctalNumber() {
+	uint32_t value = 0;
+	bool empty = true;
+
+	for (;; shiftChar()) {
+		int c = peek();
+
+		if (c == '_' && !empty) {
+			continue;
+		} else if (c >= '0' && c <= '7') {
+			c = c - '0';
+		} else {
+			break;
+		}
+
+		if (value > (UINT32_MAX - c) / 8) {
+			warning(WARNING_LARGE_CONSTANT, "Integer constant is too large\n");
+		}
+		value = value * 8 + c;
+
+		empty = false;
+	}
+
+	if (empty) {
+		error("Invalid integer constant, no digits after '&'\n");
+	}
+
+	return value;
+}
+
+static uint32_t readDecimalNumber(int initial) {
+	uint32_t value = initial ? initial - '0' : 0;
+	bool empty = !initial;
+
+	for (;; shiftChar()) {
+		int c = peek();
+
+		if (c == '_' && !empty) {
+			continue;
+		} else if (c >= '0' && c <= '9') {
+			c = c - '0';
+		} else {
+			break;
+		}
+
+		if (value > (UINT32_MAX - c) / 10) {
+			warning(WARNING_LARGE_CONSTANT, "Integer constant is too large\n");
+		}
+		value = value * 10 + c;
+
+		empty = false;
+	}
+
+	if (empty) {
+		error("Invalid integer constant, no digits\n");
 	}
 
 	return value;
@@ -1137,14 +1226,14 @@ static uint32_t readHexNumber() {
 	for (;; shiftChar()) {
 		int c = peek();
 
-		if (c >= 'a' && c <= 'f') {
+		if (c == '_' && !empty) {
+			continue;
+		} else if (c >= 'a' && c <= 'f') {
 			c = c - 'a' + 10;
 		} else if (c >= 'A' && c <= 'F') {
 			c = c - 'A' + 10;
 		} else if (c >= '0' && c <= '9') {
 			c = c - '0';
-		} else if (c == '_' && !empty) {
-			continue;
 		} else {
 			break;
 		}
@@ -1164,8 +1253,6 @@ static uint32_t readHexNumber() {
 	return value;
 }
 
-char gfxDigits[4];
-
 static uint32_t readGfxConstant() {
 	uint32_t bitPlaneLower = 0, bitPlaneUpper = 0;
 	uint8_t width = 0;
@@ -1174,17 +1261,16 @@ static uint32_t readGfxConstant() {
 		int c = peek();
 		uint32_t pixel;
 
-		// Check for '_' after digits in case one of the digits is '_'
+		if (c == '_' && width > 0) {
-		if (c == gfxDigits[0]) {
-			pixel = 0;
-		} else if (c == gfxDigits[1]) {
-			pixel = 1;
-		} else if (c == gfxDigits[2]) {
-			pixel = 2;
-		} else if (c == gfxDigits[3]) {
-			pixel = 3;
-		} else if (c == '_' && width > 0) {
 			continue;
+		} else if (c == '0' || c == gfxDigits[0]) {
+			pixel = 0;
+		} else if (c == '1' || c == gfxDigits[1]) {
+			pixel = 1;
+		} else if (c == '2' || c == gfxDigits[2]) {
+			pixel = 2;
+		} else if (c == '3' || c == gfxDigits[3]) {
+			pixel = 3;
 		} else {
 			break;
 		}
@@ -1826,7 +1912,7 @@ static Token yylex_NORMAL() {
 			case 'o':
 			case 'O':
 				shiftChar();
-				return Token(T_(NUMBER), readNumber(8, 0));
+				return Token(T_(NUMBER), readOctalNumber());
 			case 'b':
 			case 'B':
 				shiftChar();
@@ -1845,7 +1931,7 @@ static Token yylex_NORMAL() {
 		case '7':
 		case '8':
 		case '9': {
-			uint32_t n = readNumber(10, c - '0');
+			uint32_t n = readDecimalNumber(c);
 
 			if (peek() == '.') {
 				shiftChar();
@@ -1863,7 +1949,7 @@ static Token yylex_NORMAL() {
 				shiftChar();
 				return Token(T_(OP_LOGICAND));
 			} else if (c >= '0' && c <= '7') {
-				return Token(T_(NUMBER), readNumber(8, 0));
+				return Token(T_(NUMBER), readOctalNumber());
 			}
 			return Token(T_(OP_AND));
 
@@ -1872,7 +1958,7 @@ static Token yylex_NORMAL() {
 			if (c == '=') {
 				shiftChar();
 				return Token(T_(POP_MODEQ));
-			} else if (c == binDigits[0] || c == binDigits[1]) {
+			} else if (c == '0' || c == '1' || c == binDigits[0] || c == binDigits[1]) {
 				return Token(T_(NUMBER), readBinaryNumber());
 			}
 			return Token(T_(OP_MOD));

src/asm/opt.cpp

@@ -14,8 +14,8 @@
 #include "asm/warning.hpp"
 
 struct OptStackEntry {
-	char binary[2];
+	char binDigits[2];
-	char gbgfx[4];
+	char gfxDigits[4];
 	uint8_t fixPrecision;
 	uint8_t fillByte;
 	bool warningsAreErrors;
@@ -151,13 +151,8 @@ void opt_Push() {
 	OptStackEntry entry;
 
 	// Both of these are pulled from lexer.hpp
-	entry.binary[0] = binDigits[0];
+	memcpy(entry.binDigits, binDigits, std::size(binDigits));
-	entry.binary[1] = binDigits[1];
+	memcpy(entry.gfxDigits, gfxDigits, std::size(gfxDigits));
-
-	entry.gbgfx[0] = gfxDigits[0];
-	entry.gbgfx[1] = gfxDigits[1];
-	entry.gbgfx[2] = gfxDigits[2];
-	entry.gbgfx[3] = gfxDigits[3];
 
 	entry.fixPrecision = fixPrecision; // Pulled from fixpoint.hpp
 
@@ -181,8 +176,8 @@ void opt_Pop() {
 	OptStackEntry entry = stack.top();
 	stack.pop();
 
-	opt_B(entry.binary);
+	opt_B(entry.binDigits);
-	opt_G(entry.gbgfx);
+	opt_G(entry.gfxDigits);
 	opt_P(entry.fillByte);
 	opt_Q(entry.fixPrecision);
 	opt_R(entry.maxRecursionDepth);

test/asm/invalid-opt.asm

@@ -1,5 +1,13 @@
 opt b123
+opt b_1
+opt b10
+opt b00
 opt g12345
+opt g012_
+opt g$123
+opt g0234
+opt gxxyy
+opt gxyzy
 opt pxy
 opt p1234
 opt Qxy

test/asm/invalid-opt.err

@@ -1,23 +1,39 @@
 error: invalid-opt.asm(1):
     Must specify exactly 2 characters for option 'b'
 error: invalid-opt.asm(2):
-    Must specify exactly 4 characters for option 'g'
+    Invalid digit for binary constant '_'
 error: invalid-opt.asm(3):
-    Invalid argument for option 'p'
+    Changed digit for binary constant '1'
 error: invalid-opt.asm(4):
-    Invalid argument for option 'p'
+    Repeated digit for binary constant '0'
 error: invalid-opt.asm(5):
-    Invalid argument for option 'Q'
+    Must specify exactly 4 characters for option 'g'
 error: invalid-opt.asm(6):
-    Invalid argument for option 'Q'
+    Invalid digit for graphics constant '_'
 error: invalid-opt.asm(7):
-    Argument for option 'Q' must be between 1 and 31
+    Invalid digit for graphics constant '$'
 error: invalid-opt.asm(8):
-    Argument to 'r' is out of range ("99999999999999999999999999")
+    Changed digit for graphics constant '2'
 error: invalid-opt.asm(9):
-    Must specify an argument for option 'W'
+    Repeated digit for graphics constant 'x'
 error: invalid-opt.asm(10):
-    syntax error, unexpected end of line, expecting string
+    Repeated digit for graphics constant 'y'
 error: invalid-opt.asm(11):
+    Invalid argument for option 'p'
+error: invalid-opt.asm(12):
+    Invalid argument for option 'p'
+error: invalid-opt.asm(13):
+    Invalid argument for option 'Q'
+error: invalid-opt.asm(14):
+    Invalid argument for option 'Q'
+error: invalid-opt.asm(15):
+    Argument for option 'Q' must be between 1 and 31
+error: invalid-opt.asm(16):
+    Argument to 'r' is out of range ("99999999999999999999999999")
+error: invalid-opt.asm(17):
+    Must specify an argument for option 'W'
+error: invalid-opt.asm(18):
+    syntax error, unexpected end of line, expecting string
+error: invalid-opt.asm(19):
     No entries in the option stack
-error: Assembly aborted (11 errors)!
+error: Assembly aborted (19 errors)!

test/asm/null-outside-string.asm

@@ -1,7 +1,7 @@
 SECTION "test", ROM0
 	; '\0' is not special here; it's lexed as a line continuation...
-	DEF foo\0bar EQU 42
+	DEF foo\0qux EQU 42
-	db foo\0bar
+	db foo\0qux
 	; ...just like any other non-whitespace character
 	DEF spam\Xeggs EQU 69
 	db spam\Xeggs

test/asm/opt-g.asm

@@ -1,4 +1,4 @@
 PRINTLN `pqpq_rsrs
 
-OPT g.x0X
+OPT g.xOX
-PRINTLN `.x.x_0X0X
+PRINTLN `.x.x_OXOX

test/asm/underscore-in-numeric-literal.asm

@@ -22,7 +22,7 @@ _1234::
 	dl 6_._283_185 ; fixed point
 	dw `0123_3210, `00_33_22_11_ ; gfx
 
-; underscores as digits
+; underscores with custom digits
-	opt g_ABC, b_X
+	opt g.ABC, b.X
-	db %_X_X__XX
+	db %.X.X_..XX_
-	dw `_A_B_C__
+	dw `.A.B_.C.._