diff mbox series

Use response files from the driver in more cases

Message ID 254126280.ziJDXHqnrs@polaris
State New
Headers show
Series Use response files from the driver in more cases | expand

Commit Message

Eric Botcazou April 26, 2018, 10:51 a.m. UTC
Hi,

response files were added to the GNU toolchain to work around length limits 
put on the command line by some OSes, typically Windows.  For the compiler, 
the implementation is as follows: if a response file is passed to the driver 
at link time, typically containing the list of object files, then the list of 
object files is passed to collect2 in a (second) response file and, if the 
linker is the GNU linker, collect2 then passes *all* the arguments to the 
linker in a (third) response file.

So you can flawlessly pass a gigantic list of object files at link time.  But 
this doesn't work for a gigantic list of -L switches, because the driver will 
pass them to collect2 on the command line, even if they were passed to it in a 
response file and even if they'll be passed to the GNU linker this way again.

In other words, the ball is dropped between driver and collect2.  Similarly, 
if you pass a gigantic list of -I switches at compile time, the driver will 
pass them to cc1 on the command line, even if they were passed to it in a 
response file.

Therefore the attached patch makes it so that the -I switches and -L switches 
are passed by the driver to cc1 and collect2 respectively in a response file, 
if they were passed to it this way.  That's implemented by extending the 
spec syntax with:

 %@{...}
	like %{...} but puts the result into a FILE and substitutes @FILE
	if an @file argument has been supplied.

and adding '@' to the LINK_COMMAND_SPEC and cpp_unique_options specs.

Tested on x86-64/Linux and Windows.  We have been using this in production for 
a couple of months.  OK for the mainline?


2018-04-26  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.c: Document new %@{...} sequence.
	(LINK_COMMAND_SPEC): Use it for the -L switches.
	(cpp_unique_options): Use it for the -I switches.
	(at_file_argbuf): New global variable.
	(in_at_file): Likewise.
	(alloc_args): Create at_file_argbuf.
	(clear_args): Truncate at_file_argbuf.
	(store_arg): If in_at_file, push the argument onto at_file_argbuf.
	(open_at_file): New function.
	(close_at_file): Likewise.
	(create_at_file): Delete.
	(do_spec_1) <'i'>: Use open_at_file/close_at_file.
	<'o'>: Likewise.
	<'@'>: New case.
	(validate_switches_from_spec): Deal with %@{...} sequence.
	(validate_switches): Likewise.
	(driver::finalize): Call clear_args.

Comments

Tamar Christina June 14, 2018, 10:55 a.m. UTC | #1
Sending to list as well.

> -----Original Message-----
> From: Tamar Christina
> Sent: Thursday, June 14, 2018 11:54
> To: 'Eric Botcazou' <ebotcazou@adacore.com>; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] Use response files from the driver in more cases
> 
> This also fixes PR86030,
> 
> Thanks Eric!
> 
> Regards,
> Tamar
> 
> > -----Original Message-----
> > From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> On
> > Behalf Of Eric Botcazou
> > Sent: Thursday, April 26, 2018 11:52
> > To: gcc-patches@gcc.gnu.org
> > Subject: [PATCH] Use response files from the driver in more cases
> >
> > Hi,
> >
> > response files were added to the GNU toolchain to work around length
> > limits put on the command line by some OSes, typically Windows.  For
> > the compiler, the implementation is as follows: if a response file is
> > passed to the driver at link time, typically containing the list of
> > object files, then the list of object files is passed to collect2 in a
> > (second) response file and, if the linker is the GNU linker, collect2
> > then passes *all* the arguments to the linker in a (third) response file.
> >
> > So you can flawlessly pass a gigantic list of object files at link
> > time.  But this doesn't work for a gigantic list of -L switches,
> > because the driver will pass them to collect2 on the command line,
> > even if they were passed to it in a response file and even if they'll be
> passed to the GNU linker this way again.
> >
> > In other words, the ball is dropped between driver and collect2.
> > Similarly, if you pass a gigantic list of -I switches at compile time,
> > the driver will pass them to cc1 on the command line, even if they were
> passed to it in a response file.
> >
> > Therefore the attached patch makes it so that the -I switches and -L
> > switches are passed by the driver to cc1 and collect2 respectively in
> > a response file, if they were passed to it this way.  That's
> > implemented by extending the spec syntax with:
> >
> >  %@{...}
> > 	like %{...} but puts the result into a FILE and substitutes @FILE
> > 	if an @file argument has been supplied.
> >
> > and adding '@' to the LINK_COMMAND_SPEC and cpp_unique_options
> specs.
> >
> > Tested on x86-64/Linux and Windows.  We have been using this in
> > production for a couple of months.  OK for the mainline?
> >
> >
> > 2018-04-26  Eric Botcazou  <ebotcazou@adacore.com>
> >
> > 	* gcc.c: Document new %@{...} sequence.
> > 	(LINK_COMMAND_SPEC): Use it for the -L switches.
> > 	(cpp_unique_options): Use it for the -I switches.
> > 	(at_file_argbuf): New global variable.
> > 	(in_at_file): Likewise.
> > 	(alloc_args): Create at_file_argbuf.
> > 	(clear_args): Truncate at_file_argbuf.
> > 	(store_arg): If in_at_file, push the argument onto at_file_argbuf.
> > 	(open_at_file): New function.
> > 	(close_at_file): Likewise.
> > 	(create_at_file): Delete.
> > 	(do_spec_1) <'i'>: Use open_at_file/close_at_file.
> > 	<'o'>: Likewise.
> > 	<'@'>: New case.
> > 	(validate_switches_from_spec): Deal with %@{...} sequence.
> > 	(validate_switches): Likewise.
> > 	(driver::finalize): Call clear_args.
> >
> > --
> > Eric Botcazou
Eric Botcazou June 14, 2018, 11:21 a.m. UTC | #2
> This also fixes PR86030,

