Module Name: src
Committed By: sjg
Date: Tue May 5 21:51:09 UTC 2015
Modified Files:
src/usr.bin/make: cond.c nonints.h var.c
src/usr.bin/make/unit-tests: Makefile
Added Files:
src/usr.bin/make/unit-tests: cond2.exp cond2.mk
Log Message:
When evaluating condtionals from .if we want to require
that the lhs is a variable reference, a number or a quoted string.
This helps avoid subtle bugs caused by typos.
When conditionals are being evaluated during variable expansion
we cannot be as strict becuase lhs will already have been expanded.
We therefor pass a boolean to Cond_EvalExpression to tell it how
lhs should be treated.
Add unit-tests/cond2.mk to test the above
Reviewed by: christos, joerg
To generate a diff of this commit:
cvs rdiff -u -r1.67 -r1.68 src/usr.bin/make/cond.c src/usr.bin/make/nonints.h
cvs rdiff -u -r1.191 -r1.192 src/usr.bin/make/var.c
cvs rdiff -u -r1.51 -r1.52 src/usr.bin/make/unit-tests/Makefile
cvs rdiff -u -r0 -r1.1 src/usr.bin/make/unit-tests/cond2.exp \
src/usr.bin/make/unit-tests/cond2.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.67 src/usr.bin/make/cond.c:1.68
--- src/usr.bin/make/cond.c:1.67 Sat Nov 3 13:59:27 2012
+++ src/usr.bin/make/cond.c Tue May 5 21:51:09 2015
@@ -1,4 +1,4 @@
-/* $NetBSD: cond.c,v 1.67 2012/11/03 13:59:27 christos Exp $ */
+/* $NetBSD: cond.c,v 1.68 2015/05/05 21:51:09 sjg Exp $ */
/*
* Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -70,14 +70,14 @@
*/
#ifndef MAKE_NATIVE
-static char rcsid[] = "$NetBSD: cond.c,v 1.67 2012/11/03 13:59:27 christos Exp $";
+static char rcsid[] = "$NetBSD: cond.c,v 1.68 2015/05/05 21:51:09 sjg Exp $";
#else
#include <sys/cdefs.h>
#ifndef lint
#if 0
static char sccsid[] = "@(#)cond.c 8.2 (Berkeley) 1/2/94";
#else
-__RCSID("$NetBSD: cond.c,v 1.67 2012/11/03 13:59:27 christos Exp $");
+__RCSID("$NetBSD: cond.c,v 1.68 2015/05/05 21:51:09 sjg Exp $");
#endif
#endif /* not lint */
#endif
@@ -181,6 +181,15 @@ static Token condPushBack=TOK_NONE; /*
static unsigned int cond_depth = 0; /* current .if nesting level */
static unsigned int cond_min_depth = 0; /* depth at makefile open */
+/*
+ * Indicate when we should be strict about lhs of comparisons.
+ * TRUE when Cond_EvalExpression is called from Cond_Eval (.if etc)
+ * FALSE when Cond_EvalExpression is called from var.c:ApplyModifiers
+ * since lhs is already expanded and we cannot tell if
+ * it was a variable reference or not.
+ */
+static Boolean lhsStrict;
+
static int
istoken(const char *str, const char *tok, size_t len)
{
@@ -517,7 +526,7 @@ CondCvtArg(char *str, double *value)
*/
/* coverity:[+alloc : arg-*2] */
static char *
-CondGetString(Boolean doEval, Boolean *quoted, void **freeIt)
+CondGetString(Boolean doEval, Boolean *quoted, void **freeIt, Boolean strictLHS)
{
Buffer buf;
char *cp;
@@ -601,6 +610,16 @@ CondGetString(Boolean doEval, Boolean *q
condExpr--; /* don't skip over next char */
break;
default:
+ if (strictLHS && !qt && *start != '$' &&
+ !isdigit((unsigned char) *start)) {
+ /* lhs must be quoted, a variable reference or number */
+ if (*freeIt) {
+ free(*freeIt);
+ *freeIt = NULL;
+ }
+ str = NULL;
+ goto cleanup;
+ }
Buf_AddByte(&buf, *condExpr);
break;
}
@@ -648,7 +667,7 @@ compare_expression(Boolean doEval)
* Parse the variable spec and skip over it, saving its
* value in lhs.
*/
- lhs = CondGetString(doEval, &lhsQuoted, &lhsFree);
+ lhs = CondGetString(doEval, &lhsQuoted, &lhsFree, lhsStrict);
if (!lhs)
goto done;
@@ -709,7 +728,7 @@ compare_expression(Boolean doEval)
goto done;
}
- rhs = CondGetString(doEval, &rhsQuoted, &rhsFree);
+ rhs = CondGetString(doEval, &rhsQuoted, &rhsFree, FALSE);
if (!rhs)
goto done;
@@ -1135,7 +1154,7 @@ CondE(Boolean doEval)
*-----------------------------------------------------------------------
*/
int
-Cond_EvalExpression(const struct If *info, char *line, Boolean *value, int eprint)
+Cond_EvalExpression(const struct If *info, char *line, Boolean *value, int eprint, Boolean strictLHS)
{
static const struct If *dflt_info;
const struct If *sv_if_info = if_info;
@@ -1143,6 +1162,8 @@ Cond_EvalExpression(const struct If *inf
Token sv_condPushBack = condPushBack;
int rval;
+ lhsStrict = strictLHS;
+
while (*line == ' ' || *line == '\t')
line++;
@@ -1359,7 +1380,7 @@ Cond_Eval(char *line)
}
/* And evaluate the conditional expresssion */
- if (Cond_EvalExpression(ifp, line, &value, 1) == COND_INVALID) {
+ if (Cond_EvalExpression(ifp, line, &value, 1, TRUE) == COND_INVALID) {
/* Syntax error in conditional, error message already output. */
/* Skip everything to matching .endif */
cond_state[cond_depth] = SKIP_TO_ELSE;
Index: src/usr.bin/make/nonints.h
diff -u src/usr.bin/make/nonints.h:1.67 src/usr.bin/make/nonints.h:1.68
--- src/usr.bin/make/nonints.h:1.67 Sun Sep 7 20:55:34 2014
+++ src/usr.bin/make/nonints.h Tue May 5 21:51:09 2015
@@ -1,4 +1,4 @@
-/* $NetBSD: nonints.h,v 1.67 2014/09/07 20:55:34 joerg Exp $ */
+/* $NetBSD: nonints.h,v 1.68 2015/05/05 21:51:09 sjg Exp $ */
/*-
* Copyright (c) 1988, 1989, 1990, 1993
@@ -91,7 +91,7 @@ int Compat_Make(void *, void *);
/* cond.c */
struct If;
-int Cond_EvalExpression(const struct If *, char *, Boolean *, int);
+int Cond_EvalExpression(const struct If *, char *, Boolean *, int, Boolean);
int Cond_Eval(char *);
void Cond_restore_depth(unsigned int);
unsigned int Cond_save_depth(void);
Index: src/usr.bin/make/var.c
diff -u src/usr.bin/make/var.c:1.191 src/usr.bin/make/var.c:1.192
--- src/usr.bin/make/var.c:1.191 Sun Sep 14 02:32:51 2014
+++ src/usr.bin/make/var.c Tue May 5 21:51:09 2015
@@ -1,4 +1,4 @@
-/* $NetBSD: var.c,v 1.191 2014/09/14 02:32:51 dholland Exp $ */
+/* $NetBSD: var.c,v 1.192 2015/05/05 21:51:09 sjg Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -69,14 +69,14 @@
*/
#ifndef MAKE_NATIVE
-static char rcsid[] = "$NetBSD: var.c,v 1.191 2014/09/14 02:32:51 dholland Exp $";
+static char rcsid[] = "$NetBSD: var.c,v 1.192 2015/05/05 21:51:09 sjg Exp $";
#else
#include <sys/cdefs.h>
#ifndef lint
#if 0
static char sccsid[] = "@(#)var.c 8.3 (Berkeley) 3/19/94";
#else
-__RCSID("$NetBSD: var.c,v 1.191 2014/09/14 02:32:51 dholland Exp $");
+__RCSID("$NetBSD: var.c,v 1.192 2015/05/05 21:51:09 sjg Exp $");
#endif
#endif /* not lint */
#endif
@@ -3260,7 +3260,7 @@ ApplyModifiers(char *nstr, const char *t
termc = *--cp;
delim = '\0';
- if (Cond_EvalExpression(NULL, v->name, &value, 0)
+ if (Cond_EvalExpression(NULL, v->name, &value, 0, FALSE)
== COND_INVALID) {
Error("Bad conditional expression `%s' in %s?%s:%s",
v->name, v->name, pattern.lhs, pattern.rhs);
Index: src/usr.bin/make/unit-tests/Makefile
diff -u src/usr.bin/make/unit-tests/Makefile:1.51 src/usr.bin/make/unit-tests/Makefile:1.52
--- src/usr.bin/make/unit-tests/Makefile:1.51 Mon Oct 20 23:21:11 2014
+++ src/usr.bin/make/unit-tests/Makefile Tue May 5 21:51:09 2015
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.51 2014/10/20 23:21:11 sjg Exp $
+# $NetBSD: Makefile,v 1.52 2015/05/05 21:51:09 sjg Exp $
#
# Unit tests for make(1)
# The main targets are:
@@ -23,6 +23,7 @@ UNIT_TESTS:= ${.PARSEDIR}
TESTNAMES= \
comment \
cond1 \
+ cond2 \
error \
export \
export-all \
Added files:
Index: src/usr.bin/make/unit-tests/cond2.exp
diff -u /dev/null src/usr.bin/make/unit-tests/cond2.exp:1.1
--- /dev/null Tue May 5 21:51:09 2015
+++ src/usr.bin/make/unit-tests/cond2.exp Tue May 5 21:51:09 2015
@@ -0,0 +1,7 @@
+make: Bad conditional expression ` == "empty"' in == "empty"?oops:ok
+make: "cond2.mk" line 13: Malformed conditional ({TEST_TYPO} == "Ok")
+TEST_NOT_SET is empty or not defined
+make: "cond2.mk" line 20: Malformed conditional (${TEST_NOT_SET} == "empty")
+make: Fatal errors encountered -- cannot continue
+make: stopped in unit-tests
+exit status 1
Index: src/usr.bin/make/unit-tests/cond2.mk
diff -u /dev/null src/usr.bin/make/unit-tests/cond2.mk:1.1
--- /dev/null Tue May 5 21:51:09 2015
+++ src/usr.bin/make/unit-tests/cond2.mk Tue May 5 21:51:09 2015
@@ -0,0 +1,25 @@
+# $Id: cond2.mk,v 1.1 2015/05/05 21:51:09 sjg Exp $
+
+TEST_UNAME_S= NetBSD
+
+# this should be ok
+X:= ${${TEST_UNAME_S} == "NetBSD":?Ok:fail}
+.if $X == "Ok"
+Y= good
+.endif
+# expect: Bad conditional expression ` == "empty"' in == "empty"?oops:ok
+X:= ${${TEST_NOT_SET} == "empty":?oops:ok}
+# expect: Malformed conditional ({TEST_TYPO} == "Ok")
+.if {TEST_TYPO} == "Ok"
+Y= oops
+.endif
+.if empty(TEST_NOT_SET)
+Y!= echo TEST_NOT_SET is empty or not defined >&2; echo
+.endif
+# expect: Malformed conditional (${TEST_NOT_SET} == "empty")
+.if ${TEST_NOT_SET} == "empty"
+Y= oops
+.endif
+
+all:
+ @echo $@
