Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/make make: fix parsing of modifiers containing unbal...



details:   https://anonhg.NetBSD.org/src/rev/51919ccdd136
branches:  trunk
changeset: 368859:51919ccdd136
user:      rillig <rillig%NetBSD.org@localhost>
date:      Mon Aug 08 18:23:30 2022 +0000

description:
make: fix parsing of modifiers containing unbalanced subexpressions

diffstat:

 usr.bin/make/unit-tests/parse-var.exp            |   6 +--
 usr.bin/make/unit-tests/parse-var.mk             |  35 +++++++++------
 usr.bin/make/unit-tests/var-eval-short.exp       |   4 +
 usr.bin/make/unit-tests/varmod-defined.exp       |   1 +
 usr.bin/make/unit-tests/varname-dot-suffixes.exp |   1 +
 usr.bin/make/var.c                               |  56 ++++++++++-------------
 6 files changed, 52 insertions(+), 51 deletions(-)

diffs (212 lines):

diff -r 68eba3613481 -r 51919ccdd136 usr.bin/make/unit-tests/parse-var.exp
--- a/usr.bin/make/unit-tests/parse-var.exp     Mon Aug 08 16:50:35 2022 +0000
+++ b/usr.bin/make/unit-tests/parse-var.exp     Mon Aug 08 18:23:30 2022 +0000
@@ -1,5 +1,1 @@
-make: Unfinished modifier for "BRACE_GROUP" (',' missing)
-make: "parse-var.mk" line 47: Malformed conditional (0 && ${BRACE_GROUP:S,${BRACE_PAIR:S,{,{{,},<lbraces>,})
-make: Fatal errors encountered -- cannot continue
-make: stopped in unit-tests
-exit status 1
+exit status 0
diff -r 68eba3613481 -r 51919ccdd136 usr.bin/make/unit-tests/parse-var.mk
--- a/usr.bin/make/unit-tests/parse-var.mk      Mon Aug 08 16:50:35 2022 +0000
+++ b/usr.bin/make/unit-tests/parse-var.mk      Mon Aug 08 18:23:30 2022 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: parse-var.mk,v 1.2 2022/08/06 21:26:05 rillig Exp $
+# $NetBSD: parse-var.mk,v 1.3 2022/08/08 18:23:30 rillig Exp $
 #
 # Tests for parsing variable expressions.
 
@@ -14,18 +14,23 @@
 .endif
 
 
-# As of var.c 1.1027 from 2022-08-05, the exact way of parsing an expression
-# depends on whether the expression is actually evaluated or merely parsed.
+# Before var.c 1.1028 from 2022-08-08, the exact way of parsing an expression
+# depended on whether the expression was actually evaluated or merely parsed.
+#
+# If it was evaluated, nested expressions were parsed correctly, parsing each
+# modifier according to its exact definition (see varmod.mk).
 #
-# If it is evaluated, nested expressions are parsed correctly, parsing each
-# modifier according to its exact definition.  If the expression is merely
-# parsed but not evaluated (because its value would not influence the outcome
-# of the condition), and the expression contains a modifier, and that modifier
-# contains a nested expression, the nested expression is not parsed
-# correctly.  Instead, make only counts the opening and closing delimiters,
-# which fails for nested modifiers with unbalanced braces.
+# If the expression was merely parsed but not evaluated (for example, because
+# its value would not influence the outcome of the condition, or during the
+# first pass of the ':@var@body@' modifier), and the expression contained a
+# modifier, and that modifier contained a nested expression, the nested
+# expression was not parsed correctly.  Instead, make only counted the opening
+# and closing delimiters, which failed for nested modifiers with unbalanced
+# braces.
 #
-# See ParseModifierPartDollar.
+# This naive brace counting was implemented in ParseModifierPartDollar.  As of
+# var.c 1., there are still several other places that merely count braces
+# instead of properly parsing subexpressions.
 
 #.MAKEFLAGS: -dcpv
 # Keep these braces outside the conditions below, to keep them simple to
@@ -41,9 +46,11 @@
 # In the first case, the outer expression is relevant and is parsed correctly.
 .if 1 && ${BRACE_GROUP:S,${BRACE_PAIR:S,{,{{,},<lbraces>,}
 .endif
-# In the second case, the outer expression is irrelevant.  In this case, in
-# the parts of the outer ':S' modifier, make only counts the braces, and since
-# the inner expression '${:U...}' contains more '{' than '}', parsing fails.
+# In the second case, the outer expression was irrelevant.  In this case, in
+# the parts of the outer ':S' modifier, make only counted the braces, and since
+# the inner expression '${BRACE_PAIR:...}' contains more '{' than '}', parsing
+# failed with the error message 'Unfinished modifier for "BRACE_GROUP"'.  Fixed
+# in var.c 1.1028 from 2022-08-08.
 .if 0 && ${BRACE_GROUP:S,${BRACE_PAIR:S,{,{{,},<lbraces>,}
 .endif
 #.MAKEFLAGS: -d0
diff -r 68eba3613481 -r 51919ccdd136 usr.bin/make/unit-tests/var-eval-short.exp
--- a/usr.bin/make/unit-tests/var-eval-short.exp        Mon Aug 08 16:50:35 2022 +0000
+++ b/usr.bin/make/unit-tests/var-eval-short.exp        Mon Aug 08 18:23:30 2022 +0000
@@ -7,7 +7,9 @@
 CondParser_Eval: 0 && ${0:?${FAIL}then:${FAIL}else}
 Var_Parse: ${0:?${FAIL}then:${FAIL}else} (parse-only)
 Parsing modifier ${0:?...}
+Var_Parse: ${FAIL}then:${FAIL}else} (parse-only)
 Modifier part: "${FAIL}then"
+Var_Parse: ${FAIL}else} (parse-only)
 Modifier part: "${FAIL}else"
 Result of ${0:?${FAIL}then:${FAIL}else} is "" (parse-only, defined)
 Parsing line 163: DEFINED=     defined
@@ -17,7 +19,9 @@
 Parsing modifier ${DEFINED:L}
 Result of ${DEFINED:L} is "defined" (parse-only, regular)
 Parsing modifier ${DEFINED:?...}
+Var_Parse: ${FAIL}then:${FAIL}else} (parse-only)
 Modifier part: "${FAIL}then"
+Var_Parse: ${FAIL}else} (parse-only)
 Modifier part: "${FAIL}else"
 Result of ${DEFINED:?${FAIL}then:${FAIL}else} is "defined" (parse-only, regular)
 Parsing line 166: .MAKEFLAGS: -d0
diff -r 68eba3613481 -r 51919ccdd136 usr.bin/make/unit-tests/varmod-defined.exp
--- a/usr.bin/make/unit-tests/varmod-defined.exp        Mon Aug 08 16:50:35 2022 +0000
+++ b/usr.bin/make/unit-tests/varmod-defined.exp        Mon Aug 08 18:23:30 2022 +0000
@@ -10,6 +10,7 @@
 Var_Parse: ${VAR:@var@${8_DOLLARS}@} (eval-keep-dollar-and-undefined)
 Evaluating modifier ${VAR:@...} on value "$$$$$$$$" (eval-keep-dollar-and-undefined, regular)
 Modifier part: "var"
+Var_Parse: ${8_DOLLARS}@} (parse-only)
 Modifier part: "${8_DOLLARS}"
 ModifyWords: split "$$$$$$$$" into 1 word
 Global: var = $$$$$$$$
diff -r 68eba3613481 -r 51919ccdd136 usr.bin/make/unit-tests/varname-dot-suffixes.exp
--- a/usr.bin/make/unit-tests/varname-dot-suffixes.exp  Mon Aug 08 16:50:35 2022 +0000
+++ b/usr.bin/make/unit-tests/varname-dot-suffixes.exp  Mon Aug 08 18:23:30 2022 +0000
@@ -24,6 +24,7 @@
 Result of ${1 2:L} is "1 2" (eval-defined, defined)
 Evaluating modifier ${1 2:@...} on value "1 2" (eval-defined, defined)
 Modifier part: ".SUFFIXES"
+Var_Parse: ${.SUFFIXES}@} != ".c .o .1 .err .tar.gz .c .o .1 .err .tar.gz" (parse-only)
 Modifier part: "${.SUFFIXES}"
 ModifyWords: split "1 2" into 2 words
 Command: .SUFFIXES = 1 ignored (read-only)
diff -r 68eba3613481 -r 51919ccdd136 usr.bin/make/var.c
--- a/usr.bin/make/var.c        Mon Aug 08 16:50:35 2022 +0000
+++ b/usr.bin/make/var.c        Mon Aug 08 18:23:30 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: var.c,v 1.1027 2022/08/05 20:59:54 rillig Exp $        */
+/*     $NetBSD: var.c,v 1.1028 2022/08/08 18:23:30 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.1027 2022/08/05 20:59:54 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.1028 2022/08/08 18:23:30 rillig Exp $");
 
 /*
  * Variables are defined using one of the VAR=value assignments.  Their
@@ -2131,43 +2131,33 @@
        *pp = p;
 }
 
-/*
- * In a part of a modifier, parse a subexpression but don't evaluate it.
- *
- * XXX: This whole block is very similar to Var_Parse with VARE_PARSE_ONLY.
- * There may be subtle edge cases though that are not yet covered in the unit
- * tests and that are parsed differently, depending on whether they are
- * evaluated or not.
- *
- * This subtle difference is not documented in the manual page, neither is
- * the difference between parsing ':D' and ':M' documented.  No code should
- * ever depend on these details, but who knows.
- */
+/* In a part of a modifier, parse a subexpression but don't evaluate it. */
 static void
 ParseModifierPartDollar(const char **pp, LazyBuf *part)
 {
        const char *p = *pp;
-       const char *start = *pp;
 
        if (p[1] == '(' || p[1] == '{') {
-               char startc = p[1];
-               int endc = startc == '(' ? ')' : '}';
-               int depth = 1;
-
-               for (p += 2; *p != '\0' && depth > 0; p++) {
-                       if (p[-1] != '\\') {
-                               if (*p == startc)
-                                       depth++;
-                               if (*p == endc)
-                                       depth--;
-                       }
-               }
-               LazyBuf_AddBytesBetween(part, start, p);
-               *pp = p;
+               FStr unused;
+               Var_Parse(&p, SCOPE_GLOBAL, VARE_PARSE_ONLY, &unused);
+               /* TODO: handle errors */
+               FStr_Done(&unused);
        } else {
-               LazyBuf_Add(part, *start);
-               *pp = p + 1;
+               /*
+                * Only skip the '$' but not the next character; see
+                * ParseModifierPartSubst, the case for "Unescaped '$' at
+                * end", which also doesn't skip '$' + delimiter.  That is a
+                * hack as well, but for now it's consistent in both cases.
+                */
+               p++;
        }
+
+       /*
+        * XXX: There should be no need to add anything to the buffer, as it
+        * will be discarded anyway.
+        */
+       LazyBuf_AddBytesBetween(part, *pp, p);
+       *pp = p;
 }
 
 /* See ParseModifierPart for the documentation. */
@@ -4498,6 +4488,8 @@
        if (Var_Parse_FastLane(pp, emode, out_val))
                return VPR_OK;
 
+       /* TODO: Reduce computations in parse-only mode. */
+
        DEBUG2(VAR, "Var_Parse: %s (%s)\n", start, VarEvalMode_Name[emode]);
 
        *out_val = FStr_InitRefer(NULL);
@@ -4524,7 +4516,7 @@
        }
 
        expr.name = v->name.str;
-       if (v->inUse) {
+       if (v->inUse && VarEvalMode_ShouldEval(emode)) {
                if (scope->fname != NULL) {
                        fprintf(stderr, "In a command near ");
                        PrintLocation(stderr, false, scope);



Home | Main Index | Thread Index | Old Index