Module Name: src
Committed By: rillig
Date: Sun Jun 28 11:06:27 UTC 2020
Modified Files:
src/usr.bin/make: cond.c
src/usr.bin/make/unit-tests: cond-short.exp cond-short.mk
Log Message:
make(1): fix evaluation of unreachable conditions
Since 2015-10-11, make had evaluated unreachable conditions even though
the manual page said it didn't.
To generate a diff of this commit:
cvs rdiff -u -r1.75 -r1.76 src/usr.bin/make/cond.c
cvs rdiff -u -r1.1 -r1.2 src/usr.bin/make/unit-tests/cond-short.exp \
src/usr.bin/make/unit-tests/cond-short.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.75 src/usr.bin/make/cond.c:1.76
--- src/usr.bin/make/cond.c:1.75 Sun Apr 16 20:59:04 2017
+++ src/usr.bin/make/cond.c Sun Jun 28 11:06:26 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: cond.c,v 1.75 2017/04/16 20:59:04 riastradh Exp $ */
+/* $NetBSD: cond.c,v 1.76 2020/06/28 11:06:26 rillig 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.75 2017/04/16 20:59:04 riastradh Exp $";
+static char rcsid[] = "$NetBSD: cond.c,v 1.76 2020/06/28 11:06:26 rillig 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.75 2017/04/16 20:59:04 riastradh Exp $");
+__RCSID("$NetBSD: cond.c,v 1.76 2020/06/28 11:06:26 rillig Exp $");
#endif
#endif /* not lint */
#endif
@@ -146,7 +146,7 @@ typedef enum {
* last two fields are stored in condInvert and condDefProc, respectively.
*/
static void CondPushBack(Token);
-static int CondGetArg(char **, char **, const char *);
+static int CondGetArg(Boolean, char **, char **, const char *);
static Boolean CondDoDefined(int, const char *);
static int CondStrMatch(const void *, const void *);
static Boolean CondDoMake(int, const char *);
@@ -225,9 +225,6 @@ CondPushBack(Token t)
* CondGetArg --
* Find the argument of a built-in function.
*
- * Input:
- * parens TRUE if arg should be bounded by parens
- *
* Results:
* The length of the argument and the address of the argument.
*
@@ -238,7 +235,7 @@ CondPushBack(Token t)
*-----------------------------------------------------------------------
*/
static int
-CondGetArg(char **linePtr, char **argPtr, const char *func)
+CondGetArg(Boolean doEval, char **linePtr, char **argPtr, const char *func)
{
char *cp;
int argLen;
@@ -290,7 +287,8 @@ CondGetArg(char **linePtr, char **argPtr
int len;
void *freeIt;
- cp2 = Var_Parse(cp, VAR_CMD, VARF_UNDEFERR|VARF_WANTRES,
+ cp2 = Var_Parse(cp, VAR_CMD, VARF_UNDEFERR|
+ (doEval ? VARF_WANTRES : 0),
&len, &freeIt);
Buf_AddBytes(&buf, strlen(cp2), cp2);
free(freeIt);
@@ -577,7 +575,7 @@ CondGetString(Boolean doEval, Boolean *q
/* if we are in quotes, then an undefined variable is ok */
str = Var_Parse(condExpr, VAR_CMD,
((!qt && doEval) ? VARF_UNDEFERR : 0) |
- VARF_WANTRES, &len, freeIt);
+ (doEval ? VARF_WANTRES : 0), &len, freeIt);
if (str == var_Error) {
if (*freeIt) {
free(*freeIt);
@@ -813,7 +811,7 @@ done:
}
static int
-get_mpt_arg(char **linePtr, char **argPtr, const char *func MAKE_ATTR_UNUSED)
+get_mpt_arg(Boolean doEval, char **linePtr, char **argPtr, const char *func MAKE_ATTR_UNUSED)
{
/*
* Use Var_Parse to parse the spec in parens and return
@@ -827,7 +825,7 @@ get_mpt_arg(char **linePtr, char **argPt
/* We do all the work here and return the result as the length */
*argPtr = NULL;
- val = Var_Parse(cp - 1, VAR_CMD, VARF_WANTRES, &length, &freeIt);
+ val = Var_Parse(cp - 1, VAR_CMD, doEval ? VARF_WANTRES : 0, &length, &freeIt);
/*
* Advance *linePtr to beyond the closing ). Note that
* we subtract one because 'length' is calculated from 'cp - 1'.
@@ -864,7 +862,7 @@ compare_function(Boolean doEval)
static const struct fn_def {
const char *fn_name;
int fn_name_len;
- int (*fn_getarg)(char **, char **, const char *);
+ int (*fn_getarg)(Boolean, char **, char **, const char *);
Boolean (*fn_proc)(int, const char *);
} fn_defs[] = {
{ "defined", 7, CondGetArg, CondDoDefined },
@@ -892,7 +890,7 @@ compare_function(Boolean doEval)
if (*cp != '(')
break;
- arglen = fn_def->fn_getarg(&cp, &arg, fn_def->fn_name);
+ arglen = fn_def->fn_getarg(doEval, &cp, &arg, fn_def->fn_name);
if (arglen <= 0) {
condExpr = cp;
return arglen < 0 ? TOK_ERROR : TOK_FALSE;
@@ -917,7 +915,7 @@ compare_function(Boolean doEval)
* would be invalid if we did "defined(a)" - so instead treat as an
* expression.
*/
- arglen = CondGetArg(&cp, &arg, NULL);
+ arglen = CondGetArg(doEval, &cp, &arg, NULL);
for (cp1 = cp; isspace(*(unsigned char *)cp1); cp1++)
continue;
if (*cp1 == '=' || *cp1 == '!')
Index: src/usr.bin/make/unit-tests/cond-short.exp
diff -u src/usr.bin/make/unit-tests/cond-short.exp:1.1 src/usr.bin/make/unit-tests/cond-short.exp:1.2
--- src/usr.bin/make/unit-tests/cond-short.exp:1.1 Sun Jun 28 09:42:40 2020
+++ src/usr.bin/make/unit-tests/cond-short.exp Sun Jun 28 11:06:26 2020
@@ -1,5 +1,7 @@
-unexpected and
expected and
-unexpected or
+expected and exists
+expected and empty
expected or
+expected or exists
+expected or empty
exit status 0
Index: src/usr.bin/make/unit-tests/cond-short.mk
diff -u src/usr.bin/make/unit-tests/cond-short.mk:1.1 src/usr.bin/make/unit-tests/cond-short.mk:1.2
--- src/usr.bin/make/unit-tests/cond-short.mk:1.1 Sun Jun 28 09:42:40 2020
+++ src/usr.bin/make/unit-tests/cond-short.mk Sun Jun 28 11:06:26 2020
@@ -1,32 +1,53 @@
-# $NetBSD: cond-short.mk,v 1.1 2020/06/28 09:42:40 rillig Exp $
+# $NetBSD: cond-short.mk,v 1.2 2020/06/28 11:06:26 rillig Exp $
#
# Demonstrates that in conditions, the right-hand side of an && or ||
-# is evaluated even though it cannot influence the result.
+# is only evaluated if it can actually influence the result.
#
-# This is unexpected for several reasons:
-#
-# 1. The manual page says: "bmake will only evaluate a conditional as
-# far as is necessary to determine its value."
-#
-# 2. It differs from the way that these operators are evaluated in
-# almost all other programming languages.
-#
-# 3. In cond.c there are lots of doEval variables.
+# Between 2015-10-11 and 2020-06-28, the right-hand side of an && or ||
+# operator was always evaluated, which was wrong.
#
+# The && operator.
+
.if 0 && ${echo "unexpected and" 1>&2 :L:sh}
.endif
.if 1 && ${echo "expected and" 1>&2 :L:sh}
.endif
+.if 0 && exists(nonexistent${echo "unexpected and exists" 1>&2 :L:sh})
+.endif
+
+.if 1 && exists(nonexistent${echo "expected and exists" 1>&2 :L:sh})
+.endif
+
+.if 0 && empty(${echo "unexpected and empty" 1>&2 :L:sh})
+.endif
+
+.if 1 && empty(${echo "expected and empty" 1>&2 :L:sh})
+.endif
+
+# The || operator.
+
.if 1 || ${echo "unexpected or" 1>&2 :L:sh}
.endif
.if 0 || ${echo "expected or" 1>&2 :L:sh}
.endif
-# The following paragraphs demonstrate the workaround.
+.if 1 || exists(nonexistent${echo "unexpected or exists" 1>&2 :L:sh})
+.endif
+
+.if 0 || exists(nonexistent${echo "expected or exists" 1>&2 :L:sh})
+.endif
+
+.if 1 || empty(${echo "unexpected or empty" 1>&2 :L:sh})
+.endif
+
+.if 0 || empty(${echo "expected or empty" 1>&2 :L:sh})
+.endif
+
+# Unreachable nested conditions are skipped completely as well.
.if 0
. if ${echo "unexpected nested and" 1>&2 :L:sh}
