[src/trunk]: src/usr.bin/make make: fix double varname expansion in the varia...

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

details:   https://anonhg.NetBSD.org/src/rev/4ce56f34cef5
branches:  trunk
changeset: 960358:4ce56f34cef5
user:      rillig <rillig%NetBSD.org@localhost>
date:      Mon Mar 15 18:56:37 2021 +0000

description:
make: fix double varname expansion in the variable modifier '::='

This is an edge case that doesn't occur in practice since pretty much
nobody dares to use variable names that contain an actual '$' in their
name.  This is not about the fairly common VAR.${param} (as written in
the makefile), but instead about the variable whose name is literally
'VAR.${param}'.

The test demonstrates that after the fix, the variable name is taken
exactly as-is for the simple assignment modifier '::='.  There are no
such tests for the modifiers '::+=', '::!=' and '::?=', but that's ok.
The code in ApplyModifier_Assign would look assymetrical and suspicious
enough if one of these modifiers would expand its variable name and the
others wouldn't.

diffstat:

 usr.bin/make/unit-tests/varmod-assign.exp |   9 +++------
 usr.bin/make/unit-tests/varmod-assign.mk  |  11 ++++-------
 usr.bin/make/var.c                        |  11 +++++------
 3 files changed, 12 insertions(+), 19 deletions(-)

diffs (94 lines):

diff -r 2ad277349a61 -r 4ce56f34cef5 usr.bin/make/unit-tests/varmod-assign.exp
--- a/usr.bin/make/unit-tests/varmod-assign.exp Mon Mar 15 18:46:05 2021 +0000
+++ b/usr.bin/make/unit-tests/varmod-assign.exp Mon Mar 15 18:56:37 2021 +0000
@@ -6,13 +6,10 @@
 Var_Parse: ${VARNAME}::=assigned-value} (eval-defined)
 Applying ${VAR.${param}::...} to "initial-value" (eval-defined, none, regular)
 Modifier part: "assigned-value"
-Var_Parse: ${param} (eval)
-Global:VAR.twice = assigned-value
+Global:VAR.${param} = assigned-value
 Result of ${VAR.${param}::=assigned-value} is "" (eval-defined, none, regular)
-Var_Parse: $ for symmetry with the usual assignment operators. (eval)
-make: "varmod-assign.mk" line 139: FIXME: don't expand the variable name twice here, for symmetry with the usual assignment operators.
-Var_Parse: ${${VARNAME}} != "initial-value" (eval-defined)
-Var_Parse: ${VARNAME}} != "initial-value" (eval-defined)
+Var_Parse: ${${VARNAME}} != "assigned-value" (eval-defined)
+Var_Parse: ${VARNAME}} != "assigned-value" (eval-defined)
 Global:.MAKEFLAGS =  -r -k -d v -d
 Global:.MAKEFLAGS =  -r -k -d v -d 0
 mod-assign: first=1.
diff -r 2ad277349a61 -r 4ce56f34cef5 usr.bin/make/unit-tests/varmod-assign.mk
--- a/usr.bin/make/unit-tests/varmod-assign.mk  Mon Mar 15 18:46:05 2021 +0000
+++ b/usr.bin/make/unit-tests/varmod-assign.mk  Mon Mar 15 18:56:37 2021 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: varmod-assign.mk,v 1.11 2021/03/15 18:46:05 rillig Exp $
+# $NetBSD: varmod-assign.mk,v 1.12 2021/03/15 18:56:38 rillig Exp $
 #
 # Tests for the obscure ::= variable modifiers, which perform variable
 # assignments during evaluation, just like the = operator in C.
@@ -132,13 +132,10 @@
 .if ${${VARNAME}::=assigned-value} # Here the variable name gets expanded once
 .  error                       # too often.
 .endif
-.if !defined(VAR.twice)
-.  error                       # FIXME: This is the unwanted current behavior.
-.else
-.  info                FIXME: don't expand the variable name twice here, $\
-               for symmetry with the usual assignment operators.
+.if defined(VAR.twice)
+.  error The variable name in the '::=' modifier is expanded once too often.
 .endif
-.if ${${VARNAME}} != "initial-value"
+.if ${${VARNAME}} != "assigned-value"
 .  error
 .endif
 .MAKEFLAGS: -d0
diff -r 2ad277349a61 -r 4ce56f34cef5 usr.bin/make/var.c
--- a/usr.bin/make/var.c        Mon Mar 15 18:46:05 2021 +0000
+++ b/usr.bin/make/var.c        Mon Mar 15 18:56:37 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: var.c,v 1.887 2021/03/15 16:51:14 rillig Exp $ */
+/*     $NetBSD: var.c,v 1.888 2021/03/15 18:56:37 rillig Exp $ */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -140,7 +140,7 @@
 #include "metachar.h"
 
 /*     "@(#)var.c      8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.887 2021/03/15 16:51:14 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.888 2021/03/15 18:56:37 rillig Exp $");
 
 typedef enum VarFlags {
        VFL_NONE        = 0,
@@ -3425,10 +3425,9 @@
                        VarFreeEnv(gv);
        }
 
-       /* XXX: Expanding the variable name at this point sounds wrong. */
        switch (op[0]) {
        case '+':
-               Var_AppendExpand(scope, expr->var->name.str, val);
+               Var_Append(scope, expr->var->name.str, val);
                break;
        case '!': {
                const char *errfmt;
@@ -3436,7 +3435,7 @@
                if (errfmt != NULL)
                        Error(errfmt, val);
                else
-                       Var_SetExpand(scope, expr->var->name.str, cmd_output);
+                       Var_Set(scope, expr->var->name.str, cmd_output);
                free(cmd_output);
                break;
        }
@@ -3445,7 +3444,7 @@
                        break;
                /* FALLTHROUGH */
        default:
-               Var_SetExpand(scope, expr->var->name.str, val);
+               Var_Set(scope, expr->var->name.str, val);
                break;
        }
        Expr_SetValueRefer(expr, "");