From 3112c8799358cb5d051e0b63d0e916357169942c Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sat, 4 Aug 2018 12:18:39 -0400 Subject: [PATCH] Queue failed fork() (etc.) to be handled like any other failed job. If we failed to fork() we were essentially exiting make immediately without respect to ignore flags, etc. On one hand that makes sense because if you can't fork you're in real trouble, but it doesn't work so well on systems where we don't fork at all. Instead, treat a fork error like any other error by delaying the handling until the next call to reap_children(). Any child with a PID of -1 is considered to have died before starting so check these first without waiting for them. * src/commands.c (fatal_error_signal): Don't kill children that never started. * src/function.c (func_shell_base): Handle cleanup properly if the child doesn't start. * src/job.c (reap_children): Check for children that died before starting and handle them without waiting for the PID. (start_job_command): Free memory when the child doesn't start. (start_waiting_job): Don't manage children who never started. (child_execute_job): If the fork fails return PID -1. * src/vmsjobs.c: Check for children that never started. * tests/run_make_tests.pl: Parse config.status to get all options. --- src/commands.c | 6 +- src/function.c | 28 +++++---- src/job.c | 123 ++++++++++++++++++---------------------- src/job.h | 8 ++- src/vmsjobs.c | 3 +- tests/run_make_tests.pl | 13 ++++- 6 files changed, 93 insertions(+), 88 deletions(-) diff --git a/src/commands.c b/src/commands.c index 49cd5e0e..6dee0286 100644 --- a/src/commands.c +++ b/src/commands.c @@ -539,7 +539,7 @@ fatal_error_signal (int sig) { struct child *c; for (c = children; c != 0; c = c->next) - if (!c->remote) + if (!c->remote && c->pid > 0) (void) kill (c->pid, SIGTERM); } @@ -560,7 +560,7 @@ fatal_error_signal (int sig) /* Remote children won't automatically get signals sent to the process group, so we must send them. */ for (c = children; c != 0; c = c->next) - if (c->remote) + if (c->remote && c->pid > 0) (void) remote_kill (c->pid, sig); for (c = children; c != 0; c = c->next) @@ -661,7 +661,7 @@ delete_child_targets (struct child *child) { struct dep *d; - if (child->deleted) + if (child->deleted || child->pid < 0) return; /* Delete the target file if it changed. */ diff --git a/src/function.c b/src/function.c index 2b3a5386..12e0ef2f 100644 --- a/src/function.c +++ b/src/function.c @@ -1698,7 +1698,7 @@ func_shell_base (char *o, char **argv, int trim_newlines) #ifdef __MSDOS__ FILE *fpipe; #endif - char **command_argv; + char **command_argv = NULL; const char *error_prefix; char **envp; int pipedes[2]; @@ -1761,7 +1761,8 @@ func_shell_base (char *o, char **argv, int trim_newlines) if (pipedes[0] < 0) { perror_with_name (error_prefix, "pipe"); - return o; + pid = -1; + goto done; } #elif defined(WINDOWS32) @@ -1774,14 +1775,16 @@ func_shell_base (char *o, char **argv, int trim_newlines) /* Open of the pipe failed, mark as failed execution. */ shell_completed (127, 0); perror_with_name (error_prefix, "pipe"); - return o; + pid = -1; + goto done; } #else if (pipe (pipedes) < 0) { perror_with_name (error_prefix, "pipe"); - return o; + pid = -1; + goto done; } /* Close handles that are unnecessary for the child process. */ @@ -1798,10 +1801,7 @@ func_shell_base (char *o, char **argv, int trim_newlines) } if (pid < 0) - { - perror_with_name (error_prefix, "fork"); - return o; - } + goto done; #endif { @@ -1814,10 +1814,6 @@ func_shell_base (char *o, char **argv, int trim_newlines) #ifndef __MSDOS__ shell_function_completed = 0; - /* Free the storage only the child needed. */ - free (command_argv[0]); - free (command_argv); - /* Close the write side of the pipe. We test for -1, since pipedes[1] is -1 on MS-Windows, and some versions of MS libraries barf when 'close' is called with -1. */ @@ -1892,6 +1888,14 @@ func_shell_base (char *o, char **argv, int trim_newlines) free (buffer); } + done: + if (command_argv) + { + /* Free the storage only the child needed. */ + free (command_argv[0]); + free (command_argv); + } + return o; } diff --git a/src/job.c b/src/job.c index 47ce73db..0cd13eb5 100644 --- a/src/job.c +++ b/src/job.c @@ -684,10 +684,22 @@ reap_children (int block, int err) any_remote = 0; any_local = shell_function_pid != 0; - for (c = children; c != 0; c = c->next) + lastc = 0; + for (c = children; c != 0; lastc = c, c = c->next) { any_remote |= c->remote; any_local |= ! c->remote; + + /* If pid < 0, this child never even started. Handle it. */ + if (c->pid < 0) + { + exit_sig = 0; + coredump = 0; + /* According to POSIX, 127 is used for command not found. */ + exit_code = 127; + goto process_child; + } + DB (DB_JOBS, (_("Live child %p (%s) PID %s %s\n"), c, c->file->name, pid2str (c->pid), c->remote ? _(" (remote)") : "")); @@ -752,10 +764,6 @@ reap_children (int block, int err) exit_code = WEXITSTATUS (status); exit_sig = WIFSIGNALED (status) ? WTERMSIG (status) : 0; coredump = WCOREDUMP (status); - - /* If we have started jobs in this second, remove one. */ - if (job_counter) - --job_counter; } else { @@ -883,6 +891,16 @@ reap_children (int block, int err) Ignore it; it was inherited from our invoker. */ continue; + DB (DB_JOBS, (exit_sig == 0 && exit_code == 0 + ? _("Reaping losing child %p PID %s %s\n") + : _("Reaping winning child %p PID %s %s\n"), + c, pid2str (c->pid), c->remote ? _(" (remote)") : "")); + + /* If we have started jobs in this second, remove one. */ + if (job_counter) + --job_counter; + + process_child: /* Determine the failure status: 0 for success, 1 for updating target in question mode, 2 for anything else. */ if (exit_sig == 0 && exit_code == 0) @@ -892,11 +910,6 @@ reap_children (int block, int err) else child_failed = MAKE_FAILURE; - DB (DB_JOBS, (child_failed - ? _("Reaping losing child %p PID %s %s\n") - : _("Reaping winning child %p PID %s %s\n"), - c, pid2str (c->pid), c->remote ? _(" (remote)") : "")); - if (c->sh_batch_file) { int rm_status; @@ -1004,7 +1017,7 @@ reap_children (int block, int err) /* At this point c->file->update_status is success or failed. But c->file->command_state is still cs_running if all the commands - ran; notice_finish_file looks for cs_running to tell it that + ran; notice_finished_file looks for cs_running to tell it that it's interesting to check the file's modtime again now. */ if (! handling_fatal_signal) @@ -1013,9 +1026,6 @@ reap_children (int block, int err) update_status to its also_make files. */ notice_finished_file (c->file); - DB (DB_JOBS, (_("Removing child %p PID %s%s from chain.\n"), - c, pid2str (c->pid), c->remote ? _(" (remote)") : "")); - /* Block fatal signals while frobnicating the list, so that children and job_slots_used are always consistent. Otherwise a fatal signal arriving after the child is off the chain and @@ -1023,9 +1033,15 @@ reap_children (int block, int err) live and call reap_children again. */ block_sigs (); - /* There is now another slot open. */ - if (job_slots_used > 0) - --job_slots_used; + if (c->pid > 0) + { + DB (DB_JOBS, (_("Removing child %p PID %s%s from chain.\n"), + c, pid2str (c->pid), c->remote ? _(" (remote)") : "")); + + /* There is now another slot open. */ + if (job_slots_used > 0) + --job_slots_used; + } /* Remove the child from the chain and free it. */ if (lastc == 0) @@ -1110,8 +1126,10 @@ start_job_command (struct child *child) int flags; char *p; #ifdef VMS +# define FREE_ARGV(_a) char *argv; #else +# define FREE_ARGV(_a) do{ if (_a) { free ((_a)[0]); free (_a); } }while(0) char **argv; #endif @@ -1229,10 +1247,7 @@ start_job_command (struct child *child) error is 2. */ if (argv != 0 && question_flag && !(flags & COMMANDS_RECURSE)) { -#ifndef VMS - free (argv[0]); - free (argv); -#endif + FREE_ARGV (argv); #ifdef VMS /* On VMS, argv[0] can be a null string here */ if (argv[0] != 0) @@ -1250,13 +1265,7 @@ start_job_command (struct child *child) { /* Go on to the next command. It might be the recursive one. We construct ARGV only to find the end of the command line. */ -#ifndef VMS - if (argv) - { - free (argv[0]); - free (argv); - } -#endif + FREE_ARGV (argv); argv = 0; } @@ -1332,8 +1341,7 @@ start_job_command (struct child *child) && (argv[2] && argv[2][0] == ':' && argv[2][1] == '\0') && argv[3] == NULL) { - free (argv[0]); - free (argv); + FREE_ARGV (argv); goto next_command; } #endif /* !VMS && !_AMIGA */ @@ -1342,10 +1350,7 @@ start_job_command (struct child *child) if (just_print_flag && !(flags & COMMANDS_RECURSE)) { -#ifndef VMS - free (argv[0]); - free (argv); -#endif + FREE_ARGV (argv); goto next_command; } @@ -1412,11 +1417,7 @@ start_job_command (struct child *child) #ifdef VMS if (!child_execute_job (child, argv)) - { - /* Fork failed! */ - perror_with_name ("fork", ""); - goto error; - } + child->pid = -1; #else @@ -1424,18 +1425,11 @@ start_job_command (struct child *child) jobserver_pre_child (flags & COMMANDS_RECURSE); - child->pid = child_execute_job (&child->output, child->good_stdin, argv, child->environment); + child->pid = child_execute_job (&child->output, child->good_stdin, + argv, child->environment); environ = parent_environ; /* Restore value child may have clobbered. */ jobserver_post_child (flags & COMMANDS_RECURSE); - - if (child->pid < 0) - { - /* Fork failed! */ - unblock_sigs (); - perror_with_name ("fork", ""); - goto error; - } #endif /* !VMS */ } @@ -1550,33 +1544,25 @@ start_job_command (struct child *child) for (i = 0; argv[i]; i++) fprintf (stderr, "%s ", argv[i]); fprintf (stderr, _("\nCounted %d args in failed launch\n"), i); - goto error; + child->pid = -1; } } #endif /* WINDOWS32 */ #endif /* __MSDOS__ or Amiga or WINDOWS32 */ /* Bump the number of jobs started in this second. */ - ++job_counter; - - /* We are the parent side. Set the state to - say the commands are running and return. */ + if (child->pid >= 0) + ++job_counter; + /* Set the state to running. */ set_command_state (child->file, cs_running); /* Free the storage used by the child's argument list. */ -#ifndef VMS - free (argv[0]); - free (argv); -#endif + FREE_ARGV (argv); OUTPUT_UNSET(); - return; - error: - child->file->update_status = us_failed; - notice_finished_file (child->file); - OUTPUT_UNSET(); +#undef FREE_ARGV } /* Try to start a child running. @@ -1618,12 +1604,15 @@ start_waiting_job (struct child *c) { case cs_running: c->next = children; - DB (DB_JOBS, (_("Putting child %p (%s) PID %s%s on the chain.\n"), - c, c->file->name, pid2str (c->pid), - c->remote ? _(" (remote)") : "")); + if (c->pid > 0) + { + DB (DB_JOBS, (_("Putting child %p (%s) PID %s%s on the chain.\n"), + c, c->file->name, pid2str (c->pid), + c->remote ? _(" (remote)") : "")); + /* One more job slot is in use. */ + ++job_slots_used; + } children = c; - /* One more job slot is in use. */ - ++job_slots_used; unblock_sigs (); break; diff --git a/src/job.h b/src/job.h index afd9d8d8..99efa40d 100644 --- a/src/job.h +++ b/src/job.h @@ -25,10 +25,13 @@ struct child struct file *file; /* File being remade. */ char **environment; /* Environment for commands. */ + char *sh_batch_file; /* Script file for shell commands */ char **command_lines; /* Array of variable-expanded cmd lines. */ char *command_ptr; /* Ptr into command_lines[command_line]. */ + struct output output; /* Output for this child. */ + #ifdef VMS char *comname; /* Temporary command file name */ int efn; /* Completion event flag number */ @@ -37,7 +40,7 @@ struct child #endif unsigned int command_line; /* Index into command_lines. */ - struct output output; /* Output for this child. */ + pid_t pid; /* Child process's ID number. */ unsigned int remote:1; /* Nonzero if executing remotely. */ unsigned int noerror:1; /* Nonzero if commands contained a '-'. */ @@ -62,7 +65,8 @@ char **construct_command_argv (char *line, char **restp, struct file *file, #ifdef VMS int child_execute_job (struct child *child, char *argv); #else -pid_t child_execute_job (struct output *out, int good_stdin, char **argv, char **envp); +pid_t child_execute_job (struct output *out, int good_stdin, + char **argv, char **envp); #endif #ifdef _AMIGA diff --git a/src/vmsjobs.c b/src/vmsjobs.c index a8a4c38c..3a03eacc 100644 --- a/src/vmsjobs.c +++ b/src/vmsjobs.c @@ -191,7 +191,8 @@ astYHandler (void) { struct child *c; for (c = children; c != 0; c = c->next) - sys$delprc (&c->pid, 0, 0); + if (c->pid > 0) + sys$delprc (&c->pid, 0, 0); ctrlYPressed= 1; kill (getpid(),SIGQUIT); return SS$_NORMAL; diff --git a/tests/run_make_tests.pl b/tests/run_make_tests.pl index 2e063072..500524e3 100644 --- a/tests/run_make_tests.pl +++ b/tests/run_make_tests.pl @@ -71,9 +71,16 @@ use FindBin; use lib "$FindBin::Bin"; require "test_driver.pl"; -if (! eval { require "config-flags.pm" }) { - # Some target systems don't create config-flags.pm - %CONFIG_FLAGS = (); + +%CONFIG_FLAGS = (); + +my $statnm = "$FindBin::Bin/../config.status"; +if (open(my $fh, '<', $statnm)) { + while (my $line = <$fh>) { + $line =~ m/^[SD]\["([^\"]+)"\]=" *(.*)"/ and $CONFIG_FLAGS{$1} = $2; + } +} else { + warn "Failed to open $statnm: $!"; } # Some target systems might not have the POSIX module...