Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/usr.bin/make make(1): fix off-by-one error in SuffExpandChil...
details: https://anonhg.NetBSD.org/src/rev/fe3216e3744d
branches: trunk
changeset: 975892:fe3216e3744d
user: rillig <rillig%NetBSD.org@localhost>
date: Tue Sep 08 05:26:21 2020 +0000
description:
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.
diffstat:
usr.bin/make/parse.c | 19 ++++++-------------
usr.bin/make/suff.c | 28 ++++++++++++++++++----------
usr.bin/make/unit-tests/dep-var.exp | 2 ++
usr.bin/make/unit-tests/dep-var.mk | 19 +++++++++++++++++--
usr.bin/make/var.c | 17 ++++++++++++++---
5 files changed, 57 insertions(+), 28 deletions(-)
diffs (217 lines):
diff -r 2efd606349d8 -r fe3216e3744d usr.bin/make/parse.c
--- a/usr.bin/make/parse.c Tue Sep 08 00:51:29 2020 +0000
+++ b/usr.bin/make/parse.c Tue Sep 08 05:26:21 2020 +0000
@@ -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 @@
}
}
-/*-
- * Parse_Error --
- * External interface to ParseErrorInternal; uses the default filename
- * Line number.
+/* External interface to ParseErrorInternal; uses the default filename and
+ * line number.
*
- * Results:
- * None
- *
- * Side Effects:
- * None
- */
+ * Fmt is given without a trailing newline. */
/* VARARGS */
void
Parse_Error(int type, const char *fmt, ...)
diff -r 2efd606349d8 -r fe3216e3744d usr.bin/make/suff.c
--- a/usr.bin/make/suff.c Tue Sep 08 00:51:29 2020 +0000
+++ b/usr.bin/make/suff.c Tue Sep 08 05:26:21 2020 +0000
@@ -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 @@
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 @@
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 @@
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 @@
/*
* 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++;
}
}
diff -r 2efd606349d8 -r fe3216e3744d usr.bin/make/unit-tests/dep-var.exp
--- a/usr.bin/make/unit-tests/dep-var.exp Tue Sep 08 00:51:29 2020 +0000
+++ b/usr.bin/make/unit-tests/dep-var.exp Tue Sep 08 05:26:21 2020 +0000
@@ -1,4 +1,6 @@
+make: Malformed variable expression at "$)"
def2
a-def2-b
1-2-NDIRECT_2-2-1
+)
exit status 0
diff -r 2efd606349d8 -r fe3216e3744d usr.bin/make/unit-tests/dep-var.mk
--- a/usr.bin/make/unit-tests/dep-var.mk Tue Sep 08 00:51:29 2020 +0000
+++ b/usr.bin/make/unit-tests/dep-var.mk Tue Sep 08 05:26:21 2020 +0000
@@ -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 @@
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?
diff -r 2efd606349d8 -r fe3216e3744d usr.bin/make/var.c
--- a/usr.bin/make/var.c Tue Sep 08 00:51:29 2020 +0000
+++ b/usr.bin/make/var.c Tue Sep 08 05:26:21 2020 +0000
@@ -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 @@
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 @@
* 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.
Home |
Main Index |
Thread Index |
Old Index