Hi,
After having so much trouble working on the `execute' function inside
gcc.c, I decided to refactor it so that it could be more digestible.
Since I am using it on my branch, I am submitting this patch for
"battlefield" testing.
Bootstrapped and ran the testsuite on Linux x86_64.
-----
Refactor `execute' function to avoid exposing unnecessary details of
how it works, encapsulating each important step into a function.
gcc/ChangeLog
2020-05-18 Giuliano Belinassi <giuliano.belina...@usp.brr>
* gcc.c (struct command): Move from execute.
(await_commands_to_finish): New function.
(split_commands): Same as above.
(parse_argbuf): Same as above.
(execute): Refactor based on new functions.
>From df092d93b511a28180105a2db40777fb977e23da Mon Sep 17 00:00:00 2001
From: Giuliano Belinassi <giuliano.belina...@usp.br>
Date: Mon, 18 May 2020 21:08:43 -0300
Subject: [PATCH] Refactor execute in gcc.c
Refactor `execute' function to avoid exposing unnecessary details of
how it works, encapsulating each important step into a function.
gcc/ChangeLog
2020-05-18 Giuliano Belinassi <giuliano.belina...@usp.brr>
* gcc.c (struct command): Move from execute.
(await_commands_to_finish): New function.
(split_commands): Same as above.
(parse_argbuf): Same as above.
(execute): Refactor based on new functions.
---
gcc/ChangeLog | 8 +
gcc/gcc.c | 595 +++++++++++++++++++++++++++++---------------------
2 files changed, 353 insertions(+), 250 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 7f484da04f0..3ad1b9f7fb9 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2020-05-18 Giuliano Belinassi <giuliano.belina...@usp.brr>
+
+ * gcc.c (struct command): Move from execute.
+ (await_commands_to_finish): New function.
+ (split_commands): Same as above.
+ (parse_argbuf): Same as above.
+ (execute): Refactor based on new functions.
+
2020-05-18 Jason Merrill <ja...@redhat.com>
* aclocal.m4: Add ax_cxx_compile_stdcxx.m4.
diff --git a/gcc/gcc.c b/gcc/gcc.c
index b0d0308f127..082a5d1d4a8 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3019,157 +3019,105 @@ add_sysrooted_hdrs_prefix (struct path_prefix *pprefix, const char *prefix,
}
-/* Execute the command specified by the arguments on the current line of spec.
- When using pipes, this includes several piped-together commands
- with `|' between them.
- Return 0 if successful, -1 if failed. */
+struct command
+{
+ const char *prog; /* program name. */
+ const char **argv; /* vector of args. */
+};
+
+/* Print what commands will run. Return 0 if success, anything else on
+ error. */
static int
-execute (void)
+print_verbose (int n_commands, struct command commands[])
{
int i;
- int n_commands; /* # of command. */
- char *string;
- struct pex_obj *pex;
- struct command
- {
- const char *prog; /* program name. */
- const char **argv; /* vector of args. */
- };
- const char *arg;
-
- struct command *commands; /* each command buffer with above info. */
-
- gcc_assert (!processing_spec_function);
-
- if (wrapper_string)
- {
- string = find_a_file (&exec_prefixes,
- argbuf[0], X_OK, false);
- if (string)
- argbuf[0] = string;
- insert_wrapper (wrapper_string);
- }
- /* Count # of piped commands. */
- for (n_commands = 1, i = 0; argbuf.iterate (i, &arg); i++)
- if (strcmp (arg, "|") == 0)
- n_commands++;
-
- /* Get storage for each command. */
- commands = (struct command *) alloca (n_commands * sizeof (struct command));
-
- /* Split argbuf into its separate piped processes,
- and record info about each one.
- Also search for the programs that are to be run. */
-
- argbuf.safe_push (0);
-
- commands[0].prog = argbuf[0]; /* first command. */
- commands[0].argv = argbuf.address ();
-
- if (!wrapper_string)
- {
- string = find_a_file (&exec_prefixes, commands[0].prog, X_OK, false);
- if (string)
- commands[0].argv[0] = string;
- }
-
- for (n_commands = 1, i = 0; argbuf.iterate (i, &arg); i++)
- if (arg && strcmp (arg, "|") == 0)
- { /* each command. */
-#if defined (__MSDOS__) || defined (OS2) || defined (VMS)
- fatal_error (input_location, "%<-pipe%> not supported");
-#endif
- argbuf[i] = 0; /* Termination of command args. */
- commands[n_commands].prog = argbuf[i + 1];
- commands[n_commands].argv
- = &(argbuf.address ())[i + 1];
- string = find_a_file (&exec_prefixes, commands[n_commands].prog,
- X_OK, false);
- if (string)
- commands[n_commands].argv[0] = string;
- n_commands++;
- }
-
- /* If -v, print what we are about to do, and maybe query. */
+ /* For help listings, put a blank line between sub-processes. */
+ if (print_help_list)
+ fputc ('\n', stderr);
- if (verbose_flag)
+ /* Print each piped command as a separate line. */
+ for (i = 0; i < n_commands; i++)
{
- /* For help listings, put a blank line between sub-processes. */
- if (print_help_list)
- fputc ('\n', stderr);
+ const char *const *j;
- /* Print each piped command as a separate line. */
- for (i = 0; i < n_commands; i++)
+ if (verbose_only_flag)
{
- const char *const *j;
-
- if (verbose_only_flag)
+ for (j = commands[i].argv; *j; j++)
{
- for (j = commands[i].argv; *j; j++)
+ const char *p;
+ for (p = *j; *p; ++p)
+ if (!ISALNUM ((unsigned char) *p)
+ && *p != '_' && *p != '/' && *p != '-' && *p != '.')
+ break;
+ if (*p || !*j)
{
- const char *p;
+ fprintf (stderr, " \"");
for (p = *j; *p; ++p)
- if (!ISALNUM ((unsigned char) *p)
- && *p != '_' && *p != '/' && *p != '-' && *p != '.')
- break;
- if (*p || !*j)
{
- fprintf (stderr, " \"");
- for (p = *j; *p; ++p)
- {
- if (*p == '"' || *p == '\\' || *p == '$')
- fputc ('\\', stderr);
- fputc (*p, stderr);
- }
- fputc ('"', stderr);
+ if (*p == '"' || *p == '\\' || *p == '$')
+ fputc ('\\', stderr);
+ fputc (*p, stderr);
}
- /* If it's empty, print "". */
- else if (!**j)
- fprintf (stderr, " \"\"");
- else
- fprintf (stderr, " %s", *j);
+ fputc ('"', stderr);
}
- }
- else
- for (j = commands[i].argv; *j; j++)
/* If it's empty, print "". */
- if (!**j)
+ else if (!**j)
fprintf (stderr, " \"\"");
else
fprintf (stderr, " %s", *j);
-
- /* Print a pipe symbol after all but the last command. */
- if (i + 1 != n_commands)
- fprintf (stderr, " |");
- fprintf (stderr, "\n");
+ }
}
- fflush (stderr);
- if (verbose_only_flag != 0)
- {
- /* verbose_only_flag should act as if the spec was
- executed, so increment execution_count before
- returning. This prevents spurious warnings about
- unused linker input files, etc. */
- execution_count++;
- return 0;
- }
+ else
+ for (j = commands[i].argv; *j; j++)
+ /* If it's empty, print "". */
+ if (!**j)
+ fprintf (stderr, " \"\"");
+ else
+ fprintf (stderr, " %s", *j);
+
+ /* Print a pipe symbol after all but the last command. */
+ if (i + 1 != n_commands)
+ fprintf (stderr, " |");
+ fprintf (stderr, "\n");
+ }
+ fflush (stderr);
+ if (verbose_only_flag != 0)
+ {
+ /* verbose_only_flag should act as if the spec was
+ executed, so increment execution_count before
+ returning. This prevents spurious warnings about
+ unused linker input files, etc. */
+ execution_count++;
+ return 1;
+ }
#ifdef DEBUG
- fnotice (stderr, "\nGo ahead? (y or n) ");
- fflush (stderr);
- i = getchar ();
- if (i != '\n')
- while (getchar () != '\n')
- ;
-
- if (i != 'y' && i != 'Y')
- return 0;
+ fnotice (stderr, "\nGo ahead? (y or n) ");
+ fflush (stderr);
+ i = getchar ();
+ if (i != '\n')
+ while (getchar () != '\n')
+ ;
+
+ if (i != 'y' && i != 'Y')
+ return 1;
#endif /* DEBUG */
- }
+
+ return 0;
+}
#ifdef ENABLE_VALGRIND_CHECKING
+
+/* Append valgrind to each program. */
+
+static void
+append_valgrind (struct obstack *to_be_released,
+ int n_commands, struct command commands[])
+{
+ int i;
+
/* Run the each command through valgrind. To simplify prepending the
path to valgrind and the option "-q" (for quiet operation unless
something triggers), we allocate a separate argv array. */
@@ -3183,7 +3131,7 @@ execute (void)
for (argc = 0; commands[i].argv[argc] != NULL; argc++)
;
- argv = XALLOCAVEC (const char *, argc + 3);
+ argv = obstack_alloc (to_be_released, (argc + 3) * sizeof (const char *));
argv[0] = VALGRIND_PATH;
argv[1] = "-q";
@@ -3194,15 +3142,16 @@ execute (void)
commands[i].argv = argv;
commands[i].prog = argv[0];
}
+}
#endif
- /* Run each piped subprocess. */
+/* Launch a list of commands asynchronously. */
- pex = pex_init (PEX_USE_PIPES | ((report_times || report_times_to_file)
- ? PEX_RECORD_TIMES : 0),
- progname, temp_filename);
- if (pex == NULL)
- fatal_error (input_location, "%<pex_init%> failed: %m");
+static void
+async_launch_commands (struct pex_obj *pex,
+ int n_commands, struct command commands[])
+{
+ int i;
for (i = 0; i < n_commands; i++)
{
@@ -3229,150 +3178,296 @@ execute (void)
}
execution_count++;
+}
- /* Wait for all the subprocesses to finish. */
- {
- int *statuses;
- struct pex_time *times = NULL;
- int ret_code = 0;
+/* Wait for all the subprocesses to finish. Return 0 on success, -1 on
+ failure. */
- statuses = (int *) alloca (n_commands * sizeof (int));
- if (!pex_get_status (pex, n_commands, statuses))
- fatal_error (input_location, "failed to get exit status: %m");
+static int
+await_commands_to_finish (struct pex_obj *pex,
+ int n_commands, struct command commands[])
+{
- if (report_times || report_times_to_file)
- {
- times = (struct pex_time *) alloca (n_commands * sizeof (struct pex_time));
- if (!pex_get_times (pex, n_commands, times))
- fatal_error (input_location, "failed to get process times: %m");
- }
+ int *statuses;
+ struct pex_time *times = NULL;
+ int ret_code = 0, i;
- pex_free (pex);
+ statuses = (int *) alloca (n_commands * sizeof (int));
+ if (!pex_get_status (pex, n_commands, statuses))
+ fatal_error (input_location, "failed to get exit status: %m");
- for (i = 0; i < n_commands; ++i)
- {
- int status = statuses[i];
+ if (report_times || report_times_to_file)
+ {
+ times = (struct pex_time *) alloca (n_commands * sizeof (*times));
+ if (!pex_get_times (pex, n_commands, times))
+ fatal_error (input_location, "failed to get process times: %m");
+ }
- if (WIFSIGNALED (status))
- switch (WTERMSIG (status))
- {
- case SIGINT:
- case SIGTERM:
- /* SIGQUIT and SIGKILL are not available on MinGW. */
+ for (i = 0; i < n_commands; ++i)
+ {
+ int status = statuses[i];
+
+ if (WIFSIGNALED (status))
+ switch (WTERMSIG (status))
+ {
+ case SIGINT:
+ case SIGTERM:
+ /* SIGQUIT and SIGKILL are not available on MinGW. */
#ifdef SIGQUIT
- case SIGQUIT:
+ case SIGQUIT:
#endif
#ifdef SIGKILL
- case SIGKILL:
+ case SIGKILL:
#endif
- /* The user (or environment) did something to the
- inferior. Making this an ICE confuses the user into
- thinking there's a compiler bug. Much more likely is
- the user or OOM killer nuked it. */
- fatal_error (input_location,
- "%s signal terminated program %s",
- strsignal (WTERMSIG (status)),
- commands[i].prog);
- break;
+ /* The user (or environment) did something to the
+ inferior. Making this an ICE confuses the user into
+ thinking there's a compiler bug. Much more likely is
+ the user or OOM killer nuked it. */
+ fatal_error (input_location,
+ "%s signal terminated program %s",
+ strsignal (WTERMSIG (status)),
+ commands[i].prog);
+ break;
#ifdef SIGPIPE
- case SIGPIPE:
- /* SIGPIPE is a special case. It happens in -pipe mode
- when the compiler dies before the preprocessor is
- done, or the assembler dies before the compiler is
- done. There's generally been an error already, and
- this is just fallout. So don't generate another
- error unless we would otherwise have succeeded. */
- if (signal_count || greatest_status >= MIN_FATAL_STATUS)
- {
- signal_count++;
- ret_code = -1;
- break;
- }
+ case SIGPIPE:
+ /* SIGPIPE is a special case. It happens in -pipe mode
+ when the compiler dies before the preprocessor is
+ done, or the assembler dies before the compiler is
+ done. There's generally been an error already, and
+ this is just fallout. So don't generate another
+ error unless we would otherwise have succeeded. */
+ if (signal_count || greatest_status >= MIN_FATAL_STATUS)
+ {
+ signal_count++;
+ ret_code = -1;
+ break;
+ }
#endif
- /* FALLTHROUGH */
+ /* FALLTHROUGH. */
- default:
- /* The inferior failed to catch the signal. */
- internal_error_no_backtrace ("%s signal terminated program %s",
- strsignal (WTERMSIG (status)),
- commands[i].prog);
- }
- else if (WIFEXITED (status)
- && WEXITSTATUS (status) >= MIN_FATAL_STATUS)
- {
- /* For ICEs in cc1, cc1obj, cc1plus see if it is
- reproducible or not. */
- const char *p;
- if (flag_report_bug
- && WEXITSTATUS (status) == ICE_EXIT_CODE
- && i == 0
- && (p = strrchr (commands[0].argv[0], DIR_SEPARATOR))
- && ! strncmp (p + 1, "cc1", 3))
- try_generate_repro (commands[0].argv);
- if (WEXITSTATUS (status) > greatest_status)
- greatest_status = WEXITSTATUS (status);
- ret_code = -1;
+ default:
+ /* The inferior failed to catch the signal. */
+ internal_error_no_backtrace ("%s signal terminated program %s",
+ strsignal (WTERMSIG (status)),
+ commands[i].prog);
}
+ else if (WIFEXITED (status)
+ && WEXITSTATUS (status) >= MIN_FATAL_STATUS)
+ {
+ /* For ICEs in cc1, cc1obj, cc1plus see if it is
+ reproducible or not. */
+ const char *p;
+ if (flag_report_bug
+ && WEXITSTATUS (status) == ICE_EXIT_CODE
+ && i == 0
+ && (p = strrchr (commands[0].argv[0], DIR_SEPARATOR))
+ && ! strncmp (p + 1, "cc1", 3))
+ try_generate_repro (commands[0].argv);
+ if (WEXITSTATUS (status) > greatest_status)
+ greatest_status = WEXITSTATUS (status);
+ ret_code = -1;
+ }
- if (report_times || report_times_to_file)
- {
- struct pex_time *pt = ×[i];
- double ut, st;
+ if (report_times || report_times_to_file)
+ {
+ struct pex_time *pt = ×[i];
+ double ut, st;
- ut = ((double) pt->user_seconds
- + (double) pt->user_microseconds / 1.0e6);
- st = ((double) pt->system_seconds
- + (double) pt->system_microseconds / 1.0e6);
+ ut = ((double) pt->user_seconds
+ + (double) pt->user_microseconds / 1.0e6);
+ st = ((double) pt->system_seconds
+ + (double) pt->system_microseconds / 1.0e6);
- if (ut + st != 0)
- {
- if (report_times)
- fnotice (stderr, "# %s %.2f %.2f\n",
- commands[i].prog, ut, st);
+ if (ut + st != 0)
+ {
+ if (report_times)
+ fnotice (stderr, "# %s %.2f %.2f\n",
+ commands[i].prog, ut, st);
- if (report_times_to_file)
- {
- int c = 0;
- const char *const *j;
+ if (report_times_to_file)
+ {
+ int c = 0;
+ const char *const *j;
- fprintf (report_times_to_file, "%g %g", ut, st);
+ fprintf (report_times_to_file, "%g %g", ut, st);
- for (j = &commands[i].prog; *j; j = &commands[i].argv[++c])
- {
- const char *p;
- for (p = *j; *p; ++p)
- if (*p == '"' || *p == '\\' || *p == '$'
- || ISSPACE (*p))
- break;
+ for (j = &commands[i].prog; *j; j = &commands[i].argv[++c])
+ {
+ const char *p;
+ for (p = *j; *p; ++p)
+ if (*p == '"' || *p == '\\' || *p == '$'
+ || ISSPACE (*p))
+ break;
- if (*p)
- {
- fprintf (report_times_to_file, " \"");
- for (p = *j; *p; ++p)
- {
- if (*p == '"' || *p == '\\' || *p == '$')
- fputc ('\\', report_times_to_file);
- fputc (*p, report_times_to_file);
- }
- fputc ('"', report_times_to_file);
- }
- else
- fprintf (report_times_to_file, " %s", *j);
- }
+ if (*p)
+ {
+ fprintf (report_times_to_file, " \"");
+ for (p = *j; *p; ++p)
+ {
+ if (*p == '"' || *p == '\\' || *p == '$')
+ fputc ('\\', report_times_to_file);
+ fputc (*p, report_times_to_file);
+ }
+ fputc ('"', report_times_to_file);
+ }
+ else
+ fprintf (report_times_to_file, " %s", *j);
+ }
- fputc ('\n', report_times_to_file);
- }
- }
- }
+ fputc ('\n', report_times_to_file);
+ }
+ }
+ }
+ }
+
+ return ret_code;
+}
+
+/* Split a single command with pipes into several commands. */
+
+static void
+split_commands (vec<const_char_p> *argbuf_p,
+ int n_commands, struct command commands[])
+{
+ int i;
+ const char *arg;
+ vec<const_char_p> &argbuf = *argbuf_p;
+
+ for (n_commands = 1, i = 0; argbuf.iterate (i, &arg); i++)
+ if (arg && strcmp (arg, "|") == 0)
+ { /* each command. */
+ const char *string;
+#if defined (__MSDOS__) || defined (OS2) || defined (VMS)
+ fatal_error (input_location, "%<-pipe%> not supported");
+#endif
+ argbuf[i] = 0; /* Termination of command args. */
+ commands[n_commands].prog = argbuf[i + 1];
+ commands[n_commands].argv
+ = &(argbuf.address ())[i + 1];
+ string = find_a_file (&exec_prefixes, commands[n_commands].prog,
+ X_OK, false);
+ if (string)
+ commands[n_commands].argv[0] = string;
+ n_commands++;
}
+}
- if (commands[0].argv[0] != commands[0].prog)
- free (CONST_CAST (char *, commands[0].argv[0]));
+struct command
+*parse_argbuf (vec <const_char_p> *argbuf_p, int *n)
+{
+ int i, n_commands;
+ vec<const_char_p> &argbuf = *argbuf_p;
+ const char *arg;
+ struct command *commands;
- return ret_code;
- }
+ /* Count # of piped commands. */
+ for (n_commands = 1, i = 0; argbuf.iterate (i, &arg); i++)
+ if (strcmp (arg, "|") == 0)
+ n_commands++;
+
+ /* Get storage for each command. */
+ commands = XNEWVEC (struct command, n_commands);
+
+ /* Split argbuf into its separate piped processes,
+ and record info about each one.
+ Also search for the programs that are to be run. */
+
+ argbuf.safe_push (0);
+
+ commands[0].prog = argbuf[0]; /* first command. */
+ commands[0].argv = argbuf.address ();
+
+ split_commands (argbuf_p, n_commands, commands);
+
+ *n = n_commands;
+ return commands;
+}
+
+/* Execute the command specified by the arguments on the current line of spec.
+ When using pipes, this includes several piped-together commands
+ with `|' between them.
+
+ Return 0 if successful, -1 if failed. */
+
+static int
+execute (void)
+{
+ struct pex_obj *pex;
+ struct command *commands; /* each command buffer with program to call
+ and arguments. */
+ int n_commands; /* # of command. */
+ int ret = 0;
+
+ gcc_assert (!processing_spec_function);
+
+ if (wrapper_string)
+ {
+ char *string = find_a_file (&exec_prefixes, argbuf[0], X_OK, false);
+ if (string)
+ argbuf[0] = string;
+ insert_wrapper (wrapper_string);
+ }
+
+ /* Parse the argbuf into several commands. */
+ commands = parse_argbuf (&argbuf, &n_commands);
+
+ if (!wrapper_string)
+ {
+ char *string = find_a_file (&exec_prefixes, commands[0].prog,
+ X_OK, false);
+ if (string)
+ commands[0].argv[0] = string;
+ }
+
+ /* If -v, print what we are about to do, and maybe query. */
+
+ if (verbose_flag)
+ {
+ int ret_verbose = print_verbose (n_commands, commands);
+ if (ret_verbose > 0)
+ {
+ ret = 0;
+ goto cleanup;
+ }
+ }
+
+#ifdef ENABLE_VALGRIND_CHECKING
+ /* Stack of strings to be released on function return. */
+ struct obstack to_be_released;
+ obstack_init (&to_be_released);
+ append_valgrind (&to_be_released, n_commands, commands);
+#endif
+
+ /* Run each piped subprocess. */
+
+ pex = pex_init (PEX_USE_PIPES | ((report_times || report_times_to_file)
+ ? PEX_RECORD_TIMES : 0),
+ progname, temp_filename);
+ if (pex == NULL)
+ fatal_error (input_location, "%<pex_init%> failed: %m");
+
+ /* Lauch the commands. */
+ async_launch_commands (pex, n_commands, commands);
+
+ /* Await them to be done. */
+ ret = await_commands_to_finish (pex, n_commands, commands);
+
+ /* Cleanup. */
+ pex_free (pex);
+
+#ifdef ENABLE_VALGRIND_CHECKING
+ obstack_free (&to_be_released, NULL);
+#endif
+
+cleanup:
+ if (commands[0].argv[0] != commands[0].prog)
+ free (CONST_CAST (char *, commands[0].argv[0]));
+
+ free (commands);
+
+ return ret;
}
/* Find all the switches given to us
--
2.26.2
