Module Name: src
Committed By: rillig
Date: Tue Sep 8 05:26:22 UTC 2020
Modified Files:
src/usr.bin/make: parse.c suff.c var.c
src/usr.bin/make/unit-tests: dep-var.exp dep-var.mk
Log Message:
make(1): fix off-by-one error in SuffExpandChildren
In suff.c r1.144 from yesterday, in the line "cp += nested_p - cp", I
accidentally removed the "- 1". Since these "- 1" lines lead to slow
execution, each branch now increments the pointer separately by the
actually needed amount.
Fixing this bug posed way more new questions than it answered, and it
revealed an inconsistency in the parser about how characters are to be
escaped, and missing details in the documentation of Var_Parse, as well
as a parse error that unexpectedly doesn't stop make from continuing.
To generate a diff of this commit:
cvs rdiff -u -r1.288 -r1.289 src/usr.bin/make/parse.c
cvs rdiff -u -r1.144 -r1.145 src/usr.bin/make/suff.c
cvs rdiff -u -r1.490 -r1.491 src/usr.bin/make/var.c
cvs rdiff -u -r1.3 -r1.4 src/usr.bin/make/unit-tests/dep-var.exp \
src/usr.bin/make/unit-tests/dep-var.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/parse.c
diff -u src/usr.bin/make/parse.c:1.288 src/usr.bin/make/parse.c:1.289
--- src/usr.bin/make/parse.c:1.288 Mon Sep 7 18:37:09 2020
+++ src/usr.bin/make/parse.c Tue Sep 8 05:26:21 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: parse.c,v 1.288 2020/09/07 18:37:09 rillig Exp $ */
+/* $NetBSD: parse.c,v 1.289 2020/09/08 05:26:21 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -69,14 +69,14 @@
*/
#ifndef MAKE_NATIVE
-static char rcsid[] = "$NetBSD: parse.c,v 1.288 2020/09/07 18:37:09 rillig Exp $";
+static char rcsid[] = "$NetBSD: parse.c,v 1.289 2020/09/08 05:26:21 rillig Exp $";
#else
#include <sys/cdefs.h>
#ifndef lint
#if 0
static char sccsid[] = "@(#)parse.c 8.3 (Berkeley) 3/19/94";
#else
-__RCSID("$NetBSD: parse.c,v 1.288 2020/09/07 18:37:09 rillig Exp $");
+__RCSID("$NetBSD: parse.c,v 1.289 2020/09/08 05:26:21 rillig Exp $");
#endif
#endif /* not lint */
#endif
@@ -708,17 +708,10 @@ ParseErrorInternal(const char *cfname, s
}
}
-/*-
- * Parse_Error --
- * External interface to ParseErrorInternal; uses the default filename
- * Line number.
- *
- * Results:
- * None
+/* External interface to ParseErrorInternal; uses the default filename and
+ * line number.
*
- * Side Effects:
- * None
- */
+ * Fmt is given without a trailing newline. */
/* VARARGS */
void
Parse_Error(int type, const char *fmt, ...)
Index: src/usr.bin/make/suff.c
diff -u src/usr.bin/make/suff.c:1.144 src/usr.bin/make/suff.c:1.145
--- src/usr.bin/make/suff.c:1.144 Mon Sep 7 07:15:26 2020
+++ src/usr.bin/make/suff.c Tue Sep 8 05:26:21 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: suff.c,v 1.144 2020/09/07 07:15:26 rillig Exp $ */
+/* $NetBSD: suff.c,v 1.145 2020/09/08 05:26:21 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -69,14 +69,14 @@
*/
#ifndef MAKE_NATIVE
-static char rcsid[] = "$NetBSD: suff.c,v 1.144 2020/09/07 07:15:26 rillig Exp $";
+static char rcsid[] = "$NetBSD: suff.c,v 1.145 2020/09/08 05:26:21 rillig Exp $";
#else
#include <sys/cdefs.h>
#ifndef lint
#if 0
static char sccsid[] = "@(#)suff.c 8.4 (Berkeley) 3/21/94";
#else
-__RCSID("$NetBSD: suff.c,v 1.144 2020/09/07 07:15:26 rillig Exp $");
+__RCSID("$NetBSD: suff.c,v 1.145 2020/09/08 05:26:21 rillig Exp $");
#endif
#endif /* not lint */
#endif
@@ -1302,7 +1302,8 @@ SuffExpandChildren(LstNode cln, GNode *p
for (start = cp; *start == ' ' || *start == '\t'; start++)
continue;
- for (cp = start; *cp != '\0'; cp++) {
+ cp = start;
+ while (*cp != '\0') {
if (*cp == ' ' || *cp == '\t') {
/*
* White-space -- terminate element, find the node,
@@ -1314,11 +1315,7 @@ SuffExpandChildren(LstNode cln, GNode *p
while (*cp == ' ' || *cp == '\t') {
cp++;
}
- /*
- * Adjust cp for increment at start of loop, but
- * set start to first non-space.
- */
- start = cp--;
+ start = cp; /* Continue at the next non-space. */
} else if (*cp == '$') {
/*
* Start of a variable spec -- contact variable module
@@ -1328,9 +1325,15 @@ SuffExpandChildren(LstNode cln, GNode *p
const char *junk;
void *freeIt;
+ /* XXX: Why VARE_WANTRES when the result is not used? */
junk = Var_ParsePP(&nested_p, pgn,
VARE_UNDEFERR|VARE_WANTRES, &freeIt);
- if (junk != var_Error) {
+ if (junk == var_Error) {
+ Parse_Error(PARSE_FATAL,
+ "Malformed variable expression at \"%s\"",
+ cp);
+ cp++;
+ } else {
cp += nested_p - cp;
}
@@ -1339,6 +1342,11 @@ SuffExpandChildren(LstNode cln, GNode *p
/*
* Escaped something -- skip over it
*/
+ /* XXX: In other places, escaping at this syntactical
+ * position is done by a '$', not a '\'. The '\' is only
+ * used in variable modifiers. */
+ cp += 2;
+ } else {
cp++;
}
}
Index: src/usr.bin/make/var.c
diff -u src/usr.bin/make/var.c:1.490 src/usr.bin/make/var.c:1.491
--- src/usr.bin/make/var.c:1.490 Mon Sep 7 07:10:56 2020
+++ src/usr.bin/make/var.c Tue Sep 8 05:26:21 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: var.c,v 1.490 2020/09/07 07:10:56 rillig Exp $ */
+/* $NetBSD: var.c,v 1.491 2020/09/08 05:26:21 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -69,14 +69,14 @@
*/
#ifndef MAKE_NATIVE
-static char rcsid[] = "$NetBSD: var.c,v 1.490 2020/09/07 07:10:56 rillig Exp $";
+static char rcsid[] = "$NetBSD: var.c,v 1.491 2020/09/08 05:26:21 rillig 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.490 2020/09/07 07:10:56 rillig Exp $");
+__RCSID("$NetBSD: var.c,v 1.491 2020/09/08 05:26:21 rillig Exp $");
#endif
#endif /* not lint */
#endif
@@ -3029,6 +3029,7 @@ ApplyModifiers(
if (rval[0] != '\0' &&
(c = *nested_p) != '\0' && c != ':' && c != st.endc) {
free(freeIt);
+ /* XXX: apply_mods doesn't sound like "not interested". */
goto apply_mods;
}
@@ -3325,6 +3326,16 @@ VarIsDynamic(GNode *ctxt, const char *va
* varNoError if there was a parse error and VARE_UNDEFERR was not set.
*
* Parsing should continue at str + *lengthPtr.
+ * TODO: Document the value of *lengthPtr on parse errors. It might be
+ * 0, or +1, or the index of the parse error, or the guessed end of the
+ * variable expression.
+ *
+ * If var_Error is returned, a diagnostic may or may not have been
+ * printed. XXX: This is inconsistent.
+ *
+ * If varNoError is returned, a diagnostic may or may not have been
+ * printed. XXX: This is inconsistent, and as of 2020-09-08, returning
+ * varNoError is even used to return a regular, non-error empty string.
*
* After using the returned value, *freePtr must be freed, preferably
* using bmake_free since it is NULL in most cases.
Index: src/usr.bin/make/unit-tests/dep-var.exp
diff -u src/usr.bin/make/unit-tests/dep-var.exp:1.3 src/usr.bin/make/unit-tests/dep-var.exp:1.4
--- src/usr.bin/make/unit-tests/dep-var.exp:1.3 Thu Sep 3 19:50:14 2020
+++ src/usr.bin/make/unit-tests/dep-var.exp Tue Sep 8 05:26:22 2020
@@ -1,4 +1,6 @@
+make: Malformed variable expression at "$)"
def2
a-def2-b
1-2-NDIRECT_2-2-1
+)
exit status 0
Index: src/usr.bin/make/unit-tests/dep-var.mk
diff -u src/usr.bin/make/unit-tests/dep-var.mk:1.3 src/usr.bin/make/unit-tests/dep-var.mk:1.4
--- src/usr.bin/make/unit-tests/dep-var.mk:1.3 Thu Sep 3 19:50:14 2020
+++ src/usr.bin/make/unit-tests/dep-var.mk Tue Sep 8 05:26:22 2020
@@ -1,4 +1,4 @@
-# $NetBSD: dep-var.mk,v 1.3 2020/09/03 19:50:14 rillig Exp $
+# $NetBSD: dep-var.mk,v 1.4 2020/09/08 05:26:22 rillig Exp $
#
# Tests for variable references in dependency declarations.
#
@@ -61,5 +61,20 @@ INDIRECT_3= indirect
UNDEF1= undef1
DEF2= def2
-undef1 def2 a-def2-b 1-2-$$INDIRECT_2-2-1:
+# Cover the code in SuffExpandChildren that deals with malformed variable
+# expressions.
+#
+# This seems to be an edge case that never happens in practice, and it would
+# probably be appropriate to just error out in such a case.
+#
+# To trigger this piece of code, the variable name must contain "$)" or "$:"
+# or "$)" or "$$". Using "$:" does not work since the dependency line is
+# fully expanded before parsing, therefore any ':' in a target or source name
+# would be interpreted as a dependency operator instead.
+all: $$$$)
+
+undef1 def2 a-def2-b 1-2-$$INDIRECT_2-2-1 ${:U\$)}:
@echo ${.TARGET:Q}
+
+# XXX: Why is the exit status still 0, even though Parse_Error is called
+# with PARSE_FATAL in SuffExpandChildren?
