diff options
| author | rillig <rillig@NetBSD.org> | 2020-09-08 05:26:21 +0000 |
|---|---|---|
| committer | rillig <rillig@NetBSD.org> | 2020-09-08 05:26:21 +0000 |
| commit | 9957c6c4e179dfd762b0c5287d71ec541ae08d8d (patch) | |
| tree | 6d7a8c943497130e9ad84e8cd46a4c3971f6170f /usr.bin/make | |
| parent | 24fabd450d8cf1a005e2179ccc124cd291d34839 (diff) | |
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 (limited to 'usr.bin/make')
| -rw-r--r-- | usr.bin/make/parse.c | 19 | ||||
| -rw-r--r-- | usr.bin/make/suff.c | 28 | ||||
| -rw-r--r-- | usr.bin/make/unit-tests/dep-var.exp | 2 | ||||
| -rw-r--r-- | usr.bin/make/unit-tests/dep-var.mk | 19 | ||||
| -rw-r--r-- | usr.bin/make/var.c | 17 |
5 files changed, 57 insertions, 28 deletions
diff --git a/usr.bin/make/parse.c b/usr.bin/make/parse.c index f0e236fb3f6..17de652d99d 100644 --- a/usr.bin/make/parse.c +++ b/usr.bin/make/parse.c @@ -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, size_t clineno, int type, } } -/*- - * 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, ...) diff --git a/usr.bin/make/suff.c b/usr.bin/make/suff.c index 3ba84ecb2d6..6ba415827b5 100644 --- a/usr.bin/make/suff.c +++ b/usr.bin/make/suff.c @@ -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 *pgn) 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 *pgn) 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 *pgn) 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 *pgn) /* * 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 --git a/usr.bin/make/unit-tests/dep-var.exp b/usr.bin/make/unit-tests/dep-var.exp index 4b3a7afe6ae..d32aca455ce 100644 --- a/usr.bin/make/unit-tests/dep-var.exp +++ b/usr.bin/make/unit-tests/dep-var.exp @@ -1,4 +1,6 @@ +make: Malformed variable expression at "$)" def2 a-def2-b 1-2-NDIRECT_2-2-1 +) exit status 0 diff --git a/usr.bin/make/unit-tests/dep-var.mk b/usr.bin/make/unit-tests/dep-var.mk index b9beb542267..127f3ba6647 100644 --- a/usr.bin/make/unit-tests/dep-var.mk +++ b/usr.bin/make/unit-tests/dep-var.mk @@ -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? diff --git a/usr.bin/make/var.c b/usr.bin/make/var.c index c2acc2eebdd..bbb4a4c3d78 100644 --- a/usr.bin/make/var.c +++ b/usr.bin/make/var.c @@ -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 *varname, size_t namelen) * 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. |
