From 03ecd94488b85adc38746ec3e7c2a297a522598e Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sun, 26 Feb 2023 18:24:30 -0500 Subject: [PATCH] Add new warnings invalid-var and invalid-ref The "invalid-var" warning triggers if the makefile attempts to assign a value to an invalid variable name (a name containing whitespace). The "invalid-ref" warning triggers if the makefile attempts to reference an invalid variable name. Both new warnings have a default action of "warn". * NEWS: Add these new warnings. * doc/make.1: Document them in the man page. * doc/make.texi (Warnings): Document them in the user's manual. * src/warning.h: Add enum values for the new warning types. * src/main.c (initialize_warnings): Initialize the new warnings. * src/variable.h (undefine_variable_in_set, undefine_variable_global): Ask callers to provide a struct floc specifying where the variable is undefined. * src/read.c (do_undefine): Pass floc when undefining. * src/variable.c (check_valid_name): If invalid-var is enabled, check the variable name. (define_variable_in_set): Call it. (undefine_variable_in_set): Ditto. (check_variable_reference): If invalid-ref is enabled, check the variable reference. (lookup_variable): Call it. (lookup_variable_in_set): Ditto. * tests/scripts/options/warn: Add tests for the new warning types. --- NEWS | 9 ++--- doc/make.1 | 10 ++++++ doc/make.texi | 11 ++++-- src/default.c | 2 +- src/main.c | 8 +++-- src/read.c | 2 +- src/variable.c | 53 ++++++++++++++++++++++++++++- src/variable.h | 8 +++-- src/warning.h | 4 ++- tests/scripts/options/warn | 68 ++++++++++++++++++++++++++++++++++++-- 10 files changed, 158 insertions(+), 17 deletions(-) diff --git a/NEWS b/NEWS index ad56d660..98fa7647 100644 --- a/NEWS +++ b/NEWS @@ -22,10 +22,11 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=111&se * New feature: Makefile warning reporting control A new option "--warn" controls reporting of warnings for makefiles. Actions - can be set to "ignore", "warn", or "error". The existing warning for - undefined variables is implemented and defaults to "ignore". - "--warn-undefined-variables" is deprecated, and is translated to - "--warn=undefined-vars" internally. + can be set to "ignore", "warn", or "error". Two new warnings are reported: + assigning to invalid variable names, and referencing invalid variable names + (both set to "warn" by default), in addition to the existing warning for + undefined variables (defaults to "ignore"). "--warn-undefined-variables" is + deprecated, and is translated to "--warn=undefined-vars" internally. Version 4.4.1 (26 Feb 2023) diff --git a/doc/make.1 b/doc/make.1 index 933a8273..8a407b03 100644 --- a/doc/make.1 +++ b/doc/make.1 @@ -374,6 +374,10 @@ can be an action; one of or .I error to set the default action for all warnings, or it can be a specific warning: +.I invalid-var +(assigning to an invalid variable name), +.I invalid-ref +(referencing an invalid variable name), or .I undefined-var (referencing an undefined variable). The behavior of each warning can be set by adding @@ -387,6 +391,12 @@ is provided the action for all warnings is If no .B \-\-warn option is provided the default action for +.I invalid-var +and +.I invalid-ref +is +.I warn +and the default action for .I undefined-var is .IR ignore . diff --git a/doc/make.texi b/doc/make.texi index 31db623b..b3cc57a0 100644 --- a/doc/make.texi +++ b/doc/make.texi @@ -9313,6 +9313,12 @@ GNU Make can detect some types of incorrect usage in makefiles and show warnings about them. Currently these issues can be detected: @table @samp +@item invalid-var +Assigning to an invalid variable name (e.g., a name containing whitespace). + +@item invalid-ref +Using an invalid variable name in a variable reference. + @item undefined-var Referencing a variable that has not been defined. @end table @@ -9331,8 +9337,9 @@ Show a warning about the usage and continue processing the makefile. Show an error for the usage and immediately stop processing the makefile. @end table -The default action when no warning control options are provided for -@samp{undefined-var} is @samp{warn}. +The default action of GNU Make when no warning control options are provided +is @samp{ignore} for @samp{undefined-var}, and @samp{warn} for +@samp{invalid-var} and @samp{invalid-ref}. To modify this default behavior, you can use the @code{--warn} option. This option can be specified on the command line, or by adding it to the diff --git a/src/default.c b/src/default.c index 82b52c4b..eb8da0cd 100644 --- a/src/default.c +++ b/src/default.c @@ -764,5 +764,5 @@ undefine_default_variables (void) const char **s; for (s = default_variables; *s != 0; s += 2) - undefine_variable_global (s[0], strlen (s[0]), o_default); + undefine_variable_global (NILF, s[0], strlen (s[0]), o_default); } diff --git a/src/main.c b/src/main.c index 221870ab..843cab09 100644 --- a/src/main.c +++ b/src/main.c @@ -667,6 +667,8 @@ static void initialize_warnings () { /* All warnings must have a default. */ + default_warnings[wt_invalid_var] = w_warn; + default_warnings[wt_invalid_ref] = w_warn; default_warnings[wt_undefined_var] = w_ignore; } @@ -885,7 +887,9 @@ decode_debug_flags (void) } static const char *w_state_map[w_error+1] = {NULL, "ignore", "warn", "error"}; -static const char *w_name_map[wt_max] = {"undefined-var"}; +static const char *w_name_map[wt_max] = {"invalid-var", + "invalid-ref", + "undefined-var"}; #define encode_warn_state(_b,_s) variable_buffer_output (_b, w_state_map[_s], strlen (w_state_map[_s])) #define encode_warn_name(_b,_t) variable_buffer_output (_b, w_name_map[_t], strlen (w_name_map[_t])) @@ -906,7 +910,7 @@ decode_warn_state (const char *state, size_t length) static enum warning_type decode_warn_name (const char *name, size_t length) { - for (enum warning_type wt = wt_undefined_var; wt < wt_max; ++wt) + for (enum warning_type wt = wt_invalid_var; wt < wt_max; ++wt) { size_t len = strlen (w_name_map[wt]); if (length == len && strncasecmp (name, w_name_map[wt], length) == 0) diff --git a/src/read.c b/src/read.c index 6913928f..1694ec7d 100644 --- a/src/read.c +++ b/src/read.c @@ -1368,7 +1368,7 @@ do_undefine (char *name, enum variable_origin origin, struct ebuffer *ebuf) --p; p[1] = '\0'; - undefine_variable_global (name, p - name + 1, origin); + undefine_variable_global (&ebuf->floc, name, p - name + 1, origin); free (var); } diff --git a/src/variable.c b/src/variable.c index f1911759..d2cfcc94 100644 --- a/src/variable.c +++ b/src/variable.c @@ -184,6 +184,26 @@ struct variable_set_list *current_variable_set_list = &global_setlist; /* Implement variables. */ +static void +check_valid_name (const floc* flocp, const char *name, size_t length) +{ + const char *cp, *end; + + if (!warn_check (wt_invalid_var)) + return; + + for (cp = name, end = name + length; cp < end; ++cp) + if (ISSPACE (*cp)) + break; + if (cp == end) + return; + + if (warn_error (wt_invalid_var)) + ONS (fatal, flocp, _("invalid variable name '%.*s'"), (int)length, name); + + ONS (error, flocp, _("invalid variable name '%.*s'"), (int)length, name); +} + void init_hash_global_variable_set (void) { @@ -208,6 +228,8 @@ define_variable_in_set (const char *name, size_t length, struct variable **var_slot; struct variable var_key; + check_valid_name (flocp, name, length); + if (set == NULL) set = &global_variable_set; @@ -330,7 +352,7 @@ free_variable_set (struct variable_set_list *list) } void -undefine_variable_in_set (const char *name, size_t length, +undefine_variable_in_set (const floc *flocp, const char *name, size_t length, enum variable_origin origin, struct variable_set *set) { @@ -338,6 +360,8 @@ undefine_variable_in_set (const char *name, size_t length, struct variable **var_slot; struct variable var_key; + check_valid_name (flocp, name, length); + if (set == NULL) set = &global_variable_set; @@ -452,6 +476,29 @@ lookup_special_var (struct variable *var) } +/* Check the variable name for validity. */ +static void +check_variable_reference (const char *name, size_t length) +{ + const char *cp, *end; + + if (!warn_check (wt_invalid_ref)) + return; + + for (cp = name, end = name + length; cp < end; ++cp) + if (ISSPACE (*cp)) + break; + if (cp == end) + return; + + if (warn_error (wt_invalid_ref)) + ONS (fatal, *expanding_var, + _("invalid variable reference '%.*s'"), (int)length, name); + + ONS (error, *expanding_var, + _("invalid variable reference '%.*s'"), (int)length, name); +} + /* Lookup a variable whose name is a string starting at NAME and with LENGTH chars. NAME need not be null-terminated. Returns address of the 'struct variable' containing all info @@ -464,6 +511,8 @@ lookup_variable (const char *name, size_t length) struct variable var_key; int is_parent = 0; + check_variable_reference (name, length); + var_key.name = (char *) name; var_key.length = (unsigned int) length; @@ -573,6 +622,8 @@ lookup_variable_in_set (const char *name, size_t length, { struct variable var_key; + check_variable_reference (name, length); + var_key.name = (char *) name; var_key.length = (unsigned int) length; diff --git a/src/variable.h b/src/variable.h index bbda212f..ade57531 100644 --- a/src/variable.h +++ b/src/variable.h @@ -184,6 +184,7 @@ void hash_init_function_table (void); void define_new_function(const floc *flocp, const char *name, unsigned int min, unsigned int max, unsigned int flags, gmk_func_ptr func); + struct variable *lookup_variable (const char *name, size_t length); struct variable *lookup_variable_for_file (const char *name, size_t length, struct file *file); @@ -226,14 +227,15 @@ void warn_undefined (const char* name, size_t length); #define define_variable_for_file(n,l,v,o,r,f) \ define_variable_in_set((n),(l),(v),(o),(r),(f)->variables->set,NILF) -void undefine_variable_in_set (const char *name, size_t length, +void undefine_variable_in_set (const floc *flocp, + const char *name, size_t length, enum variable_origin origin, struct variable_set *set); /* Remove variable from the current variable set. */ -#define undefine_variable_global(n,l,o) \ - undefine_variable_in_set((n),(l),(o),NULL) +#define undefine_variable_global(f,n,l,o) \ + undefine_variable_in_set((f),(n),(l),(o),NULL) char **target_environment (struct file *file, int recursive); diff --git a/src/warning.h b/src/warning.h index 778ee407..78e99893 100644 --- a/src/warning.h +++ b/src/warning.h @@ -17,7 +17,9 @@ this program. If not, see . */ /* Types of warnings we can show. */ enum warning_type { - wt_undefined_var = 0, /* Reference an undefined variable name. */ + wt_invalid_var = 0, /* Assign to an invalid variable name. */ + wt_invalid_ref, /* Reference an invalid variable name. */ + wt_undefined_var, /* Reference an undefined variable name. */ wt_max }; diff --git a/tests/scripts/options/warn b/tests/scripts/options/warn index 98155d46..f41f84ff 100644 --- a/tests/scripts/options/warn +++ b/tests/scripts/options/warn @@ -5,8 +5,8 @@ $description = "Test the --warn option."; my %warn_test = ( '--warn' => '', '--warn=warn' => '', '--warn=error --warn=warn' => '', '--warn --warn=error' => '=error', - '--warn=ignore --warn=error --warn=ignore --warn=undefined-var' => '=ignore,undefined-var', - '--warn=undefined-var:error --warn' => '=warn,undefined-var:error' + '--warn=ignore --warn=error --warn=ignore --warn=invalid-var,invalid-ref,undefined-var' => '=ignore,invalid-var,invalid-ref,undefined-var', + '--warn=invalid-ref:ignore --warn=error --warn=invalid-var:warn,,,,,undefined-var:error,,,,,' => '=error,invalid-var,invalid-ref:ignore,undefined-var:error' ); # Verify the deprecated --warn-undefined-variables option @@ -83,4 +83,68 @@ ref"); run_make_test(undef, '--warn=undefined-var:error', "#MAKEFILE#:7: *** reference to undefined variable 'UNDEFINED'. Stop.", 512); +# Check invalid variable reference warnings + +# With no options we still check for invalid references +run_make_test(' +IREF = $(bad variable) +SIREF := $(IREF) + +define nl + + +endef + +all: ; @echo ref $(also$(nl)bad) $(IREF) $(SIREF)', + '', "#MAKEFILE#:2: invalid variable reference 'bad variable' +#MAKEFILE#:10: invalid variable reference 'also\nbad' +#MAKEFILE#:2: invalid variable reference 'bad variable' +ref"); + +run_make_test(undef, '--warn=ignore', 'ref'); + +run_make_test(undef, '--warn=invalid-ref:ignore', 'ref'); + +# Check and errors +run_make_test(undef, '--warn=invalid-ref:error', + "#MAKEFILE#:2: *** invalid variable reference 'bad variable'. Stop.", 512); + +# Check invalid variable name warnings + +# With no options we still check for invalid references +run_make_test(' +EMPTY = +SPACE = $(EMPTY) $(EMPTY) +BAD$(SPACE)VAR = foo + +define nl + + +endef + +NL$(nl)VAR = bar + +define BAD$(SPACE)DEF := +foo +endef + +define NL$(nl)DEF := +foo +endef + +all: ; @echo ref', + '', "#MAKEFILE#:4: invalid variable name 'BAD VAR' +#MAKEFILE#:11: invalid variable name 'NL\nVAR' +#MAKEFILE#:13: invalid variable name 'BAD DEF' +#MAKEFILE#:17: invalid variable name 'NL\nDEF' +ref"); + +run_make_test(undef, '--warn=ignore', 'ref'); + +run_make_test(undef, '--warn=invalid-var:ignore', 'ref'); + +# Check and errors +run_make_test(undef, '--warn=invalid-var:error', + "#MAKEFILE#:4: *** invalid variable name 'BAD VAR'. Stop.", 512); + 1;