diff --git a/include/asm/section.h b/include/asm/section.h index 2ff5b156..48313f37 100644 --- a/include/asm/section.h +++ b/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_AbsWordGroup(uint8_t const *s, int32_t length); -void sect_AbsLongGroup(uint8_t const *s, int32_t length); -void sect_Skip(int32_t skip, bool ds); +void sect_AbsByteGroup(uint8_t const *s, uint32_t length); +void sect_AbsWordGroup(uint8_t const *s, uint32_t length); +void sect_AbsLongGroup(uint8_t const *s, uint32_t length); +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); diff --git a/include/asm/warning.h b/include/asm/warning.h index 310b7198..1d6860d7 100644 --- a/include/asm/warning.h +++ b/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, diff --git a/src/asm/parser.y b/src/asm/parser.y index b1eb329a..44a4adf6 100644 --- a/src/asm/parser.y +++ b/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; - while (i < length) { + for (uint32_t i = length < 4 ? 0 : length - 4; i < length; i++) { 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); diff --git a/src/asm/rgbasm.1 b/src/asm/rgbasm.1 index 99404865..c08a99ec 100644 --- a/src/asm/rgbasm.1 +++ b/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. diff --git a/src/asm/section.c b/src/asm/section.c index d5a92b89..f268df7a 100644 --- a/src/asm/section.c +++ b/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; diff --git a/src/asm/warning.c b/src/asm/warning.c index 92ef1ddb..6bedc979 100644 --- a/src/asm/warning.c +++ b/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, */ diff --git a/src/link/main.c b/src/link/main.c index d1f6fc6e..cc8e2e73 100644 --- a/src/link/main.c +++ b/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(); diff --git a/test/asm/db-dw-dl-string.asm b/test/asm/db-dw-dl-string.asm index 7a72ca45..44fa3297 100644 --- a/test/asm/db-dw-dl-string.asm +++ b/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 - dw 1, ("UVWXYZ") & $ffff, -1 - dl 1, ("UVWXYZ"), -1 + db 1, ("WXYZ") & $ff, -1 + dw 1, ("WXYZ") & $ffff, -1 + dl 1, ("WXYZ"), -1 diff --git a/test/asm/db-dw-dl-string.err b/test/asm/db-dw-dl-string.err index e69de29b..e03c4e1e 100644 --- a/test/asm/db-dw-dl-string.err +++ b/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 diff --git a/test/asm/if@-no-sect.err b/test/asm/if@-no-sect.err index da640237..d95a0a8b 100644 --- a/test/asm/if@-no-sect.err +++ b/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)! diff --git a/test/asm/multiple-charmaps.err b/test/asm/multiple-charmaps.err index 13053523..23a53c97 100644 --- a/test/asm/multiple-charmaps.err +++ b/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): diff --git a/test/asm/warn-numeric-string.asm b/test/asm/warn-numeric-string.asm new file mode 100644 index 00000000..7140e43f --- /dev/null +++ b/test/asm/warn-numeric-string.asm @@ -0,0 +1,25 @@ +charmap "", $00 + + +SECTION "ROM", ROM0 + +MACRO try + OPT \1 + ; no warning + db "A" * 2 + db ("") + ; warn at level 1 + dl ("ABCD") + dl "NULL>" + ; warn at level 2 + dl (STRCAT("A", "B")) + dl "AZ" + 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 diff --git a/test/asm/warn-numeric-string.err b/test/asm/warn-numeric-string.err new file mode 100644 index 00000000..0fee15c9 --- /dev/null +++ b/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 diff --git a/test/asm/warn-numeric-string.out b/test/asm/warn-numeric-string.out new file mode 100644 index 00000000..e69de29b