[src/trunk]: src/usr.bin/make make(1): fix edge case in := with undefined in ...

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

details:   https://anonhg.NetBSD.org/src/rev/94712cfc60f7
branches:  trunk
changeset: 958178:94712cfc60f7
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sun Dec 27 22:29:37 2020 +0000

description:
make(1): fix edge case in := with undefined in variable name

Previously, the assignment "VAR${UNDEF} := value" actually assigned to 2
variables.  See var-op-expand.mk for details.

diffstat:

 usr.bin/make/parse.c                     |  19 +++++--------------
 usr.bin/make/unit-tests/var-op-expand.mk |  25 ++++++++++++-------------
 2 files changed, 17 insertions(+), 27 deletions(-)

diffs (96 lines):

diff -r bdcd1d11789c -r 94712cfc60f7 usr.bin/make/parse.c
--- a/usr.bin/make/parse.c      Sun Dec 27 21:31:27 2020 +0000
+++ b/usr.bin/make/parse.c      Sun Dec 27 22:29:37 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: parse.c,v 1.519 2020/12/27 18:22:28 rillig Exp $       */
+/*     $NetBSD: parse.c,v 1.520 2020/12/27 22:29:37 rillig Exp $       */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -117,7 +117,7 @@
 #include "pathnames.h"
 
 /*     "@(#)parse.c    8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: parse.c,v 1.519 2020/12/27 18:22:28 rillig Exp $");
+MAKE_RCSID("$NetBSD: parse.c,v 1.520 2020/12/27 22:29:37 rillig Exp $");
 
 /* types and constants */
 
@@ -1931,17 +1931,6 @@
        char *evalue;
        Boolean savedPreserveUndefined = preserveUndefined;
 
-       /* TODO: Can this assignment to preserveUndefined be moved further down
-        * to the actually interesting Var_Subst call, without affecting any
-        * edge cases?
-        *
-        * It might affect the implicit expansion of the variable name in the
-        * Var_Exists and Var_Set calls, even though it's unlikely that anyone
-        * cared about this edge case when adding this code.  In addition,
-        * variable assignments should not refer to any undefined variables in
-        * the variable name. */
-       preserveUndefined = TRUE;
-
        /*
         * make sure that we set the variable the first time to nothing
         * so that it gets substituted!
@@ -1949,9 +1938,11 @@
        if (!Var_Exists(name, ctxt))
                Var_Set(name, "", ctxt);
 
+       preserveUndefined = TRUE;
        (void)Var_Subst(uvalue, ctxt, VARE_WANTRES | VARE_KEEP_DOLLAR, &evalue);
+       preserveUndefined = savedPreserveUndefined;
        /* TODO: handle errors */
-       preserveUndefined = savedPreserveUndefined;
+
        avalue = evalue;
        Var_Set(name, avalue, ctxt);
 
diff -r bdcd1d11789c -r 94712cfc60f7 usr.bin/make/unit-tests/var-op-expand.mk
--- a/usr.bin/make/unit-tests/var-op-expand.mk  Sun Dec 27 21:31:27 2020 +0000
+++ b/usr.bin/make/unit-tests/var-op-expand.mk  Sun Dec 27 22:29:37 2020 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: var-op-expand.mk,v 1.7 2020/12/27 21:31:27 rillig Exp $
+# $NetBSD: var-op-expand.mk,v 1.8 2020/12/27 22:29:37 rillig Exp $
 #
 # Tests for the := variable assignment operator, which expands its
 # right-hand side.
@@ -116,26 +116,25 @@
 .endif
 
 
-# XXX: edge case: When a variable name refers to an undefined variable, the
-# behavior differs between the '=' and the ':=' assignment operators.
-# This bug exists since var.c 1.42 from 2000-05-11.
+# Between var.c 1.42 from 2000-05-11 and before parse.c 1.520 from 2020-12-27,
+# if the variable name in a ':=' assignment referred to an undefined variable,
+# there were actually 2 assignments to different variables:
 #
-# The '=' operator expands the undefined variable to an empty string, thus
-# assigning to VAR_ASSIGN_.  In the name of variables to be set, it should
-# really be forbidden to refer to undefined variables.
+#      Global["VAR_SUBST_${UNDEF}"] = ""
+#      Global["VAR_SUBST_"] = ""
 #
-# The ':=' operator expands the variable name twice.  In one of these
-# expansions, the undefined variable expression is preserved (controlled by
-# preserveUndefined in VarAssign_EvalSubst), in the other expansion it expands
-# to an empty string.  This way, 2 variables are created using a single
-# variable assignment.  It's magic. :-/
+# The variable name with the empty value actually included a dollar sign.
+# Variable names with dollars are not used in practice.
+#
+# It might be a good idea to forbid undefined variables on the left-hand side
+# of a variable assignment.
 .undef UNDEF
 VAR_ASSIGN_${UNDEF}=   assigned by '='
 VAR_SUBST_${UNDEF}:=   assigned by ':='
 .if ${VAR_ASSIGN_} != "assigned by '='"
 .  error
 .endif
-.if ${${:UVAR_SUBST_\${UNDEF\}}} != ""
+.if defined(${:UVAR_SUBST_\${UNDEF\}})
 .  error
 .endif
 .if ${VAR_SUBST_} != "assigned by ':='"