Glad to see we're not alone on the Windows ship. :-)

> Thanks Eric!

You're welcome.
diff mbox series

Patch

Index: gcc.c
===================================================================
--- gcc.c	(revision 259642)
+++ gcc.c	(working copy)
@@ -476,8 +476,11 @@  or with constant text in a single argume
 	into the sequence of arguments that %o will substitute later.
  %V	indicates that this compilation produces no "output file".
  %W{...}
-	like %{...} but mark last argument supplied within
-	as a file to be deleted on failure.
+	like %{...} but marks the last argument supplied within as a file
+	to be deleted on failure.
+ %@{...}
+	like %{...} but puts the result into a FILE and substitutes @FILE
+	if an @file argument has been supplied.
  %o	substitutes the names of all the output files, with spaces
 	automatically placed around them.  You should write spaces
 	around the %o as well or the results are undefined.
@@ -1037,7 +1040,7 @@  proper position among the other output f
    "%{fuse-ld=*:-fuse-ld=%*} " LINK_COMPRESS_DEBUG_SPEC \
    "%X %{o*} %{e*} %{N} %{n} %{r}\
     %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!nostartfiles:%S}} \
-    %{static|no-pie|static-pie:} %{L*} %(mfwrap) %(link_libgcc) " \
+    %{static|no-pie|static-pie:} %@{L*} %(mfwrap) %(link_libgcc) " \
     VTABLE_VERIFICATION_SPEC " " SANITIZER_EARLY_SPEC " %o " CHKP_SPEC " \
     %{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*:%*} 1):\
 	%:include(libgomp.spec)%(link_gomp)}\
@@ -1112,7 +1115,7 @@  static const char *trad_capable_cpp =
    therefore no dependency entry, confuses make into thinking a .o
    file that happens to exist is up-to-date.  */
 static const char *cpp_unique_options =
-"%{!Q:-quiet} %{nostdinc*} %{C} %{CC} %{v} %{I*&F*} %{P} %I\
+"%{!Q:-quiet} %{nostdinc*} %{C} %{CC} %{v} %@{I*&F*} %{P} %I\
  %{MD:-MD %{!o:%b.d}%{o*:%.d%*}}\
  %{MMD:-MMD %{!o:%b.d}%{o*:%.d%*}}\
  %{M} %{MM} %{MF*} %{MG} %{MP} %{MQ*} %{MT*}\
