[src/trunk]: src/usr.bin/make When evaluating condtionals from .if we want to...

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

details:   https://anonhg.NetBSD.org/src/rev/194ce511bfae
branches:  trunk
changeset: 337976:194ce511bfae
user:      sjg <sjg%NetBSD.org@localhost>
date:      Tue May 05 21:51:09 2015 +0000

description:
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

diffstat:

 usr.bin/make/cond.c               |  37 +++++++++++++++++++++++++++++--------
 usr.bin/make/nonints.h            |   4 ++--
 usr.bin/make/unit-tests/Makefile  |   3 ++-
 usr.bin/make/unit-tests/cond2.exp |   7 +++++++
 usr.bin/make/unit-tests/cond2.mk  |  25 +++++++++++++++++++++++++
 usr.bin/make/var.c                |   8 ++++----
 6 files changed, 69 insertions(+), 15 deletions(-)

diffs (223 lines):

diff -r 9ca5619f59eb -r 194ce511bfae usr.bin/make/cond.c
--- a/usr.bin/make/cond.c       Tue May 05 17:03:18 2015 +0000
+++ b/usr.bin/make/cond.c       Tue May 05 21:51:09 2015 +0000
@@ -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 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 @@
  */
 /* 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 @@
            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 @@
      * 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 @@
        goto done;
     }
 
-    rhs = CondGetString(doEval, &rhsQuoted, &rhsFree);
+    rhs = CondGetString(doEval, &rhsQuoted, &rhsFree, FALSE);
     if (!rhs)
        goto done;
 
@@ -1135,7 +1154,7 @@
  *-----------------------------------------------------------------------
  */
 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 @@
     Token sv_condPushBack = condPushBack;
     int rval;
 
+    lhsStrict = strictLHS;
+
     while (*line == ' ' || *line == '\t')
        line++;
 
@@ -1359,7 +1380,7 @@
     }
 
     /* 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;
diff -r 9ca5619f59eb -r 194ce511bfae usr.bin/make/nonints.h
--- a/usr.bin/make/nonints.h    Tue May 05 17:03:18 2015 +0000
+++ b/usr.bin/make/nonints.h    Tue May 05 21:51:09 2015 +0000
@@ -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 @@
 
 /* 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);
diff -r 9ca5619f59eb -r 194ce511bfae usr.bin/make/unit-tests/Makefile
--- a/usr.bin/make/unit-tests/Makefile  Tue May 05 17:03:18 2015 +0000
+++ b/usr.bin/make/unit-tests/Makefile  Tue May 05 21:51:09 2015 +0000
@@ -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 @@
 TESTNAMES= \
        comment \
        cond1 \
+       cond2 \
        error \
        export \
        export-all \
diff -r 9ca5619f59eb -r 194ce511bfae usr.bin/make/unit-tests/cond2.exp
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/usr.bin/make/unit-tests/cond2.exp Tue May 05 21:51:09 2015 +0000
@@ -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
diff -r 9ca5619f59eb -r 194ce511bfae usr.bin/make/unit-tests/cond2.mk
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/usr.bin/make/unit-tests/cond2.mk  Tue May 05 21:51:09 2015 +0000
@@ -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 $@
diff -r 9ca5619f59eb -r 194ce511bfae usr.bin/make/var.c
--- a/usr.bin/make/var.c        Tue May 05 17:03:18 2015 +0000
+++ b/usr.bin/make/var.c        Tue May 05 21:51:09 2015 +0000
@@ -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 @@
 
                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);