diff --git a/include/asm/warning.h b/include/asm/warning.h index 1bd9f53d..310b7198 100644 --- a/include/asm/warning.h +++ b/include/asm/warning.h @@ -21,39 +21,48 @@ enum WarningState { }; enum WarningID { - WARNING_ASSERT, /* Assertions */ - WARNING_BACKWARDS_FOR, /* `for` loop with backwards range */ - WARNING_BUILTIN_ARG, /* Invalid args to builtins */ - WARNING_CHARMAP_REDEF, /* Charmap entry re-definition */ - WARNING_DIV, /* Division undefined behavior */ - WARNING_EMPTY_DATA_DIRECTIVE, /* `db`, `dw` or `dl` directive without data in ROM */ - WARNING_EMPTY_MACRO_ARG, /* Empty macro argument */ - WARNING_EMPTY_STRRPL, /* Empty second argument in `STRRPL` */ - WARNING_LARGE_CONSTANT, /* Constants too large */ - WARNING_LONG_STR, /* String too long for internal buffers */ - WARNING_MACRO_SHIFT, /* Shift past available arguments in macro */ - WARNING_NESTED_COMMENT, /* Comment-start delimiter in a block comment */ - WARNING_OBSOLETE, /* Obsolete things */ - WARNING_SHIFT, /* Shifting undefined behavior */ - WARNING_SHIFT_AMOUNT, /* Strange shift amount */ - WARNING_TRUNCATION, /* Implicit truncation loses some bits */ - WARNING_USER, /* User warnings */ + WARNING_ASSERT, // Assertions + WARNING_BACKWARDS_FOR, // `for` loop with backwards range + WARNING_BUILTIN_ARG, // Invalid args to builtins + WARNING_CHARMAP_REDEF, // Charmap entry re-definition + WARNING_DIV, // Division undefined behavior + WARNING_EMPTY_DATA_DIRECTIVE, // `db`, `dw` or `dl` directive without data in ROM + WARNING_EMPTY_MACRO_ARG, // Empty macro argument + WARNING_EMPTY_STRRPL, // Empty second argument in `STRRPL` + WARNING_LARGE_CONSTANT, // Constants too large + WARNING_LONG_STR, // String too long for internal buffers + WARNING_MACRO_SHIFT, // Shift past available arguments in macro + WARNING_NESTED_COMMENT, // Comment-start delimiter in a block comment + WARNING_OBSOLETE, // Obsolete things + WARNING_SHIFT, // Shifting undefined behavior + WARNING_SHIFT_AMOUNT, // Strange shift amount + WARNING_USER, // User warnings - NB_WARNINGS, + NB_PLAIN_WARNINGS, - /* Warnings past this point are "meta" warnings */ - WARNING_ALL = NB_WARNINGS, + // Warnings past this point are "parametric" warnings, only mapping to a single flag +#define PARAM_WARNINGS_START NB_PLAIN_WARNINGS + // Implicit truncation loses some bits + WARNING_TRUNCATION_1 = PARAM_WARNINGS_START, + WARNING_TRUNCATION_2, + + NB_PLAIN_AND_PARAM_WARNINGS, +#define NB_PARAM_WARNINGS (NB_PLAIN_AND_PARAM_WARNINGS - PARAM_WARNINGS_START) + + // Warnings past this point are "meta" warnings +#define META_WARNINGS_START NB_PLAIN_AND_PARAM_WARNINGS + WARNING_ALL = META_WARNINGS_START, WARNING_EXTRA, WARNING_EVERYTHING, - NB_WARNINGS_ALL -#define NB_META_WARNINGS (NB_WARNINGS_ALL - NB_WARNINGS) + NB_WARNINGS, +#define NB_META_WARNINGS (NB_WARNINGS - META_WARNINGS_START) }; -extern enum WarningState warningStates[NB_WARNINGS]; +extern enum WarningState warningStates[NB_PLAIN_AND_PARAM_WARNINGS]; extern bool warningsAreErrors; -void processWarningFlag(char const *flag); +void processWarningFlag(char *flag); /* * Used to warn the user about problems that don't prevent the generation of diff --git a/src/asm/opt.c b/src/asm/opt.c index 9207d841..7fde6ef8 100644 --- a/src/asm/opt.c +++ b/src/asm/opt.c @@ -17,7 +17,8 @@ struct OptStackEntry { bool haltnop; bool optimizeLoads; bool warningsAreErrors; - enum WarningState warningStates[NB_WARNINGS]; + // Don't be confused: we use the size of the **global variable** `warningStates`! + enum WarningState warningStates[sizeof(warningStates)]; struct OptStackEntry *next; }; @@ -48,7 +49,7 @@ void opt_L(bool optimize) optimizeLoads = optimize; } -void opt_W(char const *flag) +void opt_W(char *flag) { processWarningFlag(flag); } diff --git a/src/asm/rgbasm.1 b/src/asm/rgbasm.1 index 644d4d2f..53043ef4 100644 --- a/src/asm/rgbasm.1 +++ b/src/asm/rgbasm.1 @@ -248,10 +248,20 @@ Warn when shifting right a negative value. Use a division by 2**N instead. .It Fl Wshift-amount Warn when a shift's operand is negative or greater than 32. -.It Fl Wno-truncation +.It Fl Wtruncation= Warn when an implicit truncation (for example, -.Ic db ) -loses some bits. +.Ic db +to an 8-bit value) loses some bits. +.Fl Wtruncation=0 +or +.Fl Wno-truncation +disables this warning. +.Fl Wtruncation=1 +warns when an N-bit value's absolute value is 2**N or greater. +.Fl Wtruncation=2 +or just +.Fl Wtruncation +also warns when an N-bit value is below -2**(N-1), which will not fit in two's complement encoding. .It Fl Wno-user Warn when the .Ic WARN diff --git a/src/asm/rpn.c b/src/asm/rpn.c index bcd4998b..2734627a 100644 --- a/src/asm/rpn.c +++ b/src/asm/rpn.c @@ -284,8 +284,10 @@ void rpn_CheckNBit(struct Expression const *expr, uint8_t n) if (rpn_isKnown(expr)) { int32_t val = expr->val; - if (val < -(1 << (n - 1)) || val >= 1 << n) - warning(WARNING_TRUNCATION, "Expression must be %u-bit\n", n); + if (val <= -(1 << n) || val >= 1 << n) + warning(WARNING_TRUNCATION_1, "Expression must be %u-bit\n", n); + else if (val < -(1 << (n - 1))) + warning(WARNING_TRUNCATION_2, "Expression must be %u-bit\n", n); } } diff --git a/src/asm/warning.c b/src/asm/warning.c index dc233866..92ef1ddb 100644 --- a/src/asm/warning.c +++ b/src/asm/warning.c @@ -6,6 +6,7 @@ * SPDX-License-Identifier: MIT */ +#include #include #include #include @@ -21,7 +22,7 @@ unsigned int nbErrors = 0; -static enum WarningState const defaultWarnings[NB_WARNINGS] = { +static const enum WarningState defaultWarnings[ARRAY_SIZE(warningStates)] = { [WARNING_ASSERT] = WARNING_ENABLED, [WARNING_BACKWARDS_FOR] = WARNING_DISABLED, [WARNING_BUILTIN_ARG] = WARNING_DISABLED, @@ -37,11 +38,13 @@ static enum WarningState const defaultWarnings[NB_WARNINGS] = { [WARNING_OBSOLETE] = WARNING_ENABLED, [WARNING_SHIFT] = WARNING_DISABLED, [WARNING_SHIFT_AMOUNT] = WARNING_DISABLED, - [WARNING_TRUNCATION] = WARNING_ENABLED, [WARNING_USER] = WARNING_ENABLED, + + [WARNING_TRUNCATION_1] = WARNING_ENABLED, + [WARNING_TRUNCATION_2] = WARNING_DISABLED, }; -enum WarningState warningStates[NB_WARNINGS]; +enum WarningState warningStates[ARRAY_SIZE(warningStates)]; bool warningsAreErrors; /* Set if `-Werror` was specified */ @@ -64,7 +67,7 @@ static enum WarningState warningState(enum WarningID id) return state; } -static char const *warningFlags[NB_WARNINGS_ALL] = { +static const char * const warningFlags[NB_WARNINGS] = { "assert", "backwards-for", "builtin-args", @@ -80,15 +83,63 @@ static char const *warningFlags[NB_WARNINGS_ALL] = { "obsolete", "shift", "shift-amount", - "truncation", "user", + // Parametric warnings + "truncation", + "truncation", + /* Meta warnings */ "all", "extra", - "everything" /* Especially useful for testing */ + "everything", /* Especially useful for testing */ }; +static const struct { + char const *name; + uint8_t nbLevels; + uint8_t defaultLevel; +} paramWarnings[] = { + { "truncation", 2, 2 }, +}; + +static bool tryProcessParamWarning(char const *flag, uint8_t param, enum WarningState state) +{ + enum WarningID baseID = PARAM_WARNINGS_START; + + for (size_t i = 0; i < ARRAY_SIZE(paramWarnings); i++) { + uint8_t maxParam = paramWarnings[i].nbLevels; + + if (!strcmp(paramWarnings[i].name, flag)) { // Match! + // If making the warning an error but param is 0, set to the maximum + // This accommodates `-Werror=flag`, but also `-Werror=flag=0`, which is + // thus filtered out by the caller. + // A param of 0 makes sense for disabling everything, but neither for + // enabling nor "erroring". Use the default for those. + if (param == 0 && state != WARNING_DISABLED) { + param = paramWarnings[i].defaultLevel; + } else if (param > maxParam) { + if (param != 255) // Don't warn if already capped + warnx("Got parameter %" PRIu8 + " for warning flag \"%s\", but the maximum is %" + PRIu8 "; capping.\n", + param, flag, maxParam); + param = maxParam; + } + + // Set the first to enabled/error, and disable the rest + for (uint8_t ofs = 0; ofs < maxParam; ofs++) { + warningStates[baseID + ofs] = + ofs < param ? state : WARNING_DISABLED; + } + return true; + } + + baseID += maxParam; + } + return false; +} + enum MetaWarningCommand { META_WARNING_DONE = NB_WARNINGS }; @@ -102,6 +153,8 @@ static uint8_t const _wallCommands[] = { WARNING_EMPTY_STRRPL, WARNING_LARGE_CONSTANT, WARNING_LONG_STR, + WARNING_NESTED_COMMENT, + WARNING_OBSOLETE, META_WARNING_DONE }; @@ -110,6 +163,9 @@ static uint8_t const _wextraCommands[] = { WARNING_EMPTY_MACRO_ARG, WARNING_MACRO_SHIFT, WARNING_NESTED_COMMENT, + WARNING_OBSOLETE, + WARNING_TRUNCATION_1, + WARNING_TRUNCATION_2, META_WARNING_DONE }; @@ -128,7 +184,8 @@ static uint8_t const _weverythingCommands[] = { WARNING_OBSOLETE, WARNING_SHIFT, WARNING_SHIFT_AMOUNT, - /* WARNING_TRUNCATION, */ + WARNING_TRUNCATION_1, + WARNING_TRUNCATION_2, /* WARNING_USER, */ META_WARNING_DONE }; @@ -139,12 +196,12 @@ static uint8_t const *metaWarningCommands[NB_META_WARNINGS] = { _weverythingCommands }; -void processWarningFlag(char const *flag) +void processWarningFlag(char *flag) { static bool setError = false; /* First, try to match against a "meta" warning */ - for (enum WarningID id = NB_WARNINGS; id < NB_WARNINGS_ALL; id++) { + for (enum WarningID id = META_WARNINGS_START; id < NB_WARNINGS; id++) { /* TODO: improve the matching performance? */ if (!strcmp(flag, warningFlags[id])) { /* We got a match! */ @@ -152,7 +209,7 @@ void processWarningFlag(char const *flag) errx(1, "Cannot make meta warning \"%s\" into an error", flag); - uint8_t const *ptr = metaWarningCommands[id - NB_WARNINGS]; + uint8_t const *ptr = metaWarningCommands[id - META_WARNINGS_START]; for (;;) { if (*ptr == META_WARNING_DONE) @@ -168,7 +225,7 @@ void processWarningFlag(char const *flag) /* If it's not a meta warning, specially check against `-Werror` */ if (!strncmp(flag, "error", strlen("error"))) { - char const *errorFlag = flag + strlen("error"); + char *errorFlag = flag + strlen("error"); switch (*errorFlag) { case '\0': @@ -189,14 +246,60 @@ void processWarningFlag(char const *flag) /* Well, it's either a normal warning or a mistake */ - /* Check if this is a negation */ - bool isNegation = !strncmp(flag, "no-", strlen("no-")) && !setError; - char const *rootFlag = isNegation ? flag + strlen("no-") : flag; enum WarningState state = setError ? WARNING_ERROR : - isNegation ? WARNING_DISABLED : WARNING_ENABLED; + /* Not an error, then check if this is a negation */ + strncmp(flag, "no-", strlen("no-")) ? WARNING_ENABLED + : WARNING_DISABLED; + char const *rootFlag = state == WARNING_DISABLED ? flag + strlen("no-") : flag; + + // Is this a "parametric" warning? + if (state != WARNING_DISABLED) { // The `no-` form cannot be parametrized + // First, check if there is an "equals" sign followed by a decimal number + char *equals = strchr(rootFlag, '='); + + if (equals && equals[1] != '\0') { // Ignore an equal sign at the very end as well + // Is the rest of the string a decimal number? + // We want to avoid `strtoul`'s whitespace and sign, so we parse manually + uint8_t param = 0; + char const *ptr = equals + 1; + bool warned = false; + + // The `if`'s condition above ensures that this will run at least once + do { + // If we don't have a digit, bail + if (*ptr < '0' || *ptr > '9') + break; + // Avoid overflowing! + if (param > UINT8_MAX - (*ptr - '0')) { + if (!warned) + warnx("Invalid warning flag \"%s\": capping parameter at 255\n", + flag); + warned = true; // Only warn once, cap always + param = 255; + continue; + } + param = param * 10 + (*ptr - '0'); + + ptr++; + } while (*ptr); + + // If we managed to the end of the string, check that the warning indeed + // accepts a parameter + if (*ptr == '\0') { + if (setError && param == 0) { + warnx("Ignoring nonsensical warning flag \"%s\"\n", flag); + return; + } + *equals = '\0'; // Truncate the param at the '=' + if (tryProcessParamWarning(rootFlag, param, + param == 0 ? WARNING_DISABLED : state)) + return; + } + } + } /* Try to match the flag against a "normal" flag */ - for (enum WarningID id = 0; id < NB_WARNINGS; id++) { + for (enum WarningID id = 0; id < NB_PLAIN_WARNINGS; id++) { if (!strcmp(rootFlag, warningFlags[id])) { /* We got a match! */ warningStates[id] = state; @@ -204,6 +307,11 @@ void processWarningFlag(char const *flag) } } + // Lastly, this might be a "parametric" warning without an equals sign + // If it is, treat the param as 1 if enabling, or 0 if disabling + if (tryProcessParamWarning(rootFlag, 0, state)) + return; + warnx("Unknown warning `%s`", flag); } diff --git a/test/asm/warn-truncation.asm b/test/asm/warn-truncation.asm new file mode 100644 index 00000000..272dad73 --- /dev/null +++ b/test/asm/warn-truncation.asm @@ -0,0 +1,38 @@ +FOO_F EQU 5 +BAR_F EQU 7 + + +SECTION "RAM", WRAMX[$d500] + +wLabel:: + + +SECTION "ROM", ROM0 + +MACRO try + OPT \1 + ; no warning + db 1 << FOO_F + db $ff ^ (1 << FOO_F) + db ~(1 << FOO_F) + db 1 << BAR_F + db $ff ^ (1 << BAR_F) + dw wLabel + dw $10000 - wLabel + ; warn at level 1 + db 1 << 8 + db ~(1 << 8) + dw $1d200 + dw -$1d200 + ; warn at level 2 + db ~(1 << BAR_F) + dw -wLabel +ENDM + + try Wno-truncation + try Wtruncation + try Wtruncation=0 + try Wtruncation=1 + try Wtruncation=2 + try Werror=truncation=1 + try Werror=truncation=2 diff --git a/test/asm/warn-truncation.err b/test/asm/warn-truncation.err new file mode 100644 index 00000000..291ec2ca --- /dev/null +++ b/test/asm/warn-truncation.err @@ -0,0 +1,52 @@ +warning: warn-truncation.asm(33) -> warn-truncation.asm::try(23): [-Wtruncation] + Expression must be 8-bit +warning: warn-truncation.asm(33) -> warn-truncation.asm::try(24): [-Wtruncation] + Expression must be 8-bit +warning: warn-truncation.asm(33) -> warn-truncation.asm::try(25): [-Wtruncation] + Expression must be 16-bit +warning: warn-truncation.asm(33) -> warn-truncation.asm::try(26): [-Wtruncation] + Expression must be 16-bit +warning: warn-truncation.asm(33) -> warn-truncation.asm::try(28): [-Wtruncation] + Expression must be 8-bit +warning: warn-truncation.asm(33) -> warn-truncation.asm::try(29): [-Wtruncation] + Expression must be 16-bit +warning: warn-truncation.asm(35) -> warn-truncation.asm::try(23): [-Wtruncation] + Expression must be 8-bit +warning: warn-truncation.asm(35) -> warn-truncation.asm::try(24): [-Wtruncation] + Expression must be 8-bit +warning: warn-truncation.asm(35) -> warn-truncation.asm::try(25): [-Wtruncation] + Expression must be 16-bit +warning: warn-truncation.asm(35) -> warn-truncation.asm::try(26): [-Wtruncation] + Expression must be 16-bit +warning: warn-truncation.asm(36) -> warn-truncation.asm::try(23): [-Wtruncation] + Expression must be 8-bit +warning: warn-truncation.asm(36) -> warn-truncation.asm::try(24): [-Wtruncation] + Expression must be 8-bit +warning: warn-truncation.asm(36) -> warn-truncation.asm::try(25): [-Wtruncation] + Expression must be 16-bit +warning: warn-truncation.asm(36) -> warn-truncation.asm::try(26): [-Wtruncation] + Expression must be 16-bit +warning: warn-truncation.asm(36) -> warn-truncation.asm::try(28): [-Wtruncation] + Expression must be 8-bit +warning: warn-truncation.asm(36) -> warn-truncation.asm::try(29): [-Wtruncation] + Expression must be 16-bit +ERROR: warn-truncation.asm(37) -> warn-truncation.asm::try(23): [-Werror=truncation] + Expression must be 8-bit +ERROR: warn-truncation.asm(37) -> warn-truncation.asm::try(24): [-Werror=truncation] + Expression must be 8-bit +ERROR: warn-truncation.asm(37) -> warn-truncation.asm::try(25): [-Werror=truncation] + Expression must be 16-bit +ERROR: warn-truncation.asm(37) -> warn-truncation.asm::try(26): [-Werror=truncation] + Expression must be 16-bit +ERROR: warn-truncation.asm(38) -> warn-truncation.asm::try(23): [-Werror=truncation] + Expression must be 8-bit +ERROR: warn-truncation.asm(38) -> warn-truncation.asm::try(24): [-Werror=truncation] + Expression must be 8-bit +ERROR: warn-truncation.asm(38) -> warn-truncation.asm::try(25): [-Werror=truncation] + Expression must be 16-bit +ERROR: warn-truncation.asm(38) -> warn-truncation.asm::try(26): [-Werror=truncation] + Expression must be 16-bit +ERROR: warn-truncation.asm(38) -> warn-truncation.asm::try(28): [-Werror=truncation] + Expression must be 8-bit +ERROR: warn-truncation.asm(38) -> warn-truncation.asm::try(29): [-Werror=truncation] + Expression must be 16-bit diff --git a/test/asm/warn-truncation.out b/test/asm/warn-truncation.out new file mode 100644 index 00000000..e69de29b