Module Name:    src
Committed By:   rillig
Date:           Tue Sep 21 22:38:25 UTC 2021

Modified Files:
        src/usr.bin/make: cond.c
        src/usr.bin/make/unit-tests: cond-token-plain.exp cond-token-plain.mk

Log Message:
make: do not allow unquoted 'left == right' after modifier ':?'

Having a static variable for state that clearly belongs in the parser
looked suspicious, and indeed it was wrong.

When the distinction between .if conditions and expressions of the form
${condition:?:} was added in cond.c 1.68 from 2015-05-05, a new unit
test was added, but it didn't cover this edge case.  At that time, the
state of the condition parser consisted of a few global variables
instead of a separate data type, as would have been appropriate for
parsing nested conditions.


To generate a diff of this commit:
cvs rdiff -u -r1.275 -r1.276 src/usr.bin/make/cond.c
cvs rdiff -u -r1.11 -r1.12 src/usr.bin/make/unit-tests/cond-token-plain.exp \
    src/usr.bin/make/unit-tests/cond-token-plain.mk

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/usr.bin/make/cond.c
diff -u src/usr.bin/make/cond.c:1.275 src/usr.bin/make/cond.c:1.276
--- src/usr.bin/make/cond.c:1.275	Tue Sep 21 21:43:32 2021
+++ src/usr.bin/make/cond.c	Tue Sep 21 22:38:25 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: cond.c,v 1.275 2021/09/21 21:43:32 rillig Exp $	*/
+/*	$NetBSD: cond.c,v 1.276 2021/09/21 22:38:25 rillig Exp $	*/
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -95,7 +95,7 @@
 #include "dir.h"
 
 /*	"@(#)cond.c	8.2 (Berkeley) 1/2/94"	*/
-MAKE_RCSID("$NetBSD: cond.c,v 1.275 2021/09/21 21:43:32 rillig Exp $");
+MAKE_RCSID("$NetBSD: cond.c,v 1.276 2021/09/21 22:38:25 rillig Exp $");
 
 /*
  * The parsing of conditional expressions is based on this grammar:
@@ -154,6 +154,20 @@ typedef struct CondParser {
 	bool (*evalBare)(size_t, const char *);
 	bool negateEvalBare;
 
+	/*
+	 * Whether the left-hand side of a comparison may NOT be an unquoted
+	 * string.  This is allowed for expressions of the form
+	 * ${condition:?:}, see ApplyModifier_IfElse.  Such a condition is
+	 * expanded before it is evaluated, due to ease of implementation.
+	 * This means that at the point where the condition is evaluated,
+	 * make cannot know anymore whether the left-hand side had originally
+	 * been a variable expression or a plain word.
+	 *
+	 * In all other contexts, the left-hand side must either be a
+	 * variable expression, a quoted string or a number.
+	 */
+	bool lhsStrict;
+
 	const char *p;		/* The remaining condition to parse */
 	Token curr;		/* Single push-back token used in parsing */
 
@@ -174,18 +188,6 @@ static unsigned int cond_min_depth = 0;	
 /* Names for ComparisonOp. */
 static const char *opname[] = { "<", "<=", ">", ">=", "==", "!=" };
 
-/*
- * Indicate when we should be strict about lhs of comparisons.
- * In strict mode, the lhs must be a variable expression or a string literal
- * in quotes. In non-strict mode it may also be an unquoted string literal.
- *
- * True when CondEvalExpression is called from Cond_EvalLine (.if etc).
- * False when CondEvalExpression is called from ApplyModifier_IfElse
- * since lhs is already expanded, and at that point we cannot tell if
- * it was a variable reference or not.
- */
-static bool lhsStrict;
-
 static bool
 is_token(const char *str, const char *tok, size_t len)
 {
@@ -682,7 +684,7 @@ CondParser_Comparison(CondParser *par, b
 	ComparisonOp op;
 	bool lhsQuoted, rhsQuoted;
 
-	CondParser_Leaf(par, doEval, lhsStrict, &lhs, &lhsQuoted);
+	CondParser_Leaf(par, doEval, par->lhsStrict, &lhs, &lhsQuoted);
 	if (lhs.str == NULL)
 		goto done_lhs;
 
@@ -1063,13 +1065,12 @@ CondEvalExpression(const char *cond, boo
 	CondParser par;
 	CondEvalResult rval;
 
-	lhsStrict = strictLHS;
-
 	cpp_skip_hspace(&cond);
 
 	par.plain = plain;
 	par.evalBare = evalBare;
 	par.negateEvalBare = negate;
+	par.lhsStrict = strictLHS;
 	par.p = cond;
 	par.curr = TOK_NONE;
 	par.printedError = false;

Index: src/usr.bin/make/unit-tests/cond-token-plain.exp
diff -u src/usr.bin/make/unit-tests/cond-token-plain.exp:1.11 src/usr.bin/make/unit-tests/cond-token-plain.exp:1.12
--- src/usr.bin/make/unit-tests/cond-token-plain.exp:1.11	Tue Sep 21 21:59:56 2021
+++ src/usr.bin/make/unit-tests/cond-token-plain.exp	Tue Sep 21 22:38:25 2021
@@ -53,9 +53,9 @@ CondParser_Eval: left == right
 make: "cond-token-plain.mk" line 191: Malformed conditional (left == right)
 CondParser_Eval: ${0:?:} || left == right
 CondParser_Eval: 0
-lhs = "left", rhs = "right", op = ==
+make: "cond-token-plain.mk" line 197: Malformed conditional (${0:?:} || left == right)
 CondParser_Eval: left == right || ${0:?:}
-make: "cond-token-plain.mk" line 201: Malformed conditional (left == right || ${0:?:})
+make: "cond-token-plain.mk" line 202: Malformed conditional (left == right || ${0:?:})
 make: Fatal errors encountered -- cannot continue
 make: stopped in unit-tests
 exit status 1
Index: src/usr.bin/make/unit-tests/cond-token-plain.mk
diff -u src/usr.bin/make/unit-tests/cond-token-plain.mk:1.11 src/usr.bin/make/unit-tests/cond-token-plain.mk:1.12
--- src/usr.bin/make/unit-tests/cond-token-plain.mk:1.11	Tue Sep 21 21:59:56 2021
+++ src/usr.bin/make/unit-tests/cond-token-plain.mk	Tue Sep 21 22:38:25 2021
@@ -1,4 +1,4 @@
-# $NetBSD: cond-token-plain.mk,v 1.11 2021/09/21 21:59:56 rillig Exp $
+# $NetBSD: cond-token-plain.mk,v 1.12 2021/09/21 22:38:25 rillig Exp $
 #
 # Tests for plain tokens (that is, string literals without quotes)
 # in .if conditions.
@@ -193,6 +193,7 @@ ${:U\\\\}=	backslash
 # Before cond.c 1.276 from 2021-09-21, a variable expression containing the
 # modifier ':?:' allowed unquoted string literals for the rest of the
 # condition.  This was an unintended implementation mistake.
+# expect+1: Malformed conditional (${0:?:} || left == right)
 .if ${0:?:} || left == right
 .endif
 # This affected only the comparisons after the expression, so the following

Reply via email to