Module Name: src
Committed By: rillig
Date: Wed Sep 23 03:06:39 UTC 2020
Modified Files:
src/distrib/sets/lists/tests: mi
src/usr.bin/make: compat.c job.c make.h nonints.h targ.c
src/usr.bin/make/unit-tests: Makefile deptgt-end.mk
Added Files:
src/usr.bin/make/unit-tests: deptgt-end-jobs.exp deptgt-end-jobs.mk
Log Message:
make(1): fix assertion failure in -j mode with .END node
There had been two separate global variables for the .END node, and in
parallel mode, only the one in jobs.c was initialized.
The code in JobRun heads over to Compat_Make without calling Compat_Run
first, which left the variable ENDNode uninitialized.
To generate a diff of this commit:
cvs rdiff -u -r1.925 -r1.926 src/distrib/sets/lists/tests/mi
cvs rdiff -u -r1.148 -r1.149 src/usr.bin/make/compat.c
cvs rdiff -u -r1.234 -r1.235 src/usr.bin/make/job.c
cvs rdiff -u -r1.144 -r1.145 src/usr.bin/make/make.h
cvs rdiff -u -r1.126 -r1.127 src/usr.bin/make/nonints.h
cvs rdiff -u -r1.89 -r1.90 src/usr.bin/make/targ.c
cvs rdiff -u -r1.145 -r1.146 src/usr.bin/make/unit-tests/Makefile
cvs rdiff -u -r0 -r1.1 src/usr.bin/make/unit-tests/deptgt-end-jobs.exp \
src/usr.bin/make/unit-tests/deptgt-end-jobs.mk
cvs rdiff -u -r1.4 -r1.5 src/usr.bin/make/unit-tests/deptgt-end.mk
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/distrib/sets/lists/tests/mi
diff -u src/distrib/sets/lists/tests/mi:1.925 src/distrib/sets/lists/tests/mi:1.926
--- src/distrib/sets/lists/tests/mi:1.925 Tue Sep 22 01:09:33 2020
+++ src/distrib/sets/lists/tests/mi Wed Sep 23 03:06:38 2020
@@ -1,4 +1,4 @@
-# $NetBSD: mi,v 1.925 2020/09/22 01:09:33 kamil Exp $
+# $NetBSD: mi,v 1.926 2020/09/23 03:06:38 rillig Exp $
#
# Note: don't delete entries from here - mark them as "obsolete" instead.
#
@@ -4670,6 +4670,8 @@
./usr/tests/usr.bin/make/unit-tests/deptgt-default.mk tests-usr.bin-tests compattestfile,atf
./usr/tests/usr.bin/make/unit-tests/deptgt-delete_on_error.exp tests-usr.bin-tests compattestfile,atf
./usr/tests/usr.bin/make/unit-tests/deptgt-delete_on_error.mk tests-usr.bin-tests compattestfile,atf
+./usr/tests/usr.bin/make/unit-tests/deptgt-end-jobs.exp tests-usr.bin-tests compattestfile,atf
+./usr/tests/usr.bin/make/unit-tests/deptgt-end-jobs.mk tests-usr.bin-tests compattestfile,atf
./usr/tests/usr.bin/make/unit-tests/deptgt-end.exp tests-usr.bin-tests compattestfile,atf
./usr/tests/usr.bin/make/unit-tests/deptgt-end.mk tests-usr.bin-tests compattestfile,atf
./usr/tests/usr.bin/make/unit-tests/deptgt-error.exp tests-usr.bin-tests compattestfile,atf
Index: src/usr.bin/make/compat.c
diff -u src/usr.bin/make/compat.c:1.148 src/usr.bin/make/compat.c:1.149
--- src/usr.bin/make/compat.c:1.148 Tue Sep 22 20:19:46 2020
+++ src/usr.bin/make/compat.c Wed Sep 23 03:06:38 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: compat.c,v 1.148 2020/09/22 20:19:46 rillig Exp $ */
+/* $NetBSD: compat.c,v 1.149 2020/09/23 03:06:38 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -99,10 +99,9 @@
#include "pathnames.h"
/* "@(#)compat.c 8.2 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: compat.c,v 1.148 2020/09/22 20:19:46 rillig Exp $");
+MAKE_RCSID("$NetBSD: compat.c,v 1.149 2020/09/23 03:06:38 rillig Exp $");
static GNode *curTarg = NULL;
-static GNode *ENDNode;
static void CompatInterrupt(int);
static pid_t compatChild;
static int compatSigno;
@@ -226,10 +225,12 @@ Compat_RunCommand(char *cmdp, struct GNo
cmd = cmdStart;
LstNode_Set(cmdNode, cmdStart);
- if ((gn->type & OP_SAVE_CMDS) && (gn != ENDNode)) {
- assert(ENDNode != NULL);
- Lst_Append(ENDNode->commands, cmdStart);
- return 0;
+ if (gn->type & OP_SAVE_CMDS) {
+ GNode *endNode = Targ_GetEndNode();
+ if (gn != endNode) {
+ Lst_Append(endNode->commands, cmdStart);
+ return 0;
+ }
}
if (strcmp(cmdStart, "...") == 0) {
gn->type |= OP_SAVE_CMDS;
@@ -682,8 +683,10 @@ Compat_Run(GNodeList *targs)
bmake_signal(SIGQUIT, CompatInterrupt);
}
- ENDNode = Targ_FindNode(".END", TARG_CREATE);
- ENDNode->type = OP_SPECIAL;
+ /* Create the .END node now, to keep the (debug) output of the
+ * counter.mk test the same as before 2020-09-23. This implementation
+ * detail probably doesn't matter though. */
+ (void)Targ_GetEndNode();
/*
* If the user has defined a .BEGIN target, execute the commands attached
* to it.
@@ -732,7 +735,9 @@ Compat_Run(GNodeList *targs)
* If the user has defined a .END target, run its commands.
*/
if (errors == 0) {
- Compat_Make(ENDNode, ENDNode);
+ GNode *endNode = Targ_GetEndNode();
+ Compat_Make(endNode, endNode);
+ /* XXX: Did you mean endNode->made instead of gn->made? */
if (gn->made == ERROR) {
PrintOnError(gn, "\nStop.");
exit(1);
Index: src/usr.bin/make/job.c
diff -u src/usr.bin/make/job.c:1.234 src/usr.bin/make/job.c:1.235
--- src/usr.bin/make/job.c:1.234 Tue Sep 22 20:19:46 2020
+++ src/usr.bin/make/job.c Wed Sep 23 03:06:38 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: job.c,v 1.234 2020/09/22 20:19:46 rillig Exp $ */
+/* $NetBSD: job.c,v 1.235 2020/09/23 03:06:38 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -140,7 +140,7 @@
#include "trace.h"
/* "@(#)job.c 8.2 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: job.c,v 1.234 2020/09/22 20:19:46 rillig Exp $");
+MAKE_RCSID("$NetBSD: job.c,v 1.235 2020/09/23 03:06:38 rillig Exp $");
# define STATIC static
@@ -165,14 +165,6 @@ int jobTokensRunning = 0;
*/
#define FILENO(a) ((unsigned) fileno(a))
-/*
- * post-make command processing. The node postCommands is really just the
- * .END target but we keep it around to avoid having to search for it
- * all the time.
- */
-static GNode *postCommands = NULL;
- /* node containing commands to execute when
- * everything else is done */
static int numCommands; /* The number of commands actually printed
* for a target. Should this number be
* 0, no shell will be executed. */
@@ -907,7 +899,7 @@ JobSaveCommand(void *cmd, void *gn)
char *expanded_cmd;
(void)Var_Subst(cmd, (GNode *)gn, VARE_WANTRES, &expanded_cmd);
/* TODO: handle errors */
- Lst_Append(postCommands->commands, expanded_cmd);
+ Lst_Append(Targ_GetEndNode()->commands, expanded_cmd);
return 0;
}
@@ -2301,7 +2293,9 @@ Job_Init(void)
#undef ADDSIG
(void)Job_RunTarget(".BEGIN", NULL);
- postCommands = Targ_FindNode(".END", TARG_CREATE);
+ /* Create the .END node now, even though no code in the unit tests
+ * depends on it. See also Targ_GetEndNode in Compat_Run. */
+ (void)Targ_GetEndNode();
}
static void JobSigReset(void)
@@ -2601,29 +2595,20 @@ JobInterrupt(int runINTERRUPT, int signo
exit(signo);
}
-/*
- *-----------------------------------------------------------------------
- * Job_Finish --
- * Do final processing such as the running of the commands
- * attached to the .END target.
+/* Do the final processing, i.e. run the commands attached to the .END target.
*
* Results:
* Number of errors reported.
- *
- * Side Effects:
- * None.
- *-----------------------------------------------------------------------
*/
int
Job_Finish(void)
{
- if (postCommands != NULL &&
- (!Lst_IsEmpty(postCommands->commands) ||
- !Lst_IsEmpty(postCommands->children))) {
+ GNode *endNode = Targ_GetEndNode();
+ if (!Lst_IsEmpty(endNode->commands) || !Lst_IsEmpty(endNode->children)) {
if (errors) {
Error("Errors reported so .END ignored");
} else {
- JobRun(postCommands);
+ JobRun(endNode);
}
}
return errors;
Index: src/usr.bin/make/make.h
diff -u src/usr.bin/make/make.h:1.144 src/usr.bin/make/make.h:1.145
--- src/usr.bin/make/make.h:1.144 Tue Sep 22 04:05:41 2020
+++ src/usr.bin/make/make.h Wed Sep 23 03:06:38 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: make.h,v 1.144 2020/09/22 04:05:41 rillig Exp $ */
+/* $NetBSD: make.h,v 1.145 2020/09/23 03:06:38 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -251,7 +251,9 @@ typedef enum {
/* Target has all the commands it should. Used when parsing to catch
* multiple commands for a target. */
OP_HAS_COMMANDS = 1 << 27,
- /* Saving commands on .END (Compat) */
+ /* The special command "..." has been seen. All further commands from
+ * this node will be saved on the .END node instead, to be executed at
+ * the very end. */
OP_SAVE_CMDS = 1 << 26,
/* Already processed by Suff_FindDeps */
OP_DEPS_FOUND = 1 << 25,
Index: src/usr.bin/make/nonints.h
diff -u src/usr.bin/make/nonints.h:1.126 src/usr.bin/make/nonints.h:1.127
--- src/usr.bin/make/nonints.h:1.126 Tue Sep 22 20:19:46 2020
+++ src/usr.bin/make/nonints.h Wed Sep 23 03:06:38 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: nonints.h,v 1.126 2020/09/22 20:19:46 rillig Exp $ */
+/* $NetBSD: nonints.h,v 1.127 2020/09/23 03:06:38 rillig Exp $ */
/*-
* Copyright (c) 1988, 1989, 1990, 1993
@@ -174,6 +174,7 @@ void Targ_Stats(void);
GNodeList *Targ_List(void);
GNode *Targ_NewGN(const char *);
GNode *Targ_FindNode(const char *, int);
+GNode *Targ_GetEndNode(void);
GNodeList *Targ_FindList(StringList *, int);
Boolean Targ_Ignore(GNode *);
Boolean Targ_Silent(GNode *);
Index: src/usr.bin/make/targ.c
diff -u src/usr.bin/make/targ.c:1.89 src/usr.bin/make/targ.c:1.90
--- src/usr.bin/make/targ.c:1.89 Tue Sep 22 04:05:41 2020
+++ src/usr.bin/make/targ.c Wed Sep 23 03:06:38 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: targ.c,v 1.89 2020/09/22 04:05:41 rillig Exp $ */
+/* $NetBSD: targ.c,v 1.90 2020/09/23 03:06:38 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -122,7 +122,7 @@
#include "dir.h"
/* "@(#)targ.c 8.2 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: targ.c,v 1.89 2020/09/22 04:05:41 rillig Exp $");
+MAKE_RCSID("$NetBSD: targ.c,v 1.90 2020/09/23 03:06:38 rillig Exp $");
static GNodeList *allTargets; /* the list of all targets found so far */
#ifdef CLEANUP
@@ -283,6 +283,19 @@ Targ_FindNode(const char *name, int flag
return gn;
}
+/* Return the .END node, which contains the commands to be executed when
+ * everything else is done. */
+GNode *Targ_GetEndNode(void)
+{
+ /* Save the node locally to avoid having to search for it all the time. */
+ static GNode *endNode = NULL;
+ if (endNode == NULL) {
+ endNode = Targ_FindNode(".END", TARG_CREATE);
+ endNode->type = OP_SPECIAL;
+ }
+ return endNode;
+}
+
/* Make a complete list of GNodes from the given list of names.
* If flags is TARG_CREATE, nodes will be created for all names in
* names which do not yet have graph nodes. If flags is TARG_NOCREATE,
Index: src/usr.bin/make/unit-tests/Makefile
diff -u src/usr.bin/make/unit-tests/Makefile:1.145 src/usr.bin/make/unit-tests/Makefile:1.146
--- src/usr.bin/make/unit-tests/Makefile:1.145 Mon Sep 21 04:20:35 2020
+++ src/usr.bin/make/unit-tests/Makefile Wed Sep 23 03:06:38 2020
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.145 2020/09/21 04:20:35 rillig Exp $
+# $NetBSD: Makefile,v 1.146 2020/09/23 03:06:38 rillig Exp $
#
# Unit tests for make(1)
#
@@ -102,6 +102,7 @@ TESTS+= deptgt-begin
TESTS+= deptgt-default
TESTS+= deptgt-delete_on_error
TESTS+= deptgt-end
+TESTS+= deptgt-end-jobs
TESTS+= deptgt-error
TESTS+= deptgt-ignore
TESTS+= deptgt-interrupt
Index: src/usr.bin/make/unit-tests/deptgt-end.mk
diff -u src/usr.bin/make/unit-tests/deptgt-end.mk:1.4 src/usr.bin/make/unit-tests/deptgt-end.mk:1.5
--- src/usr.bin/make/unit-tests/deptgt-end.mk:1.4 Tue Sep 22 19:41:09 2020
+++ src/usr.bin/make/unit-tests/deptgt-end.mk Wed Sep 23 03:06:38 2020
@@ -1,4 +1,4 @@
-# $NetBSD: deptgt-end.mk,v 1.4 2020/09/22 19:41:09 rillig Exp $
+# $NetBSD: deptgt-end.mk,v 1.5 2020/09/23 03:06:38 rillig Exp $
#
# Tests for the special target .END in dependency declarations,
# which is run after making the desired targets.
@@ -11,6 +11,7 @@ VAR= Should not be expanded.
: $@ '$${VAR}' deferred
# Oops: The deferred command must not be expanded twice.
# The Var_Subst in Compat_RunCommand looks suspicious.
+# The Var_Subst in JobSaveCommand looks suspicious.
.END:
: $@ '$${VAR}'
@@ -23,6 +24,7 @@ all:
: $@ '$${VAR}' deferred
# Oops: The deferred command must not be expanded twice.
# The Var_Subst in Compat_RunCommand looks suspicious.
+# The Var_Subst in JobSaveCommand looks suspicious.
# The deferred commands are run in the order '.END .BEGIN all'.
# This may be unexpected at first since the natural order would be
Added files:
Index: src/usr.bin/make/unit-tests/deptgt-end-jobs.exp
diff -u /dev/null src/usr.bin/make/unit-tests/deptgt-end-jobs.exp:1.1
--- /dev/null Wed Sep 23 03:06:39 2020
+++ src/usr.bin/make/unit-tests/deptgt-end-jobs.exp Wed Sep 23 03:06:38 2020
@@ -0,0 +1,8 @@
+: .BEGIN '${VAR}'
+--- all ---
+: all '${VAR}'
+: .END '${VAR}'
+: .END '${VAR}' deferred
+: .BEGIN 'Should not be expanded.' deferred
+: all 'Should not be expanded.' deferred
+exit status 0
Index: src/usr.bin/make/unit-tests/deptgt-end-jobs.mk
diff -u /dev/null src/usr.bin/make/unit-tests/deptgt-end-jobs.mk:1.1
--- /dev/null Wed Sep 23 03:06:39 2020
+++ src/usr.bin/make/unit-tests/deptgt-end-jobs.mk Wed Sep 23 03:06:38 2020
@@ -0,0 +1,46 @@
+# $NetBSD: deptgt-end-jobs.mk,v 1.1 2020/09/23 03:06:38 rillig Exp $
+#
+# Tests for the special target .END in dependency declarations,
+# which is run after making the desired targets.
+#
+# This test is very similar to deptgt-end.mk, except for the -j option.
+# This option enables parallel mode, in which the code from job.c partially
+# replaces the code from compat.c.
+#
+# Before 2020-08-22, this test crashed with a null pointer dereference.
+# Before 2020-09-23, this test crashed with an assertion failure.
+.MAKEFLAGS: -j 8
+
+VAR= Should not be expanded.
+
+.BEGIN:
+ : $@ '$${VAR}'
+ ...
+ : $@ '$${VAR}' deferred
+# Oops: The deferred command must not be expanded twice.
+# The Var_Subst in Compat_RunCommand looks suspicious.
+# The Var_Subst in JobSaveCommand looks suspicious.
+
+.END:
+ : $@ '$${VAR}'
+ ...
+ : $@ '$${VAR}' deferred
+
+all:
+ : $@ '$${VAR}'
+ ...
+ : $@ '$${VAR}' deferred
+# Oops: The deferred command must not be expanded twice.
+# The Var_Subst in Compat_RunCommand looks suspicious.
+# The Var_Subst in JobSaveCommand looks suspicious.
+
+# The deferred commands are run in the order '.END .BEGIN all'.
+# This may be unexpected at first since the natural order would be
+# '.BEGIN all .END', but it is implemented correctly.
+#
+# At the point where the commands of a node with deferred commands are run,
+# the deferred commands are appended to the commands of the .END node.
+# This happens in Compat_RunCommand, and to prevent an endless loop, the
+# deferred commands of the .END node itself are not appended to itself.
+# Instead, the deferred commands of the .END node are run as if they were
+# immediate commands.