@@ -1927,9 +1930,14 @@  set_spec (const char *name, const char *
 typedef const char *const_char_p; /* For DEF_VEC_P.  */
 
 /* Vector of pointers to arguments in the current line of specifications.  */
-
 static vec<const_char_p> argbuf;
 
+/* Likewise, but for the current @file.  */
+static vec<const_char_p> at_file_argbuf;
+
+/* Whether an @file is currently open.  */
+static bool in_at_file = false;
+
 /* Were the options -c, -S or -E passed.  */
 static int have_c = 0;
 
@@ -1969,6 +1977,7 @@  static void
 alloc_args (void)
 {
   argbuf.create (10);
+  at_file_argbuf.create (10);
 }
 
 /* Clear out the vector of arguments (after a command is executed).  */
@@ -1977,6 +1986,7 @@  static void
 clear_args (void)
 {
   argbuf.truncate (0);
+  at_file_argbuf.truncate (0);
 }
 
 /* Add one argument to the vector at the end.
@@ -1989,7 +1999,10 @@  clear_args (void)
 static void
 store_arg (const char *arg, int delete_always, int delete_failure)
 {
-  argbuf.safe_push (arg);
+  if (in_at_file)
+    at_file_argbuf.safe_push (arg);
+  else
+    argbuf.safe_push (arg);
 
   if (delete_always || delete_failure)
     {
@@ -2002,6 +2015,67 @@  store_arg (const char *arg, int delete_a
       record_temp_file (arg, delete_always, delete_failure);
     }
 }
+
+/* Open a temporary @file into which subsequent arguments will be stored.  */
+
+static void
+open_at_file (void)
+{
+   if (in_at_file)
+     fatal_error (input_location, "cannot open nested response file");
+   else
+     in_at_file = true;
+}
+
+/* Close the temporary @file and add @file to the argument list.  */
+
+static void
+close_at_file (void)
+{
+  if (!in_at_file)
+    fatal_error (input_location, "cannot close nonexistent response file");
+
+  in_at_file = false;
+
+  const unsigned int n_args = at_file_argbuf.length ();
+  if (n_args == 0)
+    return;
+
+  char **argv = (char **) alloca (sizeof (char *) * (n_args + 1));
+  char *temp_file = make_temp_file ("");
+  char *at_argument = concat ("@", temp_file, NULL);
+  FILE *f = fopen (temp_file, "w");
+  int status;
+  unsigned int i;
+
+  /* Copy the strings over.  */
+  for (i = 0; i < n_args; i++)
+    argv[i] = CONST_CAST (char *, at_file_argbuf[i]);
+  argv[i] = NULL;
+
+  at_file_argbuf.truncate (0);
+
+  if (f == NULL)
+    fatal_error (input_location, "could not open temporary response file %s",
+		 temp_file);
+
+  status = writeargv (argv, f);
+
+  if (status)
+    fatal_error (input_location,
+		 "could not write to temporary response file %s",
+		 temp_file);
+
+  status = fclose (f);
+
+  if (status == EOF)
+    fatal_error (input_location, "could not close temporary response file %s",
+		 temp_file);
+
+  store_arg (at_argument, 0, 0);
+
+  record_temp_file (temp_file, !save_temps_flag, !save_temps_flag);
+}
 
 /* Load specs from a file name named FILENAME, replacing occurrences of
    various different types of line-endings, \r\n, \n\r and just \r, with
@@ -5084,39 +5158,6 @@  spec_path (char *path, void *data)
   return NULL;
 }
 
-/* Create a temporary FILE with the contents of ARGV. Add @FILE to the
-   argument list. */
-
-static void
-create_at_file (char **argv)
-{
-  char *temp_file = make_temp_file ("");
-  char *at_argument = concat ("@", temp_file, NULL);
-  FILE *f = fopen (temp_file, "w");
-  int status;
-
-  if (f == NULL)
-    fatal_error (input_location, "could not open temporary response file %s",
-		 temp_file);
-
-  status = writeargv (argv, f);
-
-  if (status)
-    fatal_error (input_location,
-		 "could not write to temporary response file %s",
-		 temp_file);
-
-  status = fclose (f);
-
-  if (EOF == status)
-    fatal_error (input_location, "could not close temporary response file %s",
-		 temp_file);
-
-  store_arg (at_argument, 0, 0);
-
-  record_temp_file (temp_file, !save_temps_flag, !save_temps_flag);
-}
-
 /* True if we should compile INFILE. */
 
 static bool
@@ -5529,41 +5570,22 @@  do_spec_1 (const char *spec, int inswitc
 	  case 'i':
 	    if (combine_inputs)
 	      {
+		/* We are going to expand `%i' into `@FILE', where FILE
+		   is a newly-created temporary filename.  The filenames
+		   that would usually be expanded in place of %o will be
+		   written to the temporary file.  */
 		if (at_file_supplied)
-		  {
-		    /* We are going to expand `%i' to `@FILE', where FILE
-		       is a newly-created temporary filename.  The filenames
-		       that would usually be expanded in place of %o will be
-		       written to the temporary file.  */
-		    char **argv;
-		    int n_files = 0;
-		    int j;
-
-		    for (i = 0; i < n_infiles; i++)
-		      if (compile_input_file_p (&infiles[i]))
-			n_files++;
-
-		    argv = (char **) alloca (sizeof (char *) * (n_files + 1));
-
-		    /* Copy the strings over.  */
-		    for (i = 0, j = 0; i < n_infiles; i++)
-		      if (compile_input_file_p (&infiles[i]))
-			{
-			  argv[j] = CONST_CAST (char *, infiles[i].name);
-			  infiles[i].compiled = true;
-			  j++;
-			}
-		    argv[j] = NULL;
+		  open_at_file ();
 
-		    create_at_file (argv);
-		  }
-		else
-		  for (i = 0; (int) i < n_infiles; i++)
-		    if (compile_input_file_p (&infiles[i]))
-		      {
-			store_arg (infiles[i].name, 0, 0);
-			infiles[i].compiled = true;
-		      }
+		for (i = 0; (int) i < n_infiles; i++)
+		  if (compile_input_file_p (&infiles[i]))
+		    {
+		      store_arg (infiles[i].name, 0, 0);
+		      infiles[i].compiled = true;
+		    }
+
+		if (at_file_supplied)
+		  close_at_file ();
 	      }
 	    else
 	      {
@@ -5636,45 +5658,20 @@  do_spec_1 (const char *spec, int inswitc
 	    break;
 
 	  case 'o':
-	    {
-	      int max = n_infiles;
-	      max += lang_specific_extra_outfiles;
+	    /* We are going to expand `%o' into `@FILE', where FILE
+	       is a newly-created temporary filename.  The filenames
+	       that would usually be expanded in place of %o will be
+	       written to the temporary file.  */
+	    if (at_file_supplied)
+	      open_at_file ();
+
+	    for (i = 0; i < n_infiles + lang_specific_extra_outfiles; i++)
+	      if (outfiles[i])
+		store_arg (outfiles[i], 0, 0);
 
-              if (HAVE_GNU_LD && at_file_supplied)
-                {
-                  /* We are going to expand `%o' to `@FILE', where FILE
-                     is a newly-created temporary filename.  The filenames
-                     that would usually be expanded in place of %o will be
-                     written to the temporary file.  */
-
-                  char **argv;
-                  int n_files, j;
-
-                  /* Convert OUTFILES into a form suitable for writeargv.  */
-
-                  /* Determine how many are non-NULL.  */
-                  for (n_files = 0, i = 0; i < max; i++)
-                    n_files += outfiles[i] != NULL;
-
-                  argv = (char **) alloca (sizeof (char *) * (n_files + 1));
-
-                  /* Copy the strings over.  */
-                  for (i = 0, j = 0; i < max; i++)
-                    if (outfiles[i])
-                      {
-                        argv[j] = CONST_CAST (char *, outfiles[i]);
-                        j++;
-                      }
-                  argv[j] = NULL;
-
-		  create_at_file (argv);
-                }
-              else
-                for (i = 0; i < max; i++)
-	          if (outfiles[i])
-		    store_arg (outfiles[i], 0, 0);
-	      break;
-	    }
+	    if (at_file_supplied)
+	      close_at_file ();
+	    break;
 
 	  case 'O':
 	    obstack_grow (&obstack, TARGET_OBJECT_SUFFIX, strlen (TARGET_OBJECT_SUFFIX));
@@ -5715,6 +5712,20 @@  do_spec_1 (const char *spec, int inswitc
 	      break;
 	    }
 
+	  case '@':
+	    /* Handle the {...} following the %@.  */
+	    if (*p != '{')
+	      fatal_error (input_location,
+			   "spec %qs has invalid %<%%@%c%>", spec, *p);
+	    if (at_file_supplied)
+	      open_at_file ();
+	    p = handle_braces (p + 1);
+	    if (at_file_supplied)
+	      close_at_file ();
+	    if (p == 0)
+	      return -1;
+	    break;
+
 	  /* %x{OPTION} records OPTION for %X to output.  */
 	  case 'x':
 	    {
@@ -8504,7 +8515,11 @@  validate_switches_from_spec (const char
   const char *p = spec;
   char c;
   while ((c = *p++))
-    if (c == '%' && (*p == '{' || *p == '<' || (*p == 'W' && *++p == '{')))
+    if (c == '%'
+	&& (*p == '{'
+	    || *p == '<'
+	    || (*p == 'W' && *++p == '{')
+	    || (*p == '@' && *++p == '{')))
       /* We have a switch spec.  */
       p = validate_switches (p + 1, user);
 }
@@ -8586,6 +8601,8 @@  next_member:
 		p = validate_switches (p+1, user_spec);
 	      else if (p[0] == 'W' && p[1] == '{')
 		p = validate_switches (p+2, user_spec);
+	      else if (p[0] == '@' && p[1] == '{')
+		p = validate_switches (p+2, user_spec);
 	    }
 	  else
 	    p++;
@@ -10144,7 +10161,7 @@  driver::finalize ()
 
   processing_spec_function = 0;
 
-  argbuf.truncate (0);
+  clear_args ();
 
   have_c = 0;
   have_o = 0;