From b2047cbb9885dfee037311ff66ef76296b99aecc Mon Sep 17 00:00:00 2001 From: "H. Peter Anvin" Date: Wed, 8 Mar 2017 01:26:40 -0800 Subject: [PATCH] Make -Werror controllable on a per-warning-class basis Make -Werror possible to control on a per-warning-class basis. While I was fixing up that code anyway, merge the handling of the -w, -W and [warning] argument and directives. Furthermore, make *all* warnings suppressible; any warning that isn't categorized now belong to category "other". However, for cleanliness sake an "other" option does not get listed in the warning messages. Signed-off-by: H. Peter Anvin --- asm/directiv.c | 34 ++------------ asm/error.c | 110 ++++++++++++++++++++++++++++++++++++++++----- asm/nasm.c | 115 +++++++++++++++++++++--------------------------- doc/changes.src | 4 ++ doc/nasmdoc.src | 61 +++++++++++++++++++------ include/error.h | 22 ++++++--- 6 files changed, 221 insertions(+), 125 deletions(-) diff --git a/asm/directiv.c b/asm/directiv.c index 1c6bfa38..faee7435 100644 --- a/asm/directiv.c +++ b/asm/directiv.c @@ -480,37 +480,11 @@ bool process_directives(char *directive) } case D_WARNING: /* [WARNING {+|-|*}warn-name] */ - { - enum warn_action { WID_OFF, WID_ON, WID_RESET }; - enum warn_action action; - int i; - - value = nasm_skip_spaces(value); - switch(*value) { - case '-': action = WID_OFF; value++; break; - case '+': action = WID_ON; value++; break; - case '*': action = WID_RESET; value++; break; - default: action = WID_ON; break; + if (!set_warning_status(value)) { + nasm_error(ERR_WARNING|ERR_WARN_UNK_WARNING, + "unknown warning option: %s", value); } - - for (i = 1; i <= ERR_WARN_MAX; i++) - if (!nasm_stricmp(value, warnings[i].name)) - break; - if (i <= ERR_WARN_MAX) { - switch (action) { - case WID_OFF: - warning_on[i] = false; - break; - case WID_ON: - warning_on[i] = true; - break; - case WID_RESET: - warning_on[i] = warning_on_global[i]; - break; - } - } - break; - } + break; case D_CPU: /* [CPU] */ cpu = get_cpu(value); diff --git a/asm/error.c b/asm/error.c index bfe833b8..45811a8d 100644 --- a/asm/error.c +++ b/asm/error.c @@ -1,5 +1,5 @@ /* ----------------------------------------------------------------------- * - * + * * Copyright 1996-2017 The NASM Authors - All Rights Reserved * See the file AUTHORS included with the NASM distribution for * the specific copyright holders. @@ -14,7 +14,7 @@ * copyright notice, this list of conditions and the following * disclaimer in the documentation and/or other materials provided * with the distribution. - * + * * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF @@ -44,11 +44,10 @@ /* * Description of the suppressible warnings for the command line and - * the [warning] directive. Entry zero isn't an actual warning, but - * it used for -w+error/-Werror. + * the [warning] directive. */ -const struct warning warnings[ERR_WARN_MAX+1] = { - {"error", "treat warnings as errors", false}, +const struct warning warnings[ERR_WARN_ALL+1] = { + {"other", "any warning not specifially mentioned below", true}, {"macro-params", "macro calls with wrong parameter count", true}, {"macro-selfref", "cyclic macro references", false}, {"macro-defaults", "macros with more default than optional parameters", true}, @@ -67,10 +66,15 @@ const struct warning warnings[ERR_WARN_MAX+1] = { {"ptr", "non-NASM keyword used in other assemblers", true}, {"bad-pragma", "empty or malformed %pragma", false}, {"unknown-pragma", "unknown %pragma facility or directive", false}, - {"not-my-pragma", "%pragma not applicable to this compilation", false} + {"not-my-pragma", "%pragma not applicable to this compilation", false}, + {"unknown-warning", "unknown warning in -W/-w or warning directive", false}, + + /* THIS ENTRY MUST COME LAST */ + {"all", "all possible warnings", false} }; -bool warning_on[ERR_WARN_MAX+1]; /* Current state */ -bool warning_on_global[ERR_WARN_MAX+1]; /* Command-line state, for reset */ + +uint8_t warning_state[ERR_WARN_ALL];/* Current state */ +uint8_t warning_state_init[ERR_WARN_ALL]; /* Command-line state, for reset */ vefunc nasm_verror; /* Global error handling function */ @@ -108,5 +112,91 @@ no_return nasm_panic_from_macro(const char *file, int line) no_return nasm_assert_failed(const char *file, int line, const char *msg) { - nasm_fatal(0, "assertion %s failed at %s:%d", msg, file, line); + nasm_panic(0, "assertion %s failed at %s:%d", msg, file, line); +} + +/* + * This is called when processing a -w or -W option, or a warning directive. + * Returns true if if the action was successful. + */ +bool set_warning_status(const char *value) +{ + enum warn_action { WID_OFF, WID_ON, WID_RESET }; + enum warn_action action; + uint8_t mask; + int i; + bool ok = false; + + value = nasm_skip_spaces(value); + switch (*value) { + case '-': + action = WID_OFF; + value++; + break; + case '+': + action = WID_ON; + value++; + break; + case '*': + action = WID_RESET; + value++; + break; + case 'N': + case 'n': + if (!nasm_strnicmp(value, "no-", 3)) { + action = WID_OFF; + value += 3; + break; + } else if (!nasm_stricmp(value, "none")) { + action = WID_OFF; + value = NULL; + break; + } + /* else fall through */ + default: + action = WID_ON; + break; + } + + mask = WARN_ST_ENABLED; + + if (value && !nasm_strnicmp(value, "error", 5)) { + switch (value[5]) { + case '=': + mask = WARN_ST_ERROR; + value += 6; + break; + case '\0': + mask = WARN_ST_ERROR; + value = NULL; + break; + default: + /* Just an accidental prefix? */ + break; + } + } + + if (value && !nasm_stricmp(value, "all")) + value = NULL; + + /* This is inefficient, but it shouldn't matter... */ + for (i = 0; i < ERR_WARN_ALL; i++) { + if (!value || !nasm_stricmp(value, warnings[i].name)) { + ok = true; /* At least one action taken */ + switch (action) { + case WID_OFF: + warning_state[i] &= ~mask; + break; + case WID_ON: + warning_state[i] |= mask; + break; + case WID_RESET: + warning_state[i] &= ~mask; + warning_state[i] |= warning_state_init[i] & mask; + break; + } + } + } + + return ok; } diff --git a/asm/nasm.c b/asm/nasm.c index 62291bb8..05c51ea0 100644 --- a/asm/nasm.c +++ b/asm/nasm.c @@ -332,6 +332,9 @@ int main(int argc, char **argv) return 1; } + /* Save away the default state of warnings */ + memcpy(warning_state_init, warning_state, sizeof warning_state); + if (!using_debug_info) { /* No debug info, redirect to the null backend (empty stubs) */ dfmt = &null_debug_form; @@ -390,8 +393,9 @@ int main(int argc, char **argv) /* pass = 1; */ preproc->reset(inname, 3, depend_ptr); - memcpy(warning_on, warning_on_global, - (ERR_WARN_MAX+1) * sizeof(bool)); + + /* Revert all warnings to the default state */ + memcpy(warning_state, warning_state_init, sizeof warning_state); while ((line = preproc->getline())) { /* @@ -624,7 +628,6 @@ static bool process_arg(char *p, char *q, int pass) char *param; int i; bool advance = false; - bool do_warn; if (!p || !p[0]) return false; @@ -803,24 +806,28 @@ static bool process_arg(char *p, char *q, int pass) " -X specifies error reporting format (gnu or vc)\n" " -w+foo enables warning foo (equiv. -Wfoo)\n" " -w-foo disable warning foo (equiv. -Wno-foo)\n\n" - " -h show invocation summary and exit\n\n" + " -w[+-]error[=foo] can be used to promote warnings to errors\n" + " -h show invocation summary and exit\n\n" "--prefix,--postfix\n" - " this options prepend or append the given argument to all\n" - " extern and global variables\n" - "Warnings:\n"); - for (i = 0; i <= ERR_WARN_MAX; i++) - printf(" %-23s %s (default %s)\n", + " these options prepend or append the given string\n" + " to all extern and global variables\n" + "\n" + "Response files should contain command line parameters,\n" + "one per line.\n" + "\n" + "Warnings for the -W/-w options:\n"); + for (i = 0; i <= ERR_WARN_ALL; i++) + printf(" %-23s %s%s\n", warnings[i].name, warnings[i].help, - warnings[i].enabled ? "on" : "off"); - printf - ("\nresponse files should contain command line parameters" - ", one per line.\n"); + i == ERR_WARN_ALL ? "\n" : + warnings[i].enabled ? " (default on)" : + " (default off)"); if (p[2] == 'f') { - printf("\nvalid output formats for -f are" + printf("valid output formats for -f are" " (`*' denotes default):\n"); ofmt_list(ofmt, stdout); } else { - printf("\nFor a list of valid output formats, use -hf.\n"); + printf("For a list of valid output formats, use -hf.\n"); printf("For a list of debug formats, use -f
-y.\n"); } exit(0); /* never need usage message here */ @@ -853,48 +860,15 @@ static bool process_arg(char *p, char *q, int pass) preproc = &preproc_nop; break; + case 'w': case 'W': if (pass == 2) { - if (param[0] == 'n' && param[1] == 'o' && param[2] == '-') { - do_warn = false; - param += 3; - } else { - do_warn = true; + if (!set_warning_status(param)) { + nasm_error(ERR_WARNING|ERR_NOFILE|ERR_WARN_UNK_WARNING, + "unknown warning option: %s", param); } - goto set_warning; } - break; - - case 'w': - if (pass == 2) { - if (param[0] != '+' && param[0] != '-') { - nasm_error(ERR_NONFATAL | ERR_NOFILE | ERR_USAGE, - "invalid option to `-w'"); - break; - } - do_warn = (param[0] == '+'); - param++; - goto set_warning; - } - break; - -set_warning: - for (i = 0; i <= ERR_WARN_MAX; i++) { - if (!nasm_stricmp(param, warnings[i].name)) - break; - } - if (i <= ERR_WARN_MAX) { - warning_on_global[i] = do_warn; - } else if (!nasm_stricmp(param, "all")) { - for (i = 1; i <= ERR_WARN_MAX; i++) - warning_on_global[i] = do_warn; - } else if (!nasm_stricmp(param, "none")) { - for (i = 1; i <= ERR_WARN_MAX; i++) - warning_on_global[i] = !do_warn; - } else { - /* Ignore invalid warning names; forward compatibility */ - } - break; + break; case 'M': if (pass == 2) { @@ -1130,8 +1104,11 @@ static void parse_cmdline(int argc, char **argv, int pass) *inname = *outname = *listname = *errname = '\0'; - for (i = 0; i <= ERR_WARN_MAX; i++) - warning_on_global[i] = warnings[i].enabled; + /* Initialize all the warnings to their default state */ + for (i = 0; i < ERR_WARN_ALL; i++) { + warning_state_init[i] = warning_state[i] = + warnings[i].enabled ? WARN_ST_ENABLED : 0; + } /* * First, process the NASMENV environment variable. @@ -1246,7 +1223,9 @@ static void assemble_file(char *fname, StrList **depend_ptr) offsets = raa_init(); } preproc->reset(fname, pass1, pass1 == 2 ? depend_ptr : NULL); - memcpy(warning_on, warning_on_global, (ERR_WARN_MAX+1) * sizeof(bool)); + + /* Revert all warnings to the default state */ + memcpy(warning_state, warning_state_init, sizeof warning_state); globallineno = 0; if (passn == 1) @@ -1571,17 +1550,13 @@ static void nasm_verror_vc(int severity, const char *fmt, va_list ap) /* * check to see if this is a suppressable warning */ -static inline bool is_suppressable_warning(int severity) +static inline bool is_valid_warning(int severity) { - int index; - /* Not a warning at all */ if ((severity & ERR_MASK) != ERR_WARNING) return false; - index = WARN_IDX(severity); - - return index && index <= ERR_WARN_MAX; + return WARN_IDX(severity) < ERR_WARN_ALL; } /** @@ -1595,8 +1570,16 @@ static inline bool is_suppressable_warning(int severity) static bool is_suppressed_warning(int severity) { /* Might be a warning but suppresed explicitly */ - if (is_suppressable_warning(severity)) - return !warning_on[WARN_IDX(severity)]; + if (is_valid_warning(severity)) + return !(warning_state[WARN_IDX(severity)] & WARN_ST_ENABLED); + else + return false; +} + +static bool warning_is_error(int severity) +{ + if (is_valid_warning(severity)) + return !!(warning_state[WARN_IDX(severity)] & WARN_ST_ERROR); else return false; } @@ -1654,7 +1637,7 @@ static void nasm_verror_common(int severity, const char *fmt, va_list args) } vsnprintf(msg, sizeof msg - 64, fmt, args); - if (is_suppressable_warning(severity)) { + if (is_valid_warning(severity) && WARN_IDX(severity) != ERR_WARN_OTHER) { char *p = strchr(msg, '\0'); snprintf(p, 64, " [-w+%s]", warnings[WARN_IDX(severity)].name); } @@ -1683,7 +1666,7 @@ static void nasm_verror_common(int severity, const char *fmt, va_list args) break; case ERR_WARNING: /* Treat warnings as errors */ - if (warning_on[WARN_IDX(ERR_WARN_TERM)]) + if (warning_is_error(severity)) terminate_after_phase = true; break; case ERR_NONFATAL: diff --git a/doc/changes.src b/doc/changes.src index a24ddf21..dfc3c8ee 100644 --- a/doc/changes.src +++ b/doc/changes.src @@ -26,6 +26,10 @@ since 2007. \b \c{macho} object format now supports the \c{subsections_via_symbols} directive, see \k{macho-ssvs}. +\b All warnings can now be suppressed if desired. Furthermore, + warning-as-error can now be controlled on a per-warning-class + basis. + \S{cl-2.12.03} Version 2.12.03 \b Add new warnings for certain dangerous constructs which never ought diff --git a/doc/nasmdoc.src b/doc/nasmdoc.src index d2e9bd24..0df4911f 100644 --- a/doc/nasmdoc.src +++ b/doc/nasmdoc.src @@ -77,6 +77,8 @@ \IR{-u} \c{-u} option \IR{-v} \c{-v} option \IR{-W} \c{-W} option +\IR{-Werror} \c{-Werror} option +\IR{-Wno-error} \c{-Wno-error} option \IR{-w} \c{-w} option \IR{-y} \c{-y} option \IR{-Z} \c{-Z} option @@ -910,7 +912,10 @@ name, for example \c{orphan-labels}; you can enable warnings of this class by the command-line option \c{-w+orphan-labels} and disable it by \c{-w-orphan-labels}. -The \i{suppressible warning} classes are: +The current \i{warning classes} are: + +\b \i\c{other} specifies any warning not otherwise specified in any +class. \b \i\c{macro-params} covers warnings about \i{multi-line macros} being invoked with the wrong number of parameters. This warning @@ -969,22 +974,36 @@ to limitations in the output format. indicate a mistake in the source code. Currently only the MASM \c{PTR} keyword is recognized. -\b \i\c{error} causes warnings to be treated as errors. Disabled by -default. +\b \i\c{bad-pragma} warns about a malformed or otherwise unparsable +\c{%pragma} directive. Disabled by default. -\b \i\c{all} is an alias for \e{all} suppressible warning classes (not -including \c{error}). Thus, \c{-w+all} enables all available warnings. +\b \i\c{unknown-pragma} warns about an unknown \c{%pragma} directive. +This is not yet implemented. Disabled by default. -In addition, you can control warnings in the source code itself, using -the \i\c{[warning]} directive. -Warning classes may be enabled with \i\c{[warning +warning-name]}, -disabled with \i\c{[warning -warning-name]} or reset to their -original value with \i\c{[warning *warning-name]}. No "user form" -(without the brackets) exists. +\b \i\c{not-my-pragma} warns about a \c{%pragma} directive which is +not applicable to this particular assembly session. This is not yet +implemented. Disabled by default. + +\b \i\c{unknown-warning} warns about a \c{-w} or \c{-W} option or a +\c{[WARNING]} directive that contains an unknown warning name or is +otherwise not possible to process. + +\b \i\c{all} is an alias for \e{all} suppressible warning classes. +Thus, \c{-w+all} enables all available warnings, and \c{-w-all} +disables warnings entirely (since NASM 2.13). Since version 2.00, NASM has also supported the gcc-like syntax -\c{-Wwarning} and \c{-Wno-warning} instead of \c{-w+warning} and -\c{-w-warning}, respectively. +\c{-Wwarning-class} and \c{-Wno-warning-class} instead of \c{-w+warning-class} and +\c{-w-warning-class}, respectively; both syntaxes work identically. + +The option \c{-w+error} or \i\c{-Werror} can be used to treat warnings +as errors. This can be controlled on a per warning class basis +(\c{-w+error=}\e{warning-class}); if no \e{warning-class} is specified +NASM treats it as \c{-w+error=all}; the same applies to \c{-w-error} +or \i\c{-Wno-error}, of course. + +In addition, you can control warnings in the source code itself, using +the \i\c{[WARNING]} directive. See \k{asmdir-warning}. \S{opt-v} The \i\c{-v} Option: Display \i{Version} Info @@ -4776,6 +4795,22 @@ has avoided the use of the brackeded primitive form, (\c{[FLOAT]}). value can be saved away and invoked later to restore the setting. +\H{asmdir-warning} \i\c{[WARNING]}: Enable or disable warnings + +The \c{[WARNING]} directive can be used to enable or disable classes +of warnings in the same way as the \c{-w} option, see \k{opt-w} for +more details about warning classes. + +Warning classes may be enabled with \c{[warning +]\e{warning-class}\c{]}, disabled +with \c{[warning -}\e{warning-class}\c{]}, or reset to their original value (as +specified on the command line) with \c{[warning *}\e{warning-class}{]}. + +The \c{[WARNING]} directive also accepts the \c{all}, \c{error} and +\c{error=}\e{warning-class} specifiers. + +No "user form" (without the brackets) currently exists. + + \C{outfmt} \i{Output Formats} NASM is a portable assembler, designed to be able to compile on any diff --git a/include/error.h b/include/error.h index 18a50073..b1e42ba9 100644 --- a/include/error.h +++ b/include/error.h @@ -89,7 +89,7 @@ static inline vefunc nasm_set_verror(vefunc ve) #define WARN(x) ((x) << ERR_WARN_SHR) #define WARN_IDX(x) (((x) & ERR_WARN_MASK) >> ERR_WARN_SHR) -#define ERR_WARN_TERM WARN( 0) /* treat warnings as errors */ +#define ERR_WARN_OTHER WARN( 0) /* any noncategorized warning */ #define ERR_WARN_MNP WARN( 1) /* macro-num-parameters warning */ #define ERR_WARN_MSR WARN( 2) /* macro self-reference */ #define ERR_WARN_MDP WARN( 3) /* macro default parameters check */ @@ -110,16 +110,26 @@ static inline vefunc nasm_set_verror(vefunc ve) #define ERR_WARN_BAD_PRAGMA WARN(17) /* malformed pragma */ #define ERR_WARN_UNKNOWN_PRAGMA WARN(18) /* unknown pragma */ #define ERR_WARN_NOTMY_PRAGMA WARN(19) /* pragma inapplicable */ -#define ERR_WARN_MAX 19 /* the highest numbered one */ +#define ERR_WARN_UNK_WARNING WARN(20) /* unknown warning */ + +/* The "all" warning acts as a global switch, it must come last */ +#define ERR_WARN_ALL 21 /* Do not use WARN() here */ struct warning { const char *name; const char *help; bool enabled; }; -extern const struct warning warnings[ERR_WARN_MAX+1]; -extern bool warning_on[ERR_WARN_MAX+1]; /* Current state */ -extern bool warning_on_global[ERR_WARN_MAX+1]; /* Command-line state */ -void init_warnings(void); +extern const struct warning warnings[ERR_WARN_ALL+1]; + +/* This is a bitmask */ +#define WARN_ST_ENABLED 1 /* Warning is currently enabled */ +#define WARN_ST_ERROR 2 /* Treat this warning as an error */ + +extern uint8_t warning_state[ERR_WARN_ALL]; +extern uint8_t warning_state_init[ERR_WARN_ALL]; + +/* Process a warning option or directive */ +bool set_warning_status(const char *); #endif /* NASM_ERROR_H */