Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/make/unit-tests make(1): fix assertion failure in -j...



details:   https://anonhg.NetBSD.org/src/rev/9378794d3a4e
branches:  trunk
changeset: 938997:9378794d3a4e
user:      rillig <rillig%NetBSD.org@localhost>
date:      Wed Sep 23 03:06:38 2020 +0000

description:
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.

diffstat:

 distrib/sets/lists/tests/mi                 |   4 +-
 usr.bin/make/compat.c                       |  25 +++++++++------
 usr.bin/make/job.c                          |  35 ++++++---------------
 usr.bin/make/make.h                         |   6 ++-
 usr.bin/make/nonints.h                      |   3 +-
 usr.bin/make/targ.c                         |  17 +++++++++-
 usr.bin/make/unit-tests/Makefile            |   3 +-
 usr.bin/make/unit-tests/deptgt-end-jobs.exp |   8 +++++
 usr.bin/make/unit-tests/deptgt-end-jobs.mk  |  46 +++++++++++++++++++++++++++++
 usr.bin/make/unit-tests/deptgt-end.mk       |   4 +-
 10 files changed, 108 insertions(+), 43 deletions(-)

diffs (truncated from 346 to 300 lines):

diff -r 43b5d2d6b90a -r 9378794d3a4e distrib/sets/lists/tests/mi
--- a/distrib/sets/lists/tests/mi       Wed Sep 23 02:32:04 2020 +0000
+++ b/distrib/sets/lists/tests/mi       Wed Sep 23 03:06:38 2020 +0000
@@ -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
diff -r 43b5d2d6b90a -r 9378794d3a4e usr.bin/make/compat.c
--- a/usr.bin/make/compat.c     Wed Sep 23 02:32:04 2020 +0000
+++ b/usr.bin/make/compat.c     Wed Sep 23 03:06:38 2020 +0000
@@ -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 @@
     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 @@
        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 @@
      * 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);
diff -r 43b5d2d6b90a -r 9378794d3a4e usr.bin/make/job.c
--- a/usr.bin/make/job.c        Wed Sep 23 02:32:04 2020 +0000
+++ b/usr.bin/make/job.c        Wed Sep 23 03:06:38 2020 +0000
@@ -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 @@
  */
 #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 @@
     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 @@
 #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 @@
     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;
diff -r 43b5d2d6b90a -r 9378794d3a4e usr.bin/make/make.h
--- a/usr.bin/make/make.h       Wed Sep 23 02:32:04 2020 +0000
+++ b/usr.bin/make/make.h       Wed Sep 23 03:06:38 2020 +0000
@@ -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 @@
     /* 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,
diff -r 43b5d2d6b90a -r 9378794d3a4e usr.bin/make/nonints.h
--- a/usr.bin/make/nonints.h    Wed Sep 23 02:32:04 2020 +0000
+++ b/usr.bin/make/nonints.h    Wed Sep 23 03:06:38 2020 +0000
@@ -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 @@
 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 *);
diff -r 43b5d2d6b90a -r 9378794d3a4e usr.bin/make/targ.c
--- a/usr.bin/make/targ.c       Wed Sep 23 02:32:04 2020 +0000
+++ b/usr.bin/make/targ.c       Wed Sep 23 03:06:38 2020 +0000
@@ -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 @@
     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,
diff -r 43b5d2d6b90a -r 9378794d3a4e usr.bin/make/unit-tests/Makefile
--- a/usr.bin/make/unit-tests/Makefile  Wed Sep 23 02:32:04 2020 +0000
+++ b/usr.bin/make/unit-tests/Makefile  Wed Sep 23 03:06:38 2020 +0000
@@ -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-default
 TESTS+=                deptgt-delete_on_error
 TESTS+=                deptgt-end
+TESTS+=                deptgt-end-jobs
 TESTS+=                deptgt-error
 TESTS+=                deptgt-ignore
 TESTS+=                deptgt-interrupt
diff -r 43b5d2d6b90a -r 9378794d3a4e usr.bin/make/unit-tests/deptgt-end-jobs.exp
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/usr.bin/make/unit-tests/deptgt-end-jobs.exp       Wed Sep 23 03:06:38 2020 +0000
@@ -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
diff -r 43b5d2d6b90a -r 9378794d3a4e usr.bin/make/unit-tests/deptgt-end-jobs.mk
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/usr.bin/make/unit-tests/deptgt-end-jobs.mk        Wed Sep 23 03:06:38 2020 +0000
@@ -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}'



Home | Main Index | Thread Index | Old Index