summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Smith <psmith@gnu.org>2013-09-15 15:05:18 -0400
committerPaul Smith <psmith@gnu.org>2013-09-15 15:21:33 -0400
commita4d8444b594e53b13659a9f2c64424418f255e27 (patch)
tree13315e9de3f19f3a144774a008bd3100633a31c3
parent0a81d50d66565fd3e930fadaadc4a5cb9381d840 (diff)
downloadgunmake-a4d8444b594e53b13659a9f2c64424418f255e27.tar.gz
[SV 39934] Verify jobserver FDs before something else uses them.
-rw-r--r--ChangeLog12
-rw-r--r--main.c178
-rw-r--r--tests/ChangeLog3
-rw-r--r--tests/scripts/features/parallelism20
4 files changed, 124 insertions, 89 deletions
diff --git a/ChangeLog b/ChangeLog
index 4e80e38..899b471 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,27 +1,29 @@
2013-09-15 Paul Smith <psmith@gnu.org>
- Fix Savannah bug #39203.
+ * main.c (main): Perform the validation of the jobserver FDs
+ early, before we read makefiles, to ensure that something hasn't
+ opened and used those FDs for some other reason.
+ Fixes Savannah bug #39934.
* main.c (main): Don't set MAKEFLAGS in the environment when we
restart. We have the original command line flags so keep the
original MAKEFLAGS settings as well.
+ Fixes Savannah bug #39203.
2013-09-14 Paul Smith <psmith@gnu.org>
- Fix Savannah bug #35248.
-
* main.c (decode_debug_flags): Add support for the "n" flag to
disable all debugging.
* make.1: Document the "n" (none) flag.
* doc/make.texi (Options Summary): Ditto.
* NEWS: Ditto.
-
- Fix Savannah bug #33134. Suggested by David Boyce <dsb@boyski.com>.
+ Fixes Savannah bug #35248.
* misc.c (close_stdout): Move to output.c.
* main.c (main): Move atexit call to output_init().
* makeint.h: Remove close_stdout() declaration.
* output.c (output_init): Add close_stdout at exit only if it's open.
+ Fixes Savannah bug #33134. Suggested by David Boyce <dsb@boyski.com>.
2013-09-14 Paul Smith <psmith@gnu.org>
diff --git a/main.c b/main.c
index 0fdf501..bfc125c 100644
--- a/main.c
+++ b/main.c
@@ -1393,6 +1393,15 @@ main (int argc, char **argv, char **envp)
decode_switches (argc, argv, 0);
+ /* Figure out the level of recursion. */
+ {
+ struct variable *v = lookup_variable (STRING_SIZE_TUPLE (MAKELEVEL_NAME));
+ if (v && v->value[0] != '\0' && v->value[0] != '-')
+ makelevel = (unsigned int) atoi (v->value);
+ else
+ makelevel = 0;
+ }
+
#ifdef WINDOWS32
if (suspend_flag)
{
@@ -1468,6 +1477,91 @@ main (int argc, char **argv, char **envp)
#endif /* WINDOWS32 */
#endif
+#ifdef MAKE_JOBSERVER
+ /* If the jobserver-fds option is seen, make sure that -j is reasonable.
+ This can't be usefully set in the makefile, and we want to verify the
+ FDs are valid before any other aspect of make has a chance to start
+ using them for something else. */
+
+ if (jobserver_fds)
+ {
+ const char *cp;
+ unsigned int ui;
+
+ for (ui=1; ui < jobserver_fds->idx; ++ui)
+ if (!streq (jobserver_fds->list[0], jobserver_fds->list[ui]))
+ fatal (NILF, _("internal error: multiple --jobserver-fds options"));
+
+ /* Now parse the fds string and make sure it has the proper format. */
+
+ cp = jobserver_fds->list[0];
+
+#ifdef WINDOWS32
+ if (! open_jobserver_semaphore (cp))
+ {
+ DWORD err = GetLastError ();
+ fatal (NILF, _("internal error: unable to open jobserver semaphore '%s': (Error %ld: %s)"),
+ cp, err, map_windows32_error_to_string (err));
+ }
+ DB (DB_JOBS, (_("Jobserver client (semaphore %s)\n"), cp));
+#else
+ if (sscanf (cp, "%d,%d", &job_fds[0], &job_fds[1]) != 2)
+ fatal (NILF,
+ _("internal error: invalid --jobserver-fds string '%s'"), cp);
+
+ DB (DB_JOBS,
+ (_("Jobserver client (fds %d,%d)\n"), job_fds[0], job_fds[1]));
+#endif
+
+ /* The combination of a pipe + !job_slots means we're using the
+ jobserver. If !job_slots and we don't have a pipe, we can start
+ infinite jobs. If we see both a pipe and job_slots >0 that means the
+ user set -j explicitly. This is broken; in this case obey the user
+ (ignore the jobserver pipe for this make) but print a message.
+ If we've restarted, we already printed this the first time. */
+
+ if (job_slots > 0)
+ {
+ if (! restarts)
+ error (NILF, _("warning: -jN forced in submake: disabling jobserver mode."));
+ }
+#ifndef WINDOWS32
+#define FD_OK(_f) ((fcntl ((_f), F_GETFD) != -1) || (errno != EBADF))
+ /* Create a duplicate pipe, that will be closed in the SIGCHLD
+ handler. If this fails with EBADF, the parent has closed the pipe
+ on us because it didn't think we were a submake. If so, print a
+ warning then default to -j1. */
+ else if (!FD_OK (job_fds[0]) || !FD_OK (job_fds[1])
+ || (job_rfd = dup (job_fds[0])) < 0)
+ {
+ if (errno != EBADF)
+ pfatal_with_name (_("dup jobserver"));
+
+ error (NILF,
+ _("warning: jobserver unavailable: using -j1. Add '+' to parent make rule."));
+ job_slots = 1;
+ job_fds[0] = job_fds[1] = -1;
+ }
+#endif
+
+ if (job_slots > 0)
+ {
+#ifdef WINDOWS32
+ free_jobserver_semaphore ();
+#else
+ if (job_fds[0] >= 0)
+ close (job_fds[0]);
+ if (job_fds[1] >= 0)
+ close (job_fds[1]);
+#endif
+ job_fds[0] = job_fds[1] = -1;
+ free (jobserver_fds->list);
+ free (jobserver_fds);
+ jobserver_fds = 0;
+ }
+ }
+#endif
+
/* The extra indirection through $(MAKE_COMMAND) is done
for hysterical raisins. */
define_variable_cname ("MAKE_COMMAND", argv[0], o_default, 0);
@@ -1554,16 +1648,7 @@ main (int argc, char **argv, char **envp)
* at the wrong place when it was first evaluated.
*/
no_default_sh_exe = !find_and_set_default_shell (NULL);
-
#endif /* WINDOWS32 */
- /* Figure out the level of recursion. */
- {
- struct variable *v = lookup_variable (STRING_SIZE_TUPLE (MAKELEVEL_NAME));
- if (v != 0 && v->value[0] != '\0' && v->value[0] != '-')
- makelevel = (unsigned int) atoi (v->value);
- else
- makelevel = 0;
- }
/* Except under -s, always do -w in sub-makes and under -C. */
if (!silent_flag && (directories != 0 || makelevel > 0))
@@ -1851,81 +1936,6 @@ main (int argc, char **argv, char **envp)
#endif
#ifdef MAKE_JOBSERVER
- /* If the jobserver-fds option is seen, make sure that -j is reasonable. */
-
- if (jobserver_fds)
- {
- const char *cp;
- unsigned int ui;
-
- for (ui=1; ui < jobserver_fds->idx; ++ui)
- if (!streq (jobserver_fds->list[0], jobserver_fds->list[ui]))
- fatal (NILF, _("internal error: multiple --jobserver-fds options"));
-
- /* Now parse the fds string and make sure it has the proper format. */
-
- cp = jobserver_fds->list[0];
-
-#ifdef WINDOWS32
- if (! open_jobserver_semaphore (cp))
- {
- DWORD err = GetLastError ();
- fatal (NILF, _("internal error: unable to open jobserver semaphore '%s': (Error %ld: %s)"),
- cp, err, map_windows32_error_to_string (err));
- }
- DB (DB_JOBS, (_("Jobserver client (semaphore %s)\n"), cp));
-#else
- if (sscanf (cp, "%d,%d", &job_fds[0], &job_fds[1]) != 2)
- fatal (NILF,
- _("internal error: invalid --jobserver-fds string '%s'"), cp);
-
- DB (DB_JOBS,
- (_("Jobserver client (fds %d,%d)\n"), job_fds[0], job_fds[1]));
-#endif
-
- /* The combination of a pipe + !job_slots means we're using the
- jobserver. If !job_slots and we don't have a pipe, we can start
- infinite jobs. If we see both a pipe and job_slots >0 that means the
- user set -j explicitly. This is broken; in this case obey the user
- (ignore the jobserver pipe for this make) but print a message.
- If we've restarted, we already printed this the first time. */
-
- if (job_slots > 0)
- {
- if (! restarts)
- error (NILF, _("warning: -jN forced in submake: disabling jobserver mode."));
- }
-#ifndef WINDOWS32
- /* Create a duplicate pipe, that will be closed in the SIGCHLD
- handler. If this fails with EBADF, the parent has closed the pipe
- on us because it didn't think we were a submake. If so, print a
- warning then default to -j1. */
- else if ((job_rfd = dup (job_fds[0])) < 0)
- {
- if (errno != EBADF)
- pfatal_with_name (_("dup jobserver"));
-
- error (NILF,
- _("warning: jobserver unavailable: using -j1. Add '+' to parent make rule."));
- job_slots = 1;
- }
-#endif
-
- if (job_slots > 0)
- {
-#ifdef WINDOWS32
- free_jobserver_semaphore ();
-#else
- close (job_fds[0]);
- close (job_fds[1]);
-#endif
- job_fds[0] = job_fds[1] = -1;
- free (jobserver_fds->list);
- free (jobserver_fds);
- jobserver_fds = 0;
- }
- }
-
/* If we have >1 slot but no jobserver-fds, then we're a top-level make.
Set up the pipe and install the fds option for our children. */
diff --git a/tests/ChangeLog b/tests/ChangeLog
index c629a0e..d97e7e2 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,5 +1,8 @@
2013-09-15 Paul Smith <psmith@gnu.org>
+ * scripts/features/parallelism: Test broken jobserver on recursion.
+ Test for Savannah bug #39934.
+
* scripts/options/eval: Verify --eval during restart.
Test for Savannah bug #39203.
diff --git a/tests/scripts/features/parallelism b/tests/scripts/features/parallelism
index a895517..6e8376d 100644
--- a/tests/scripts/features/parallelism
+++ b/tests/scripts/features/parallelism
@@ -254,6 +254,26 @@ true
');
}
+# Test recursion when make doesn't think it exists.
+# See Savannah bug #39934
+# Or Red Hat bug https://bugzilla.redhat.com/show_bug.cgi?id=885474
+
+open(MAKEFILE,"> Makefile2");
+print MAKEFILE <<EOF;
+vpath %.c $ENV{HOME}/
+foo:
+EOF
+close(MAKEFILE);
+
+run_make_test(q!
+default: ; @ #MAKEPATH# -f Makefile2
+!,
+ '-j2 --no-print-directory',
+"#MAKE#[1]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule.
+#MAKE#[1]: Nothing to be done for 'foo'.");
+
+unlink('Makefile2');
+
# Make sure that all jobserver FDs are closed if we need to re-exec the
# master copy.
#