[PATCH] Move resolution file handling to gcc.c, fix PRs 43857, 43371

[PATCH] Move resolution file handling to gcc.c, fix PRs 43857, 43371

[Richard Guenther]

This moves handling of the resolution file to the gcc driver so that
-save-temps automatically preserves it.  I changed how -fresolution
is tied to its argument to also fix the mentioned PRs.  I had
to fix store_arg to queue the temporary file for deletion instead
of the whole argument (I didn't find a spec that would always
generate a temporary file like /tmp/ccXXXXX, so we now have
resolution files like 1.res).

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

I verified that I now can cut&paste and re-run all lto1 commands
after re-running WPA phase (that I can now cut&paste manually)
that -fwhopr/-flto -v -save-temps -fuse-linker-plugin outputs
and that the resolution file is still deleted w/o -save-temps.

Probably lto-plugin needlessly deletes the wpa objects as that is
done by lto-wrapper already.  That's for a followup to (maybe) fix.

Tested ontop of the previous patch to move LTRANS driving to lto-wrapper.

Ok for trunk?

Thanks,
Richard.

2010-05-06  Richard Guenther <rguenther@suse.de>

	* gcc.c (LINK_COMMAND_SPEC): Provide a resolution file to
	the linker plugin.
	(store_arg): Queue temp_filename for deletion instead of
	the whole argument.

	lto/
	PR lto/43857
	PR lto/43371
	* lang.opt (fresolution): Change to ...
	(fresolution=): ... this.
	* lto-lang.c (lto_handle_option): Adjust.

	lto-plugin/
	* lto-plugin.c (free_2): Do not free resolution_file.
	(write_resolution): Check that we were passed a resolution file.
	(all_symbols_read_handler): Adjust.
	(cleanup_handler): Do not remove the resolution file.
	(process_option): Handle -fresolution=.

Index: gcc/gcc.c
===================================================================
*** gcc/gcc.c	(revision 159097)
--- gcc/gcc.c	(working copy)
*************** proper position among the other output f
*** 782,787 ****
--- 782,788 ----
      -plugin %(linker_plugin_file) \
      -plugin-opt=%(lto_wrapper) \
      -plugin-opt=%(lto_gcc) \
+     -plugin-opt=-fresolution=%u.res \
      %{static|static-libgcc:-plugin-opt=-pass-through=%(lto_libgcc)}	\
      %{static:-plugin-opt=-pass-through=-lc}	\
      %{O*:-plugin-opt=-O%*} \
*************** static int signal_count;
*** 2044,2049 ****
--- 2045,2067 ----
  /* Name with which this program was invoked.  */
  
  static const char *programname;
+ 
+ /* This is the common prefix we use to make temp file names.
+    It is chosen once for each run of this program.
+    It is substituted into a spec by %g or %j.
+    Thus, all temp file names contain this prefix.
+    In practice, all temp file names start with this prefix.
+ 
+    This prefix comes from the envvar TMPDIR if it is defined;
+    otherwise, from the P_tmpdir macro if that is defined;
+    otherwise, in /usr/tmp or /tmp;
+    or finally the current directory if all else fails.  */
+ 
+ static const char *temp_filename;
+ 
+ /* Length of the prefix.  */
+ 
+ static int temp_filename_length;
  \f
  /* Allocate the argument vector.  */
  
*************** store_arg (const char *arg, int delete_a
*** 2081,2087 ****
    if (strcmp (arg, "-o") == 0)
      have_o_argbuf_index = argbuf_index;
    if (delete_always || delete_failure)
!     record_temp_file (arg, delete_always, delete_failure);
  }
  \f
  /* Load specs from a file name named FILENAME, replacing occurrences of
--- 2099,2105 ----
    if (strcmp (arg, "-o") == 0)
      have_o_argbuf_index = argbuf_index;
    if (delete_always || delete_failure)
!     record_temp_file (temp_filename, delete_always, delete_failure);
  }
  \f
  /* Load specs from a file name named FILENAME, replacing occurrences of
*************** read_specs (const char *filename, int ma
*** 2384,2406 ****
  /* Record the names of temporary files we tell compilers to write,
     and delete them at the end of the run.  */
  
- /* This is the common prefix we use to make temp file names.
-    It is chosen once for each run of this program.
-    It is substituted into a spec by %g or %j.
-    Thus, all temp file names contain this prefix.
-    In practice, all temp file names start with this prefix.
- 
-    This prefix comes from the envvar TMPDIR if it is defined;
-    otherwise, from the P_tmpdir macro if that is defined;
-    otherwise, in /usr/tmp or /tmp;
-    or finally the current directory if all else fails.  */
- 
- static const char *temp_filename;
- 
- /* Length of the prefix.  */
- 
- static int temp_filename_length;
- 
  /* Define the list of temporary files to delete.  */
  
  struct temp_file
