[src/trunk]: src/usr.bin/make make(1): in lint mode, complain about erroneous $$

[rillig <rillig%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/80f1486838b1
branches:  trunk
changeset: 976126:80f1486838b1
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sun Sep 13 07:42:20 2020 +0000

description:
make(1): in lint mode, complain about erroneous $$

Since 2008-12-21, make has silently ignored strange variable names in
constructs like '$$', '$}', '$' followed by nothing.  Ignoring these
bugs in makefiles instead of reporting them is not a good idea.

To improve the situation, make complains about these errors now, but
only in lint mode (-dL).  This preserves existing behavior while still
allowing to validate existing makefiles that they don't depend on this
bug.

If the test phase goes well, these error messages may be enabled
unconditionally.

https://mail-index.netbsd.org/pkgsrc-users/2020/09/12/msg032229.html

diffstat:

 usr.bin/make/unit-tests/varmod.exp |   7 ++++-
 usr.bin/make/unit-tests/varmod.mk  |  51 +++++++++++++++++++++++++++++++++++--
 usr.bin/make/var.c                 |  32 +++++++++++++++++++----
 3 files changed, 79 insertions(+), 11 deletions(-)

diffs (143 lines):

diff -r 1a143592c3d1 -r 80f1486838b1 usr.bin/make/unit-tests/varmod.exp
--- a/usr.bin/make/unit-tests/varmod.exp        Sun Sep 13 07:36:55 2020 +0000
+++ b/usr.bin/make/unit-tests/varmod.exp        Sun Sep 13 07:42:20 2020 +0000
@@ -1,1 +1,6 @@
-exit status 0
+make: "varmod.mk" line 42: To escape a dollar, use \$, not $$, at "$$:L} != """
+make: "varmod.mk" line 42: Invalid variable name ':', at "$:L} != """
+make: "varmod.mk" line 47: Dollar followed by nothing
+make: Fatal errors encountered -- cannot continue
+make: stopped in unit-tests
+exit status 1
diff -r 1a143592c3d1 -r 80f1486838b1 usr.bin/make/unit-tests/varmod.mk
--- a/usr.bin/make/unit-tests/varmod.mk Sun Sep 13 07:36:55 2020 +0000
+++ b/usr.bin/make/unit-tests/varmod.mk Sun Sep 13 07:42:20 2020 +0000
@@ -1,8 +1,51 @@
-# $NetBSD: varmod.mk,v 1.2 2020/08/16 14:25:16 rillig Exp $
+# $NetBSD: varmod.mk,v 1.3 2020/09/13 07:42:20 rillig Exp $
 #
 # Tests for variable modifiers, such as :Q, :S,from,to or :Ufallback.
 
-# TODO: Implementation
+DOLLAR1=       $$
+DOLLAR2=       ${:U\$}
+
+# To get a single '$' sign in the value of a variable expression, it has to
+# be written as '$$' in a literal variable value.
+#
+# See Var_Parse, where it calls Var_Subst.
+.if ${DOLLAR1} != "\$"
+.  error
+.endif
+
+# Another way to get a single '$' sign is to use the :U modifier.  In the
+# argument of that modifier, a '$' is escaped using the backslash instead.
+#
+# See Var_Parse, where it calls Var_Subst.
+.if ${DOLLAR2} != "\$"
+.  error
+.endif
 
-all:
-       @:;
+# It is also possible to use the :U modifier directly in the expression.
+#
+# See Var_Parse, where it calls Var_Subst.
+.if ${:U\$} != "\$"
+.  error
+.endif
+
+# XXX: As of 2020-09-13, it is not possible to use '$$' in a variable name
+# to mean a single '$'.  This contradicts the manual page, which says that
+# '$' can be escaped as '$$'.
+.if ${$$:L} != ""
+.  error
+.endif
+
+# In lint mode, make prints helpful error messages.
+# For compatibility, make does not print these error messages in normal mode.
+# Should it?
+.MAKEFLAGS: -dL
+.if ${$$:L} != ""
+.  error
+.endif
+
+# A '$' followed by nothing is an error as well.
+.if ${:Uword:@word@${word}$@} != "word"
+.  error
+.endif
+
+all: # nothing
diff -r 1a143592c3d1 -r 80f1486838b1 usr.bin/make/var.c
--- a/usr.bin/make/var.c        Sun Sep 13 07:36:55 2020 +0000
+++ b/usr.bin/make/var.c        Sun Sep 13 07:42:20 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: var.c,v 1.506 2020/09/13 05:55:39 rillig Exp $ */
+/*     $NetBSD: var.c,v 1.507 2020/09/13 07:42:20 rillig Exp $ */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -69,14 +69,14 @@
  */
 
 #ifndef MAKE_NATIVE
-static char rcsid[] = "$NetBSD: var.c,v 1.506 2020/09/13 05:55:39 rillig Exp $";
+static char rcsid[] = "$NetBSD: var.c,v 1.507 2020/09/13 07:42:20 rillig Exp $";
 #else
 #include <sys/cdefs.h>
 #ifndef lint
 #if 0
 static char sccsid[] = "@(#)var.c      8.3 (Berkeley) 3/19/94";
 #else
-__RCSID("$NetBSD: var.c,v 1.506 2020/09/13 05:55:39 rillig Exp $");
+__RCSID("$NetBSD: var.c,v 1.507 2020/09/13 07:42:20 rillig Exp $");
 #endif
 #endif /* not lint */
 #endif
@@ -3369,6 +3369,27 @@
     return Buf_Destroy(&buf, FALSE);
 }
 
+static Boolean
+ValidShortVarname(char varname, const char *start)
+{
+    if (varname != '\0' && strchr(")}:$", varname) == NULL)
+       return TRUE;
+
+    if (!DEBUG(LINT))
+       return FALSE;
+
+    if (varname == '$')
+       Parse_Error(PARSE_FATAL,
+                   "To escape a dollar, use \\$, not $$, at \"%s\"", start);
+    else if (varname == '\0')
+       Parse_Error(PARSE_FATAL, "Dollar followed by nothing");
+    else
+       Parse_Error(PARSE_FATAL,
+                   "Invalid variable name '%c', at \"%s\"", varname, start);
+
+    return FALSE;
+}
+
 /*-
  *-----------------------------------------------------------------------
  * Var_Parse --
@@ -3454,8 +3475,7 @@
         * value if it exists.
         */
 
-       /* Error out some really stupid names */
-       if (startc == '\0' || strchr(")}:$", startc)) {
+       if (!ValidShortVarname(startc, start)) {
            (*pp)++;
            return var_Error;
        }
@@ -3572,7 +3592,7 @@
      * return.
      */
     nstr = Buf_GetAll(&v->val, NULL);
-    if (strchr(nstr, '$') != NULL && (eflags & VARE_WANTRES) != 0) {
+    if (strchr(nstr, '$') != NULL && (eflags & VARE_WANTRES)) {
        nstr = Var_Subst(nstr, ctxt, eflags);
        *freePtr = nstr;
     }