[src/trunk]: src/usr.bin/make make: prevent future out-of-bounds errors when ...

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

details:   https://anonhg.NetBSD.org/src/rev/1b2a22faf15a
branches:  trunk
changeset: 369631:1b2a22faf15a
user:      rillig <rillig%NetBSD.org@localhost>
date:      Wed Aug 24 22:09:40 2022 +0000

description:
make: prevent future out-of-bounds errors when parsing expressions

A modifier in an expression ends not only at the next ':' or at the
closing '}' or ')', but also at the end of the string.

Previously, testing for the end of the string had been done separately,
which was error-prone since 2006-05-11, when indirect modifiers were
introduced.  Since then, it was possible that the string terminator '\0'
was accidentally skipped in cases where the loop condition only tested
for the ending character.  When parsing indirect modifiers, the ending
character is indeed '\0', but when parsing direct modifiers, it is '}'
or ')'.

A welcome side effect is that in the case of unclosed expressions such
as '${VAR:Modifier', the amount of error messages is reduced from 2 or 3
to only 1.  The removed error messages were wrong and thus confusing
anyway.

diffstat:

 usr.bin/make/unit-tests/Makefile            |   6 +++-
 usr.bin/make/unit-tests/varparse-errors.exp |  43 ++++++----------------------
 usr.bin/make/unit-tests/varparse-errors.mk  |  20 +++----------
 usr.bin/make/var.c                          |  22 +++++++-------
 4 files changed, 31 insertions(+), 60 deletions(-)

diffs (223 lines):

diff -r e2b0910b0297 -r 1b2a22faf15a usr.bin/make/unit-tests/Makefile
--- a/usr.bin/make/unit-tests/Makefile  Wed Aug 24 21:38:06 2022 +0000
+++ b/usr.bin/make/unit-tests/Makefile  Wed Aug 24 22:09:40 2022 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.319 2022/07/26 19:32:25 sjg Exp $
+# $NetBSD: Makefile,v 1.320 2022/08/24 22:09:41 rillig Exp $
 #
 # Unit tests for make(1)
 #
@@ -533,6 +533,7 @@
 SED_CMDS.var-op-shell+=                -e '/command/s,No such.*,not found,'
 SED_CMDS.vardebug+=            -e 's,${.SHELL},</path/to/shell>,'
 SED_CMDS.varmod-subst-regex+=  ${STD_SED_CMDS.regex}
+SED_CMDS.varparse-errors+=     ${STD_SED_CMDS.timestamp}
 SED_CMDS.varname-dot-parsedir= -e '/in some cases/ s,^make: "[^"]*,make: "<normalized>,'
 SED_CMDS.varname-dot-parsefile=        -e '/in some cases/ s,^make: "[^"]*,make: "<normalized>,'
 SED_CMDS.varname-dot-shell=    -e 's, = /[^ ]*, = (details omitted),g'
@@ -620,6 +621,9 @@
 STD_SED_CMDS.regex= \
        -e 's,\(Regex compilation error:\).*,\1 (details omitted),'
 
+STD_SED_CMDS.timestamp= \
+       -e 's,[A-Z][a-z][a-z] [A-Z][a-z][a-z] [0-9][0-9]* [0-9][0-9]*:[0-9][0-9]:[0-9][0-9] 20[0-9][0-9],<timestamp>,'
+
 # End of the configuration helpers section.
 
 UNIT_TESTS:=   ${.PARSEDIR}
diff -r e2b0910b0297 -r 1b2a22faf15a usr.bin/make/unit-tests/varparse-errors.exp
--- a/usr.bin/make/unit-tests/varparse-errors.exp       Wed Aug 24 21:38:06 2022 +0000
+++ b/usr.bin/make/unit-tests/varparse-errors.exp       Wed Aug 24 22:09:40 2022 +0000
@@ -6,43 +6,20 @@
 make: Bad modifier ":OX" for variable ""
 make: "varparse-errors.mk" line 68: Undefined variable "${:U:OX"
 make: Bad modifier ":OX" for variable ""
-make: "varparse-errors.mk" line 72: Unknown modifier "Q"
 make: Unclosed variable expression, expecting '}' for modifier "Q" of variable "" with value ""
-make: "varparse-errors.mk" line 72: Undefined variable "${:U:Q"
-make: "varparse-errors.mk" line 74: Unknown modifier "sh"
 make: Unclosed variable expression, expecting '}' for modifier "sh" of variable "" with value ""
