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.

diff --git a/NEWS b/NEWS
index ad56d66039a6b5dbd6b0cb7bf7eba75fc8bf59cd..98fa76472b0206bcf53a51a58df806b3b7302eb0 100644 (file)
--- 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.
 
 \f
 Version 4.4.1 (26 Feb 2023)

diff --git a/doc/make.1 b/doc/make.1
index 933a8273919e7ab153e501a98fa5494f59802f0d..8a407b03c952cdd226e093548663431d9b659462 100644 (file)
--- 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 31db623b12e60ae345e46b03e580be5afcd270f8..b3cc57a07c076fed43e993a1115fcc21f99be337 100644 (file)
--- 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 82b52c4be01b7e9470ddb43b23182d899fee6e92..eb8da0cd9fedf095c8288031880a15088f89f878 100644 (file)
--- 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 221870abc2b2efcf88496e4fcc27ad52256495d4..843cab094ba30a6dba17380925fea3a1919fc6b4 100644 (file)
--- 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 6913928f34b45519768a0fee6ca385ef9e2e0e20..1694ec7d7efb2ef2f20e94ed58df9568b1dfb576 100644 (file)
--- 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 f1911759268c999674f1ae172a0f70b771b60458..d2cfcc94f3b50c788431cb3955bc32258acf5185 100644 (file)
--- a/src/variable.c
+++ b/src/variable.c
@@ -184,6 +184,26 @@ struct variable_set_list *current_variable_set_list = &global_setlist;
 \f
 /* 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)
 }
 
 \f
+/* 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 bbda212f34fe862eb522440342d07011236a2adb..ade575310f2fbc2301a7b755cda83bab5dda20cd 100644 (file)
--- 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 778ee4076034e70e22bb37ac9f0d76be04717d07..78e99893343c4ca9a8dfa5c4fc789d4998ef4574 100644 (file)
--- a/src/warning.h
+++ b/src/warning.h
@@ -17,7 +17,9 @@ this program.  If not, see <https://www.gnu.org/licenses/>.  */
 /* 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 98155d4661b67577c99750b9675b7d9f9bd8be28..f41f84ff1dc339d40dfc55b5bfcdf6356b5b1811 100644 (file)
--- 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;