Implement -Wnumeric-string[=0|1|2] (#935)

Fixes #934

include/asm/section.h

@@ -61,10 +61,10 @@ void sect_EndUnion(void);
 void sect_CheckUnionClosed(void);
 
 void sect_AbsByte(uint8_t b);
-void sect_AbsByteGroup(uint8_t const *s, int32_t length);
+void sect_AbsByteGroup(uint8_t const *s, uint32_t length);
-void sect_AbsWordGroup(uint8_t const *s, int32_t length);
+void sect_AbsWordGroup(uint8_t const *s, uint32_t length);
-void sect_AbsLongGroup(uint8_t const *s, int32_t length);
+void sect_AbsLongGroup(uint8_t const *s, uint32_t length);
-void sect_Skip(int32_t skip, bool ds);
+void sect_Skip(uint32_t skip, bool ds);
 void sect_String(char const *s);
 void sect_RelByte(struct Expression *expr, uint32_t pcShift);
 void sect_RelBytes(uint32_t n, struct Expression *exprs, size_t size);

include/asm/warning.h

@@ -42,8 +42,11 @@ enum WarningID {
 
 	// Warnings past this point are "parametric" warnings, only mapping to a single flag
 #define PARAM_WARNINGS_START NB_PLAIN_WARNINGS
+	// Treating string as number may lose some bits
+	WARNING_NUMERIC_STRING_1 = PARAM_WARNINGS_START,
+	WARNING_NUMERIC_STRING_2,
 	// Implicit truncation loses some bits
-	WARNING_TRUNCATION_1 = PARAM_WARNINGS_START,
+	WARNING_TRUNCATION_1,
 	WARNING_TRUNCATION_2,
 
 	NB_PLAIN_AND_PARAM_WARNINGS,

src/asm/parser.y

@@ -52,16 +52,21 @@ static void lowerstring(char *dest, char const *src)
 	*dest = '\0';
 }
 
-static uint32_t str2int2(uint8_t *s, int32_t length)
+static uint32_t str2int2(uint8_t *s, uint32_t length)
 {
-	int32_t i;
+	if (length > 4)
+		warning(WARNING_NUMERIC_STRING_1,
+			"Treating string as a number ignores first %" PRIu32 " character%s\n",
+			length - 4, length == 5 ? "" : "s");
+	else if (length > 1)
+		warning(WARNING_NUMERIC_STRING_2,
+			"Treating %" PRIu32 "-character string as a number\n", length);
+
 	uint32_t r = 0;
 
-	i = length < 4 ? 0 : length - 4;
+	for (uint32_t i = length < 4 ? 0 : length - 4; i < length; i++) {
-	while (i < length) {
 		r <<= 8;
 		r |= s[i];
-		i++;
 	}
 
 	return r;
@@ -1311,7 +1316,7 @@ constlist_8bit_entry : reloc_8bit_no_str {
 		}
 		| string {
 			uint8_t *output = malloc(strlen($1)); /* Cannot be larger than that */
-			int32_t length = charmap_Convert($1, output);
+			uint32_t length = charmap_Convert($1, output);
 
 			sect_AbsByteGroup(output, length);
 			free(output);
@@ -1327,7 +1332,7 @@ constlist_16bit_entry : reloc_16bit_no_str {
 		}
 		| string {
 			uint8_t *output = malloc(strlen($1)); /* Cannot be larger than that */
-			int32_t length = charmap_Convert($1, output);
+			uint32_t length = charmap_Convert($1, output);
 
 			sect_AbsWordGroup(output, length);
 			free(output);
@@ -1344,7 +1349,7 @@ constlist_32bit_entry : relocexpr_no_str {
 		| string {
 			// Charmaps cannot increase the length of a string
 			uint8_t *output = malloc(strlen($1));
-			int32_t length = charmap_Convert($1, output);
+			uint32_t length = charmap_Convert($1, output);
 
 			sect_AbsLongGroup(output, length);
 			free(output);
@@ -1390,7 +1395,7 @@ relocexpr	: relocexpr_no_str
 		| string {
 			// Charmaps cannot increase the length of a string
 			uint8_t *output = malloc(strlen($1));
-			int32_t length = charmap_Convert($1, output);
+			uint32_t length = charmap_Convert($1, output);
 			uint32_t r = str2int2(output, length);
 
 			free(output);

src/asm/rgbasm.1

@@ -243,6 +243,18 @@ Warn when obsolete constructs such as the
 constant or
 .Ic PRINTT
 directive are encountered.
+.It Fl Wnumeric-string=
+Warn when a multi-character string is treated as a number.
+.Fl Wnumeric-string=0
+or
+.Fl Wno-numeric-string
+disables this warning.
+.Fl Wnumeric-string=1
+or just
+.Fl Wnumeric-string
+warns about strings longer than four characters, since four or fewer characters fit within a 32-bit integer.
+.Fl Wnumeric-string=2
+warns about any multi-character string.
 .It Fl Wshift
 Warn when shifting right a negative value.
 Use a division by 2**N instead.

src/asm/section.c

@@ -628,7 +628,7 @@ void sect_AbsByte(uint8_t b)
 	writebyte(b);
 }
 
-void sect_AbsByteGroup(uint8_t const *s, int32_t length)
+void sect_AbsByteGroup(uint8_t const *s, uint32_t length)
 {
 	if (!checkcodesection())
 		return;
@@ -639,7 +639,7 @@ void sect_AbsByteGroup(uint8_t const *s, int32_t length)
 		writebyte(*s++);
 }
 
-void sect_AbsWordGroup(uint8_t const *s, int32_t length)
+void sect_AbsWordGroup(uint8_t const *s, uint32_t length)
 {
 	if (!checkcodesection())
 		return;
@@ -650,7 +650,7 @@ void sect_AbsWordGroup(uint8_t const *s, int32_t length)
 		writeword(*s++);
 }
 
-void sect_AbsLongGroup(uint8_t const *s, int32_t length)
+void sect_AbsLongGroup(uint8_t const *s, uint32_t length)
 {
 	if (!checkcodesection())
 		return;
@@ -664,7 +664,7 @@ void sect_AbsLongGroup(uint8_t const *s, int32_t length)
 /*
  * Skip this many bytes
  */
-void sect_Skip(int32_t skip, bool ds)
+void sect_Skip(uint32_t skip, bool ds)
 {
 	if (!checksection())
 		return;

src/asm/warning.c

@@ -40,6 +40,8 @@ static const enum WarningState defaultWarnings[ARRAY_SIZE(warningStates)] = {
 	[WARNING_SHIFT_AMOUNT]		= WARNING_DISABLED,
 	[WARNING_USER]			= WARNING_ENABLED,
 
+	[WARNING_NUMERIC_STRING_1]	= WARNING_ENABLED,
+	[WARNING_NUMERIC_STRING_2]	= WARNING_DISABLED,
 	[WARNING_TRUNCATION_1]		= WARNING_ENABLED,
 	[WARNING_TRUNCATION_2]		= WARNING_DISABLED,
 };
@@ -86,6 +88,8 @@ static const char * const warningFlags[NB_WARNINGS] = {
 	"user",
 
 	// Parametric warnings
+	"numeric-string",
+	"numeric-string",
 	"truncation",
 	"truncation",
 
@@ -100,6 +104,7 @@ static const struct {
 	uint8_t nbLevels;
 	uint8_t defaultLevel;
 } paramWarnings[] = {
+	{ "numeric-string", 2, 1 },
 	{ "truncation", 2, 2 },
 };
 
@@ -155,6 +160,7 @@ static uint8_t const _wallCommands[] = {
 	WARNING_LONG_STR,
 	WARNING_NESTED_COMMENT,
 	WARNING_OBSOLETE,
+	WARNING_NUMERIC_STRING_1,
 	META_WARNING_DONE
 };
 
@@ -164,6 +170,7 @@ static uint8_t const _wextraCommands[] = {
 	WARNING_MACRO_SHIFT,
 	WARNING_NESTED_COMMENT,
 	WARNING_OBSOLETE,
+	WARNING_NUMERIC_STRING_2,
 	WARNING_TRUNCATION_1,
 	WARNING_TRUNCATION_2,
 	META_WARNING_DONE
@@ -184,6 +191,8 @@ static uint8_t const _weverythingCommands[] = {
 	WARNING_OBSOLETE,
 	WARNING_SHIFT,
 	WARNING_SHIFT_AMOUNT,
+	WARNING_NUMERIC_STRING_1,
+	WARNING_NUMERIC_STRING_2,
 	WARNING_TRUNCATION_1,
 	WARNING_TRUNCATION_2,
 	/* WARNING_USER, */

src/link/main.c

@@ -138,7 +138,7 @@ _Noreturn void fatal(struct FileStackNode const *where, uint32_t lineNo, char co
 		nbErrors++;
 
 	fprintf(stderr, "Linking aborted after %" PRIu32 " error%s\n", nbErrors,
-		nbErrors != 1 ? "s" : "");
+		nbErrors == 1 ? "" : "s");
 	exit(1);
 }
 
@@ -454,7 +454,7 @@ int main(int argc, char *argv[])
 	patch_ApplyPatches();
 	if (nbErrors) {
 		fprintf(stderr, "Linking failed with %" PRIu32 " error%s\n",
-			nbErrors, nbErrors != 1 ? "s" : "");
+			nbErrors, nbErrors == 1 ? "" : "s");
 		exit(1);
 	}
 	out_WriteFiles();

test/asm/db-dw-dl-string.asm

@@ -12,6 +12,6 @@ SECTION "Test", ROM0
 	dw "A" + 1
 	dl "A" + 1
 
-	db 1, ("UVWXYZ") & $ff, -1
+	db 1, ("WXYZ") & $ff, -1
-	dw 1, ("UVWXYZ") & $ffff, -1
+	dw 1, ("WXYZ") & $ffff, -1
-	dl 1, ("UVWXYZ"), -1
+	dl 1, ("WXYZ"), -1

test/asm/db-dw-dl-string.err

@@ -0,0 +1,6 @@
+warning: db-dw-dl-string.asm(15): [-Wnumeric-string]
+    Treating 4-character string as a number
+warning: db-dw-dl-string.asm(16): [-Wnumeric-string]
+    Treating 4-character string as a number
+warning: db-dw-dl-string.asm(17): [-Wnumeric-string]
+    Treating 4-character string as a number

test/asm/if@-no-sect.err

@@ -1,3 +1,5 @@
 ERROR: if@-no-sect.asm(1):
     PC has no value outside a section
+warning: if@-no-sect.asm(1): [-Wnumeric-string]
+    Treating 2-character string as a number
 error: Assembly aborted (1 error)!

test/asm/multiple-charmaps.err

@@ -1,3 +1,15 @@
+warning: multiple-charmaps.asm(39) -> multiple-charmaps.asm::print_mapped(27): [-Wnumeric-string]
+    Treating 2-character string as a number
+warning: multiple-charmaps.asm(47) -> multiple-charmaps.asm::print_mapped(27): [-Wnumeric-string]
+    Treating 2-character string as a number
+warning: multiple-charmaps.asm(66) -> multiple-charmaps.asm::print_mapped(27): [-Wnumeric-string]
+    Treating 2-character string as a number
+warning: multiple-charmaps.asm(89) -> multiple-charmaps.asm::print_mapped(27): [-Wnumeric-string]
+    Treating 2-character string as a number
+warning: multiple-charmaps.asm(90) -> multiple-charmaps.asm::print_mapped(27): [-Wnumeric-string]
+    Treating 2-character string as a number
+warning: multiple-charmaps.asm(98) -> multiple-charmaps.asm::print_mapped(27): [-Wnumeric-string]
+    Treating 2-character string as a number
 ERROR: multiple-charmaps.asm(100) -> multiple-charmaps.asm::new_(7):
     Charmap 'map1' already exists
 ERROR: multiple-charmaps.asm(102) -> multiple-charmaps.asm::set_(13):

test/asm/warn-numeric-string.asm

@@ -0,0 +1,25 @@
+charmap "<NULL>", $00
+
+
+SECTION "ROM", ROM0
+
+MACRO try
+	OPT \1
+	; no warning
+	db "A" * 2
+	db ("<NULL>")
+	; warn at level 1
+	dl ("AB<NULL>CD")
+	dl "<NULL" + ">NULL>"
+	; warn at level 2
+	dl (STRCAT("A", "B"))
+	dl "A<NULL>Z" + 1
+ENDM
+
+	try Wno-numeric-string
+	try Wnumeric-string
+	try Wnumeric-string=0
+	try Wnumeric-string=1
+	try Wnumeric-string=2
+	try Werror=numeric-string=1
+	try Werror=numeric-string=2

test/asm/warn-numeric-string.err

@@ -0,0 +1,38 @@
+warning: warn-numeric-string.asm(20) -> warn-numeric-string.asm::try(12): [-Wnumeric-string]
+    Treating string as a number ignores first 1 character
+warning: warn-numeric-string.asm(20) -> warn-numeric-string.asm::try(13): [-Wnumeric-string]
+    Treating string as a number ignores first 1 character
+warning: warn-numeric-string.asm(20) -> warn-numeric-string.asm::try(13): [-Wnumeric-string]
+    Treating string as a number ignores first 2 characters
+warning: warn-numeric-string.asm(22) -> warn-numeric-string.asm::try(12): [-Wnumeric-string]
+    Treating string as a number ignores first 1 character
+warning: warn-numeric-string.asm(22) -> warn-numeric-string.asm::try(13): [-Wnumeric-string]
+    Treating string as a number ignores first 1 character
+warning: warn-numeric-string.asm(22) -> warn-numeric-string.asm::try(13): [-Wnumeric-string]
+    Treating string as a number ignores first 2 characters
+warning: warn-numeric-string.asm(23) -> warn-numeric-string.asm::try(12): [-Wnumeric-string]
+    Treating string as a number ignores first 1 character
+warning: warn-numeric-string.asm(23) -> warn-numeric-string.asm::try(13): [-Wnumeric-string]
+    Treating string as a number ignores first 1 character
+warning: warn-numeric-string.asm(23) -> warn-numeric-string.asm::try(13): [-Wnumeric-string]
+    Treating string as a number ignores first 2 characters
+warning: warn-numeric-string.asm(23) -> warn-numeric-string.asm::try(15): [-Wnumeric-string]
+    Treating 2-character string as a number
+warning: warn-numeric-string.asm(23) -> warn-numeric-string.asm::try(16): [-Wnumeric-string]
+    Treating 3-character string as a number
+ERROR: warn-numeric-string.asm(24) -> warn-numeric-string.asm::try(12): [-Werror=numeric-string]
+    Treating string as a number ignores first 1 character
+ERROR: warn-numeric-string.asm(24) -> warn-numeric-string.asm::try(13): [-Werror=numeric-string]
+    Treating string as a number ignores first 1 character
+ERROR: warn-numeric-string.asm(24) -> warn-numeric-string.asm::try(13): [-Werror=numeric-string]
+    Treating string as a number ignores first 2 characters
+ERROR: warn-numeric-string.asm(25) -> warn-numeric-string.asm::try(12): [-Werror=numeric-string]
+    Treating string as a number ignores first 1 character
+ERROR: warn-numeric-string.asm(25) -> warn-numeric-string.asm::try(13): [-Werror=numeric-string]
+    Treating string as a number ignores first 1 character
+ERROR: warn-numeric-string.asm(25) -> warn-numeric-string.asm::try(13): [-Werror=numeric-string]
+    Treating string as a number ignores first 2 characters
+ERROR: warn-numeric-string.asm(25) -> warn-numeric-string.asm::try(15): [-Werror=numeric-string]
+    Treating 2-character string as a number
+ERROR: warn-numeric-string.asm(25) -> warn-numeric-string.asm::try(16): [-Werror=numeric-string]
+    Treating 3-character string as a number