--- 2402,2407 ----
Index: gcc/lto/lang.opt
===================================================================
*** gcc/lto/lang.opt	(revision 159097)
--- gcc/lto/lang.opt	(working copy)
*************** fwpa
*** 36,43 ****
  LTO Report Var(flag_wpa) Optimization
  Run the link-time optimizer in whole program analysis (WPA) mode.
  
! fresolution
! LTO Separate
  The resolution file
  
  ; This comment is to ensure we retain the blank line above.
--- 36,43 ----
  LTO Report Var(flag_wpa) Optimization
  Run the link-time optimizer in whole program analysis (WPA) mode.
  
! fresolution=
! LTO Joined
  The resolution file
  
  ; This comment is to ensure we retain the blank line above.
Index: gcc/lto/lto-lang.c
===================================================================
*** gcc/lto/lto-lang.c	(revision 159097)
--- gcc/lto/lto-lang.c	(working copy)
*************** lto_handle_option (size_t scode, const c
*** 623,629 ****
  
    switch (code)
      {
!     case OPT_fresolution:
        resolution_file_name = arg;
        result = 1;
        break;
--- 623,629 ----
  
    switch (code)
      {
!     case OPT_fresolution_:
        resolution_file_name = arg;
        result = 1;
        break;
Index: lto-plugin/lto-plugin.c
===================================================================
*** lto-plugin/lto-plugin.c	(revision 159097)
--- lto-plugin/lto-plugin.c	(working copy)
*************** free_2 (void)
*** 294,305 ****
    if (arguments_file_name)
      free (arguments_file_name);
    arguments_file_name = NULL;
- 
-   if (resolution_file)
-     {
-       free (resolution_file);
-       resolution_file = NULL;
-     }
  }
  
  /*  Writes the relocations to disk. */
--- 294,299 ----
*************** write_resolution (void)
*** 310,315 ****
--- 304,310 ----
    unsigned int i;
    FILE *f;
  
+   check (resolution_file, LDPL_FATAL, "resolution file not specified");
    f = fopen (resolution_file, "w");
    check (f, LDPL_FATAL, "could not open file");
  
*************** static enum ld_plugin_status
*** 467,473 ****
  all_symbols_read_handler (void)
  {
    unsigned i;
!   unsigned num_lto_args = num_claimed_files + lto_wrapper_num_args + 2 + 1;
    char **lto_argv;
    const char **lto_arg_ptr;
    if (num_claimed_files == 0)
--- 462,468 ----
  all_symbols_read_handler (void)
  {
    unsigned i;
!   unsigned num_lto_args = num_claimed_files + lto_wrapper_num_args + 1;
    char **lto_argv;
    const char **lto_arg_ptr;
    if (num_claimed_files == 0)
*************** all_symbols_read_handler (void)
*** 483,490 ****
    lto_arg_ptr = (const char **) lto_argv;
    assert (lto_wrapper_argv);
  
-   resolution_file = make_temp_file ("");
- 
    write_resolution ();
  
    free_1 ();
--- 478,483 ----
*************** all_symbols_read_handler (void)
*** 492,500 ****
    for (i = 0; i < lto_wrapper_num_args; i++)
      *lto_arg_ptr++ = lto_wrapper_argv[i];
  
-   *lto_arg_ptr++ = "-fresolution";
-   *lto_arg_ptr++ = resolution_file;
- 
    for (i = 0; i < num_claimed_files; i++)
      {
        struct plugin_file_info *info = &claimed_files[i];
--- 485,490 ----
*************** cleanup_handler (void)
*** 543,554 ****
        check (t == 0, LDPL_FATAL, "could not unlink arguments file");
      }
  
-   if (resolution_file)
-     {
-       t = unlink (resolution_file);
-       check (t == 0, LDPL_FATAL, "could not unlink resolution file");
-     }
- 
    for (i = 0; i < num_output_files; i++)
      {
        t = unlink (output_files[i]);
--- 533,538 ----
*************** process_option (const char *option)
*** 657,666 ****
    else
      {
        int size;
        lto_wrapper_num_args += 1;
        size = lto_wrapper_num_args * sizeof (char *);
        lto_wrapper_argv = (char **) xrealloc (lto_wrapper_argv, size);
!       lto_wrapper_argv[lto_wrapper_num_args - 1] = xstrdup(option);
      }
  }
  
--- 641,653 ----
    else
      {
        int size;
+       char *opt = xstrdup (option);
        lto_wrapper_num_args += 1;
        size = lto_wrapper_num_args * sizeof (char *);
        lto_wrapper_argv = (char **) xrealloc (lto_wrapper_argv, size);
!       lto_wrapper_argv[lto_wrapper_num_args - 1] = opt;
!       if (strncmp (option, "-fresolution=", sizeof ("-fresolution=") - 1) == 0)
! 	resolution_file = opt + sizeof ("-fresolution=") - 1;
      }
  }
  

Re: [PATCH] Move resolution file handling to gcc.c, fix PRs 43857, 43371

[Diego Novillo]

On 5/6/10 09:51 , Richard Guenther wrote:

> 2010-05-06  Richard Guenther <rguenther@suse.de>
> 
> 	* gcc.c (LINK_COMMAND_SPEC): Provide a resolution file to
> 	the linker plugin.
> 	(store_arg): Queue temp_filename for deletion instead of
> 	the whole argument.
> 
> 	lto/
> 	PR lto/43857
> 	PR lto/43371
> 	* lang.opt (fresolution): Change to ...
> 	(fresolution=): ... this.
> 	* lto-lang.c (lto_handle_option): Adjust.
> 
> 	lto-plugin/
> 	* lto-plugin.c (free_2): Do not free resolution_file.
> 	(write_resolution): Check that we were passed a resolution file.
> 	(all_symbols_read_handler): Adjust.
> 	(cleanup_handler): Do not remove the resolution file.
> 	(process_option): Handle -fresolution=.

OK.


Diego.

Re: [PATCH] Move resolution file handling to gcc.c, fix PRs 43857, 43371

[Eric Botcazou]

> 2010-05-06  Richard Guenther <rguenther@suse.de>
>
> 	* gcc.c (LINK_COMMAND_SPEC): Provide a resolution file to
> 	the linker plugin.
> 	(store_arg): Queue temp_filename for deletion instead of
> 	the whole argument.

This broke -pipe:

eric@atlantis:~/gnat/gnat-head/native32> cat t.c
int i;
eric@atlantis:~/gnat/gnat-head/native32> gcc/xgcc -Bgcc -c t.c -pipe
Segmentation fault

-- 
Eric Botcazou

Re: [PATCH] Move resolution file handling to gcc.c, fix PRs 43857, 43371

[Richard Guenther]

On Sun, 9 May 2010, Eric Botcazou wrote:

> > 2010-05-06  Richard Guenther <rguenther@suse.de>
> >
> > 	* gcc.c (LINK_COMMAND_SPEC): Provide a resolution file to
> > 	the linker plugin.
> > 	(store_arg): Queue temp_filename for deletion instead of
> > 	the whole argument.
> 
> This broke -pipe:
> 
> eric@atlantis:~/gnat/gnat-head/native32> cat t.c
> int i;
> eric@atlantis:~/gnat/gnat-head/native32> gcc/xgcc -Bgcc -c t.c -pipe
> Segmentation fault

Bah.  I have reverted the store_arg change and will propose
sth else to handle -plugin-arg=-fresolution-file=/tmp/xxxx.

Richard.

Re: [PATCH] Move resolution file handling to gcc.c, fix PRs 43857, 43371

[Richard Guenther]

On Sun, 9 May 2010, Richard Guenther wrote:

> On Sun, 9 May 2010, Eric Botcazou wrote:
> 
> > > 2010-05-06  Richard Guenther <rguenther@suse.de>
> > >
> > > 	* gcc.c (LINK_COMMAND_SPEC): Provide a resolution file to
> > > 	the linker plugin.
> > > 	(store_arg): Queue temp_filename for deletion instead of
> > > 	the whole argument.
> > 
> > This broke -pipe:
> > 
> > eric@atlantis:~/gnat/gnat-head/native32> cat t.c
> > int i;
> > eric@atlantis:~/gnat/gnat-head/native32> gcc/xgcc -Bgcc -c t.c -pipe
> > Segmentation fault
> 
> Bah.  I have reverted the store_arg change and will propose
> sth else to handle -plugin-arg=-fresolution-file=/tmp/xxxx.

Which is the following.

Bootstrap & regtest pending.

Richard.

2010-05-09  Richard Guenther  <rguenther@suse.de>

	* gcc.c (store_arg): Handle temporary file deletion for
	joined arguments.

Index: gcc/gcc.c
===================================================================
*** gcc/gcc.c	(revision 159197)
--- gcc/gcc.c	(working copy)
*************** store_arg (const char *arg, int delete_a
*** 2082,2088 ****
    if (strcmp (arg, "-o") == 0)
      have_o_argbuf_index = argbuf_index;
    if (delete_always || delete_failure)
!     record_temp_file (arg, delete_always, delete_failure);
  }
  \f
  /* Load specs from a file name named FILENAME, replacing occurrences of
--- 2082,2096 ----
    if (strcmp (arg, "-o") == 0)
      have_o_argbuf_index = argbuf_index;
    if (delete_always || delete_failure)
!     {
!       const char *p;
!       /* If the temporary file we should delete is specified as
! 	 part of a joined argument extract the filename.  */
!       if (arg[0] == '-'
! 	  && (p = strrchr (arg, '=')))
! 	arg = p + 1;
!       record_temp_file (arg, delete_always, delete_failure);
!     }
  }
  \f
  /* Load specs from a file name named FILENAME, replacing occurrences of