-make: "varparse-errors.mk" line 74: Undefined variable "${:U:sh"
-make: Bad modifier ":tA" for variable ""
-make: "varparse-errors.mk" line 76: Undefined variable "${:U:tA"
-make: Bad modifier ":tsX" for variable ""
-make: "varparse-errors.mk" line 78: Undefined variable "${:U:ts"
-make: Bad modifier ":ts" for variable ""
-make: "varparse-errors.mk" line 80: Undefined variable "${:U:ts"
-make: Bad modifier ":ts\040" for variable ""
-make: "varparse-errors.mk" line 82: Undefined variable "${:U:ts"
-make: "varparse-errors.mk" line 84: Unknown modifier "u"
+make: Unclosed variable expression, expecting '}' for modifier "tA" of variable "" with value ""
+make: Unclosed variable expression, expecting '}' for modifier "tsX" of variable "" with value ""
+make: Unclosed variable expression, expecting '}' for modifier "ts" of variable "" with value ""
+make: Unclosed variable expression, expecting '}' for modifier "ts\040" of variable "" with value ""
 make: Unclosed variable expression, expecting '}' for modifier "u" of variable "" with value ""
-make: "varparse-errors.mk" line 84: Undefined variable "${:U:u"
-make: "varparse-errors.mk" line 86: Unknown modifier "H"
-make: Unclosed variable expression, expecting '}' for modifier "H" of variable "" with value ""
-make: "varparse-errors.mk" line 86: Undefined variable "${:U:H"
-make: Bad modifier ":[1]" for variable ""
-make: "varparse-errors.mk" line 88: Undefined variable "${:U:[1]"
-make: "varparse-errors.mk" line 90: Unknown modifier "hash"
-make: Unclosed variable expression, expecting '}' for modifier "hash" of variable "" with value ""
-make: "varparse-errors.mk" line 90: Undefined variable "${:U:hash"
-make: "varparse-errors.mk" line 92: Unknown modifier "range"
-make: Unclosed variable expression, expecting '}' for modifier "range" of variable "" with value ""
-make: "varparse-errors.mk" line 92: Undefined variable "${:U:range"
-make: "varparse-errors.mk" line 94: Unknown modifier "_"
+make: Unclosed variable expression, expecting '}' for modifier "H" of variable "" with value "."
+make: Unclosed variable expression, expecting '}' for modifier "[1]" of variable "" with value ""
+make: Unclosed variable expression, expecting '}' for modifier "hash" of variable "" with value "b2af338b"
+make: Unclosed variable expression, expecting '}' for modifier "range" of variable "" with value "1"
 make: Unclosed variable expression, expecting '}' for modifier "_" of variable "" with value ""
-make: "varparse-errors.mk" line 94: Undefined variable "${:U:_"
-make: "varparse-errors.mk" line 96: Unknown modifier "gmtime"
-make: Unclosed variable expression, expecting '}' for modifier "gmtime" of variable "" with value ""
-make: "varparse-errors.mk" line 96: Undefined variable "${:U:gmtime"
-make: "varparse-errors.mk" line 98: Unknown modifier "localtime"
-make: Unclosed variable expression, expecting '}' for modifier "localtime" of variable "" with value ""
-make: "varparse-errors.mk" line 98: Undefined variable "${:U:localtime"
+make: Unclosed variable expression, expecting '}' for modifier "gmtime" of variable "" with value "<timestamp>"
+make: Unclosed variable expression, expecting '}' for modifier "localtime" of variable "" with value "<timestamp>"
 make: Fatal errors encountered -- cannot continue
 make: stopped in unit-tests
 exit status 1
diff -r e2b0910b0297 -r 1b2a22faf15a usr.bin/make/unit-tests/varparse-errors.mk
--- a/usr.bin/make/unit-tests/varparse-errors.mk        Wed Aug 24 21:38:06 2022 +0000
+++ b/usr.bin/make/unit-tests/varparse-errors.mk        Wed Aug 24 22:09:40 2022 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: varparse-errors.mk,v 1.6 2022/08/24 21:38:06 rillig Exp $
+# $NetBSD: varparse-errors.mk,v 1.7 2022/08/24 22:09:41 rillig Exp $
 
 # Tests for parsing and evaluating all kinds of variable expressions.
 #
@@ -68,31 +68,21 @@
 _:=    ${:U:OX:U${IND}} ${:U:OX:U${IND}}
 #.MAKEFLAGS: -d0
 
