Module Name: src
Committed By: rillig
Date: Sun Sep 13 18:27:39 UTC 2020
Modified Files:
src/usr.bin/make: arch.c cond.c nonints.h parse.c suff.c var.c
Log Message:
make(1): prepare Var_Parse for proper error handling and reporting
Right now, Var_Parse swallows many errors during parsing and evaluation.
Ideally, these errors should propagate from the deeply nested
expressions where they occur up to the top-level expressions. When such
an error occurs, the depending expressions should not be evaluated any
further. They may still be parsed, but side effects should be
minimized.
The goal is to prevent incomplete expressions like the "xy}" in
moderrs.exp:106 from being evaluated and eventually passed to the shell
for execution. This expression is a left-over from a parse error in the
mod-t-parse target in moderrs.mk:154.
This commit is a first step in analyzing and verifying the current state
of affairs. The modelling in VarParseErrors already looks complicated
but is expected to closely match reality.
To generate a diff of this commit:
cvs rdiff -u -r1.114 -r1.115 src/usr.bin/make/arch.c
cvs rdiff -u -r1.144 -r1.145 src/usr.bin/make/cond.c
cvs rdiff -u -r1.118 -r1.119 src/usr.bin/make/nonints.h
cvs rdiff -u -r1.303 -r1.304 src/usr.bin/make/parse.c
cvs rdiff -u -r1.155 -r1.156 src/usr.bin/make/suff.c
cvs rdiff -u -r1.513 -r1.514 src/usr.bin/make/var.c
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/arch.c
diff -u src/usr.bin/make/arch.c:1.114 src/usr.bin/make/arch.c:1.115
--- src/usr.bin/make/arch.c:1.114 Sun Sep 13 15:15:51 2020
+++ src/usr.bin/make/arch.c Sun Sep 13 18:27:39 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: arch.c,v 1.114 2020/09/13 15:15:51 rillig Exp $ */
+/* $NetBSD: arch.c,v 1.115 2020/09/13 18:27:39 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -133,7 +133,7 @@
#include "config.h"
/* "@(#)arch.c 8.2 (Berkeley) 1/2/94" */
-MAKE_RCSID("$NetBSD: arch.c,v 1.114 2020/09/13 15:15:51 rillig Exp $");
+MAKE_RCSID("$NetBSD: arch.c,v 1.115 2020/09/13 18:27:39 rillig Exp $");
#ifdef TARGET_MACHINE
#undef MAKE_MACHINE
@@ -228,8 +228,9 @@ Arch_ParseArchive(char **linePtr, Lst no
const char *result;
Boolean isError;
- result = Var_Parse(&nested_p, ctxt,
- VARE_UNDEFERR|VARE_WANTRES, &result_freeIt);
+ (void)Var_Parse(&nested_p, ctxt, VARE_UNDEFERR|VARE_WANTRES,
+ &result, &result_freeIt);
+ /* TODO: handle errors */
isError = result == var_Error;
free(result_freeIt);
if (isError)
@@ -270,8 +271,9 @@ Arch_ParseArchive(char **linePtr, Lst no
Boolean isError;
const char *nested_p = cp;
- result = Var_Parse(&nested_p, ctxt,
- VARE_UNDEFERR|VARE_WANTRES, &freeIt);
+ (void)Var_Parse(&nested_p, ctxt, VARE_UNDEFERR|VARE_WANTRES,
+ &result, &freeIt);
+ /* TODO: handle errors */
isError = result == var_Error;
free(freeIt);
Index: src/usr.bin/make/cond.c
diff -u src/usr.bin/make/cond.c:1.144 src/usr.bin/make/cond.c:1.145
--- src/usr.bin/make/cond.c:1.144 Sun Sep 13 15:15:51 2020
+++ src/usr.bin/make/cond.c Sun Sep 13 18:27:39 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: cond.c,v 1.144 2020/09/13 15:15:51 rillig Exp $ */
+/* $NetBSD: cond.c,v 1.145 2020/09/13 18:27:39 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -93,7 +93,7 @@
#include "dir.h"
/* "@(#)cond.c 8.2 (Berkeley) 1/2/94" */
-MAKE_RCSID("$NetBSD: cond.c,v 1.144 2020/09/13 15:15:51 rillig Exp $");
+MAKE_RCSID("$NetBSD: cond.c,v 1.145 2020/09/13 18:27:39 rillig Exp $");
/*
* The parsing of conditional expressions is based on this grammar:
@@ -248,7 +248,9 @@ ParseFuncArg(const char **linePtr, Boole
*/
void *freeIt;
VarEvalFlags eflags = VARE_UNDEFERR | (doEval ? VARE_WANTRES : 0);
- const char *cp2 = Var_Parse(&cp, VAR_CMD, eflags, &freeIt);
+ const char *cp2;
+ (void)Var_Parse(&cp, VAR_CMD, eflags, &cp2, &freeIt);
+ /* TODO: handle errors */
Buf_AddStr(&buf, cp2);
free(freeIt);
continue;
@@ -452,7 +454,8 @@ CondParser_String(CondParser *par, Boole
(doEval ? VARE_WANTRES : 0);
nested_p = par->p;
atStart = nested_p == start;
- str = Var_Parse(&nested_p, VAR_CMD, eflags, freeIt);
+ (void)Var_Parse(&nested_p, VAR_CMD, eflags, &str, freeIt);
+ /* TODO: handle errors */
if (str == var_Error) {
if (*freeIt) {
free(*freeIt);
@@ -693,7 +696,9 @@ ParseEmptyArg(const char **linePtr, Bool
*argPtr = NULL;
(*linePtr)--; /* Make (*linePtr)[1] point to the '('. */
- val = Var_Parse(linePtr, VAR_CMD, doEval ? VARE_WANTRES : 0, &val_freeIt);
+ (void)Var_Parse(linePtr, VAR_CMD, doEval ? VARE_WANTRES : 0,
+ &val, &val_freeIt);
+ /* TODO: handle errors */
/* If successful, *linePtr points beyond the closing ')' now. */
if (val == var_Error) {
Index: src/usr.bin/make/nonints.h
diff -u src/usr.bin/make/nonints.h:1.118 src/usr.bin/make/nonints.h:1.119
--- src/usr.bin/make/nonints.h:1.118 Sun Sep 13 15:27:25 2020
+++ src/usr.bin/make/nonints.h Sun Sep 13 18:27:39 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: nonints.h,v 1.118 2020/09/13 15:27:25 rillig Exp $ */
+/* $NetBSD: nonints.h,v 1.119 2020/09/13 18:27:39 rillig Exp $ */
/*-
* Copyright (c) 1988, 1989, 1990, 1993
@@ -204,6 +204,59 @@ typedef enum {
VAR_SET_READONLY = 0x02
} VarSet_Flags;
+/* The state of error handling returned by Var_Parse.
+ *
+ * As of 2020-09-13, this bitset looks quite bloated,
+ * with all the constants doubled.
+ *
+ * Its purpose is to first document the existing behavior,
+ * and then migrate away from the SILENT constants, step by step,
+ * as these are not suited for reliable, consistent error handling
+ * and reporting. */
+typedef enum {
+
+ /* Both parsing and evaluation succeeded. */
+ VPE_OK = 0x0000,
+
+ /* Parsing failed.
+ * An error message has already been printed. */
+ VPE_PARSE_MSG = 0x0001,
+
+ /* Parsing failed.
+ * No error message has been printed yet.
+ *
+ * This should never happen since it is impossible to say where
+ * the parsing error occurred. */
+ VPE_PARSE_SILENT = 0x0002,
+
+ /* Parsing succeeded.
+ * During evaluation, VARE_UNDEFERR was set and there was an undefined
+ * variable.
+ * An error message has already been printed. */
+ VPE_UNDEF_MSG = 0x0010,
+
+ /* Parsing succeeded.
+ * During evaluation, VARE_UNDEFERR was set and there was an undefined
+ * variable.
+ * No error message has been printed yet.
+ *
+ * This should never happen since it is impossible to say which of
+ * the variables was undefined. */
+ VPE_UNDEF_SILENT = 0x0020,
+
+ /* Parsing succeeded.
+ * Evaluation failed.
+ * An error message has already been printed. */
+ VPE_EVAL_MSG = 0x0100,
+
+ /* Parsing succeeded.
+ * Evaluation failed.
+ * No error message has been printed yet.
+ *
+ * This should never happens since it is impossible to say where
+ * exactly the evaluation error occurred. */
+ VPE_EVAL_SILENT = 0x0200
+} VarParseErrors;
void Var_Delete(const char *, GNode *);
void Var_Set(const char *, const char *, GNode *);
@@ -211,7 +264,8 @@ void Var_Set_with_flags(const char *, co
void Var_Append(const char *, const char *, GNode *);
Boolean Var_Exists(const char *, GNode *);
const char *Var_Value(const char *, GNode *, char **);
-const char *Var_Parse(const char **, GNode *, VarEvalFlags, void **);
+VarParseErrors Var_Parse(const char **, GNode *, VarEvalFlags,
+ const char **, void **);
char *Var_Subst(const char *, GNode *, VarEvalFlags);
void Var_Init(void);
void Var_End(void);
Index: src/usr.bin/make/parse.c
diff -u src/usr.bin/make/parse.c:1.303 src/usr.bin/make/parse.c:1.304
--- src/usr.bin/make/parse.c:1.303 Sun Sep 13 15:15:51 2020
+++ src/usr.bin/make/parse.c Sun Sep 13 18:27:39 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: parse.c,v 1.303 2020/09/13 15:15:51 rillig Exp $ */
+/* $NetBSD: parse.c,v 1.304 2020/09/13 18:27:39 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -131,7 +131,7 @@
#include "pathnames.h"
/* "@(#)parse.c 8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: parse.c,v 1.303 2020/09/13 15:15:51 rillig Exp $");
+MAKE_RCSID("$NetBSD: parse.c,v 1.304 2020/09/13 18:27:39 rillig Exp $");
/* types and constants */
@@ -1164,10 +1164,12 @@ ParseDoDependency(char *line)
* in the initial Var_Subst and we wouldn't be here.
*/
const char *nested_p = cp;
+ const char *nested_val;
void *freeIt;
- (void)Var_Parse(&nested_p, VAR_CMD,
- VARE_UNDEFERR|VARE_WANTRES, &freeIt);
+ (void)Var_Parse(&nested_p, VAR_CMD, VARE_UNDEFERR|VARE_WANTRES,
+ &nested_val, &freeIt);
+ /* TODO: handle errors */
free(freeIt);
cp += nested_p - cp;
} else
Index: src/usr.bin/make/suff.c
diff -u src/usr.bin/make/suff.c:1.155 src/usr.bin/make/suff.c:1.156
--- src/usr.bin/make/suff.c:1.155 Sun Sep 13 15:15:51 2020
+++ src/usr.bin/make/suff.c Sun Sep 13 18:27:39 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: suff.c,v 1.155 2020/09/13 15:15:51 rillig Exp $ */
+/* $NetBSD: suff.c,v 1.156 2020/09/13 18:27:39 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -126,7 +126,7 @@
#include "dir.h"
/* "@(#)suff.c 8.4 (Berkeley) 3/21/94" */
-MAKE_RCSID("$NetBSD: suff.c,v 1.155 2020/09/13 15:15:51 rillig Exp $");
+MAKE_RCSID("$NetBSD: suff.c,v 1.156 2020/09/13 18:27:39 rillig Exp $");
#define SUFF_DEBUG0(fmt) \
if (!DEBUG(SUFF)) (void) 0; else fprintf(debug_file, fmt)
@@ -1318,8 +1318,10 @@ SuffExpandChildren(LstNode cln, GNode *p
void *freeIt;
/* XXX: Why VARE_WANTRES when the result is not used? */
- junk = Var_Parse(&nested_p, pgn,
- VARE_UNDEFERR|VARE_WANTRES, &freeIt);
+ (void)Var_Parse(&nested_p, pgn,
+ VARE_UNDEFERR|VARE_WANTRES,
+ &junk, &freeIt);
+ /* TODO: handle errors */
if (junk == var_Error) {
Parse_Error(PARSE_FATAL,
"Malformed variable expression at \"%s\"",
Index: src/usr.bin/make/var.c
diff -u src/usr.bin/make/var.c:1.513 src/usr.bin/make/var.c:1.514
--- src/usr.bin/make/var.c:1.513 Sun Sep 13 16:47:24 2020
+++ src/usr.bin/make/var.c Sun Sep 13 18:27:39 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: var.c,v 1.513 2020/09/13 16:47:24 rillig Exp $ */
+/* $NetBSD: var.c,v 1.514 2020/09/13 18:27:39 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -121,7 +121,7 @@
#include "metachar.h"
/* "@(#)var.c 8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.513 2020/09/13 16:47:24 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.514 2020/09/13 18:27:39 rillig Exp $");
#define VAR_DEBUG_IF(cond, fmt, ...) \
if (!(DEBUG(VAR) && (cond))) \
@@ -1673,8 +1673,9 @@ ParseModifierPart(
void *nested_val_freeIt;
VarEvalFlags nested_eflags = eflags & ~(unsigned)VARE_ASSIGN;
- nested_val = Var_Parse(&nested_p, ctxt, nested_eflags,
- &nested_val_freeIt);
+ (void)Var_Parse(&nested_p, ctxt, nested_eflags,
+ &nested_val, &nested_val_freeIt);
+ /* TODO: handle errors */
Buf_AddStr(&buf, nested_val);
free(nested_val_freeIt);
p += nested_p - p;
@@ -2021,7 +2022,9 @@ ApplyModifier_Defined(const char **pp, A
const char *nested_val;
void *nested_val_freeIt;
- nested_val = Var_Parse(&p, st->ctxt, eflags, &nested_val_freeIt);
+ (void)Var_Parse(&p, st->ctxt, eflags,
+ &nested_val, &nested_val_freeIt);
+ /* TODO: handle errors */
Buf_AddStr(&buf, nested_val);
free(nested_val_freeIt);
continue;
@@ -3008,14 +3011,16 @@ ApplyModifiers(
*/
const char *nested_p = p;
void *freeIt;
- const char *rval = Var_Parse(&nested_p, st.ctxt, st.eflags,
- &freeIt);
+ const char *rval;
+ int c;
+
+ (void)Var_Parse(&nested_p, st.ctxt, st.eflags, &rval, &freeIt);
+ /* TODO: handle errors */
/*
* If we have not parsed up to st.endc or ':',
* we are not interested.
*/
- int c;
if (rval[0] != '\0' &&
(c = *nested_p) != '\0' && c != ':' && c != st.endc) {
free(freeIt);
@@ -3344,7 +3349,9 @@ ParseVarname(const char **pp, char start
/* A variable inside a variable, expand. */
if (*p == '$') {
void *freeIt;
- const char *rval = Var_Parse(&p, ctxt, eflags, &freeIt);
+ const char *rval;
+ (void)Var_Parse(&p, ctxt, eflags, &rval, &freeIt);
+ /* TODO: handle errors */
Buf_AddStr(&buf, rval);
free(freeIt);
} else {
@@ -3427,9 +3434,10 @@ ValidShortVarname(char varname, const ch
* Any effects from the modifiers, such as :!cmd! or ::=value.
*-----------------------------------------------------------------------
*/
-/* coverity[+alloc : arg-*3] */
-const char *
-Var_Parse(const char **pp, GNode *ctxt, VarEvalFlags eflags, void **freePtr)
+/* coverity[+alloc : arg-*4] */
+VarParseErrors
+Var_Parse(const char **pp, GNode *ctxt, VarEvalFlags eflags,
+ const char **out_val, void **freePtr)
{
const char *const start = *pp;
const char *p;
@@ -3471,7 +3479,10 @@ Var_Parse(const char **pp, GNode *ctxt,
if (!ValidShortVarname(startc, start)) {
(*pp)++;
- return var_Error;
+ *out_val = var_Error;
+ /* XXX: It's a mixture between VPE_PARSE_MSG and VPE_PARSE_SILENT;
+ * In lint mode, an error message is printed. */
+ return VPE_PARSE_SILENT;
}
name[0] = startc;
@@ -3480,7 +3491,8 @@ Var_Parse(const char **pp, GNode *ctxt,
if (v == NULL) {
*pp += 2;
- return ShortVarValue(start[1], ctxt, eflags);
+ *out_val = ShortVarValue(start[1], ctxt, eflags);
+ return eflags & VARE_UNDEFERR ? VPE_UNDEF_SILENT : VPE_OK;
} else {
haveModifier = FALSE;
p = start + 1;
@@ -3502,7 +3514,8 @@ Var_Parse(const char **pp, GNode *ctxt,
Parse_Error(PARSE_FATAL, "Unclosed variable \"%s\"", varname);
*pp = p;
free(varname);
- return var_Error;
+ *out_val = var_Error;
+ return VPE_PARSE_MSG;
}
v = VarFind(varname, ctxt, FIND_ENV | FIND_GLOBAL | FIND_CMD);
@@ -3543,10 +3556,12 @@ Var_Parse(const char **pp, GNode *ctxt,
char *pstr = bmake_strsedup(start, p);
*freePtr = pstr;
free(varname);
- return pstr;
+ *out_val = pstr;
+ return VPE_OK;
} else {
free(varname);
- return (eflags & VARE_UNDEFERR) ? var_Error : varNoError;
+ *out_val = eflags & VARE_UNDEFERR ? var_Error : varNoError;
+ return eflags & VARE_UNDEFERR ? VPE_UNDEF_SILENT : VPE_OK;
}
}
@@ -3649,7 +3664,8 @@ Var_Parse(const char **pp, GNode *ctxt,
free(v->name);
free(v);
}
- return nstr;
+ *out_val = nstr;
+ return VPE_OK; /* TODO: may also be errors */
}
/* Substitute for all variables in the given string in the given context.
@@ -3711,7 +3727,9 @@ Var_Subst(const char *str, GNode *ctxt,
} else {
const char *nested_str = str;
void *freeIt;
- const char *val = Var_Parse(&nested_str, ctxt, eflags, &freeIt);
+ const char *val;
+ (void)Var_Parse(&nested_str, ctxt, eflags, &val, &freeIt);
+ /* TODO: handle errors */
if (val == var_Error || val == varNoError) {
/*