-# expect+1: Unknown modifier "Q"
+
+# Before var.c 1.032 from 2022-08-24, make complained about 'Unknown modifier'
+# or 'Bad modifier' when in fact the modifier was entirely correct, it was
+# just not delimited by either ':' or '}' but instead by '\0'.
 UNCLOSED:=     ${:U:Q
-# expect+1: Unknown modifier "sh"
 UNCLOSED:=     ${:U:sh
-# expect: make: Bad modifier ":tA" for variable ""
 UNCLOSED:=     ${:U:tA
-# expect: make: Bad modifier ":tsX" for variable ""
 UNCLOSED:=     ${:U:tsX
-# expect: make: Bad modifier ":ts" for variable ""
 UNCLOSED:=     ${:U:ts
-# expect: make: Bad modifier ":ts\040" for variable ""
 UNCLOSED:=     ${:U:ts\040
-# expect+1: Unknown modifier "u"
 UNCLOSED:=     ${:U:u
-# expect+1: Unknown modifier "H"
 UNCLOSED:=     ${:U:H
-# expect: make: Bad modifier ":[1]" for variable ""
 UNCLOSED:=     ${:U:[1]
-# expect+1: Unknown modifier "hash"
 UNCLOSED:=     ${:U:hash
-# expect+1: Unknown modifier "range"
 UNCLOSED:=     ${:U:range
-# expect+1: Unknown modifier "_"
 UNCLOSED:=     ${:U:_
-# expect+1: Unknown modifier "gmtime"
 UNCLOSED:=     ${:U:gmtime
-# expect+1: Unknown modifier "localtime"
 UNCLOSED:=     ${:U:localtime
diff -r e2b0910b0297 -r 1b2a22faf15a usr.bin/make/var.c
--- a/usr.bin/make/var.c        Wed Aug 24 21:38:06 2022 +0000
+++ b/usr.bin/make/var.c        Wed Aug 24 22:09:40 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: var.c,v 1.1031 2022/08/24 21:03:57 rillig Exp $        */
+/*     $NetBSD: var.c,v 1.1032 2022/08/24 22:09:40 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -139,7 +139,7 @@
 #include "metachar.h"
 
 /*     "@(#)var.c      8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.1031 2022/08/24 21:03:57 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.1032 2022/08/24 22:09:40 rillig Exp $");
 
 /*
  * Variables are defined using one of the VAR=value assignments.  Their
@@ -2270,7 +2270,7 @@
 MAKE_INLINE bool
 IsDelimiter(char c, const ModChain *ch)
 {
-       return c == ':' || c == ch->endc;
+       return c == ':' || c == ch->endc || c == '\0';
 }
 
 /* Test whether mod starts with modname, followed by a delimiter. */
@@ -2453,7 +2453,7 @@
 
        p = *pp + 1;
        LazyBuf_Init(buf, p);
-       while (!IsDelimiter(*p, ch) && *p != '\0') {
+       while (!IsDelimiter(*p, ch)) {
 
                /*
                 * XXX: This code is similar to the one in Var_Parse. See if
@@ -3099,7 +3099,7 @@
        const char *mod = *pp;
        assert(mod[0] == 't');
 
-       if (IsDelimiter(mod[1], ch) || mod[1] == '\0') {
+       if (IsDelimiter(mod[1], ch)) {
                *pp = mod + 1;
                return AMR_BAD; /* Found ":t<endc>" or ":t:". */
        }
@@ -3107,7 +3107,7 @@
        if (mod[1] == 's')
                return ApplyModifier_ToSep(pp, ch);
 
-       if (!IsDelimiter(mod[2], ch)) {                 /* :t<unrecognized> */
+       if (!IsDelimiter(mod[2], ch)) {                 /* :t<any><any> */
                *pp = mod + 1;
                return AMR_BAD;
        }
@@ -3335,10 +3335,10 @@
        SubstringWords words;
        int (*cmp)(const void *, const void *);
 
-       if (IsDelimiter(mod[1], ch) || mod[1] == '\0') {
+       if (IsDelimiter(mod[1], ch)) {
                cmp = SubStrAsc;
                (*pp)++;
-       } else if (IsDelimiter(mod[2], ch) || mod[2] == '\0') {
+       } else if (IsDelimiter(mod[2], ch)) {
                if (mod[1] == 'n')
                        cmp = SubNumAsc;
                else if (mod[1] == 'r')
@@ -3348,7 +3348,7 @@
                else
                        goto bad;
                *pp += 2;
-       } else if (IsDelimiter(mod[3], ch) || mod[3] == '\0') {
+       } else if (IsDelimiter(mod[3], ch)) {
                if ((mod[1] == 'n' && mod[2] == 'r') ||
                    (mod[1] == 'r' && mod[2] == 'n'))
                        cmp = SubNumDesc;
@@ -3867,7 +3867,7 @@
        (void)Var_Parse(&p, expr->scope, expr->emode, &mods);
        /* TODO: handle errors */
 
-       if (mods.str[0] != '\0' && *p != '\0' && !IsDelimiter(*p, ch)) {
+       if (mods.str[0] != '\0' && !IsDelimiter(*p, ch)) {
                FStr_Done(&mods);
                return AMIR_SYSV;
        }
@@ -3926,7 +3926,7 @@
                 * errors and leads to wrong results.
                 * Parsing should rather stop here.
                 */
-               for (p++; !IsDelimiter(*p, ch) && *p != '\0'; p++)
+               for (p++; !IsDelimiter(*p, ch); p++)
                        continue;
                Parse_Error(PARSE_FATAL, "Unknown modifier \"%.*s\"",
                    (int)(p - mod), mod);