[PATCH] Enable parallel ltrans stage with -fwhopr=N

[Richard Guenther]

On Wed, 26 May 2010, Ralf Wildenhues wrote:

> * Richard Guenther wrote on Wed, May 26, 2010 at 10:50:24AM CEST:
> > On Tue, 25 May 2010, Ralf Wildenhues wrote:
> > > * Richard Guenther wrote on Tue, May 25, 2010 at 03:03:23PM CEST:
> > > > On Sun, 23 May 2010, Richard Guenther wrote:
> > > > > On Sun, May 23, 2010 at 9:16 AM, Ralf Wildenhues wrote:
> > > > > > I further noted that when interrupted with C^c, -fwhopr= may leave a
> > > > > > number of files matching *.wpa.o and *.wpa.ltrans.o around in the build
> > > > > > tree (not sure if only the current directory or also subdirs). Â Besides
> > > > > > the missing cleanup issue (interrupted make only removes incompletely
> > > > > > updated targets), are these files suitably named so that they won't
> > > > > > interfere with, say, toplevel parallel make generating prog1 and prog2
> > > > > > from the same (or an overlapping) set of *.o files but with possibly
> > > > > > different link flags? Â These names don't look random to me.
> > > > > >
> > > > > > I get some leftover files even if 'make' for some reason doesn't run at
> > > > > > all or doesn't comprehend -j.
> > > > > 
> > > > > Yeah, that's easily fixed - I can look at it.
> > > > 
> > > > I have posted a patch to fix that.
> > > 
> > > What exactly is "that"?  The patch you posted does not fix all the
> > > issues I described in the above two paragraphs.  There are still
> > > leftover *.wpa.o files after interrupt of failed 'make', and the file
> > > names still are not parallel-safe.  (That doesn't mean the patch is
> > > bad, nor that I expect you to fix the other issues.) Should I open a PR?
> > 
> > Hm, that would be a bug (the leftover *.wpa.o files).  It should
> > detect a failed make in collect_execute and end up calling 
> > lto_wrapper_exit which in turn should unlink those files.
> 
> Except that it doesn't work right yet with r159853.  I can still
> easily get leftover *.wpa.o as well as *.wpa.ltrans.o files with C^c.
> Would you like me to create a reproducer?

Yes, that would be nice.

> > The file names should be parallel-safe - or what are you refering
> > to here?
> 
> With this makefile for GNU make:
> 
> --- snip ---
> CC = gcc -O -fwhopr=3
> obj = a.o b.o c.o d.o e.o f.o
> 
> all: p1 p2
> p1 p2: $(obj)
>         $(CC) -o $@ $(obj)
> --- snip ---
> 
> and some interrelated sources in a.c, b.c, c.c, then 'make -j' may cause
> the link for p1 to create an object named a_b_c.wpa.o for some time, and
> remove it later again.  At around the same time, the link for p2 may
> create a_b_c.wpa.o, try to access it later, and remove it.  There is a
> race condition between the two link processes working on the same file.
> 
> I can easily provoke failures such as:
> 
>   lto-wrapper: deleting LTRANS file a_b_c.wpa.o: No such file or directory
>   lto-wrapper: deleting LTRANS file /tmp/cccUSDhq.args: No such file or directory
>   collect2: lto-wrapper returned 1 exit status
> 
> with a real-world setup similar to above.  The second error message looks
> like it could be exploitable, by the way.

Oh, indeed.  I see what you are refering to now.   Note that the
names should be all tmpnames if -save-temps is not specified.
I'm not sure if we care enough for the -save-temps case to be
parallel-safe - we definitely want to have user-recognizable file-names
here.

The above messages could also stem from us trying to delete files that
have been deleted by someone else or have not yet been created
(only at failure time).  Those should be fixed by

Index: gcc/lto-wrapper.c
===================================================================
--- gcc/lto-wrapper.c   (revision 159879)
+++ gcc/lto-wrapper.c   (working copy)
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.
 
 #include "config.h"
 #include "system.h"
+#include <errno.h>
 #include "coretypes.h"
 #include "tm.h"
 #include "intl.h"
@@ -222,7 +223,8 @@ maybe_unlink_file (const char *file)
 {
   if (! debug)
     {
-      if (unlink_if_ordinary (file))
+      if (unlink_if_ordinary (file)
+         && errno != ENOENT)
        fatal_perror ("deleting LTRANS file %s", file);
     }
   else
@@ -262,6 +262,7 @@ fork_execute (char **argv)
   collect_wait (new_argv[0], pex);
 
   maybe_unlink_file (args_name);
+  args_name = NULL;
   free (at_args);
 }


I will test and commit that as obvious.

> It is harder to provoke failures such as:
> 
>   ld: error: cannot open a_b_c.wpa.ltrans.o: No such file or directory
>   ld: d_e_f___.wpa.ltrans.o: in function func:d_e_f___.wpa.o(.text+0x619c): error: undefined reference to 'otherfunc'
>   [...]
>   ld: g_h_i___.wpa.ltrans.o: in function func:g_h_i___.wpa.o(.text+0x619c): error: undefined reference to 'anotherfunc:clone.11'
>   [...]
>   collect2: ld returned 1 exit status
> 
> but given the right timing (a dozen tries), that'll happen, too.
> 
> Generally, whenever the compiler or linker creates temporary files in
> the current directory, it should document that it does so, and if the
> names are guessable, then both input and output file must be keyed into
> the file name in order for above things to work with a general parallel
> build system.

Were you able to have temporary files in the current directory
without specifying -save-temps?  If so, that would be a bug.

> Whenever it creates files in /tmp, they may be removed exactly once
> only.

Yes, though with a failure from make we're not sure if they are
already created (I simply try to remove all inputs and outputs),
so removing a file that isn't there shouldn't be an error.

> Unrelated to the above reasoning, the naming strategy should also be
> documented, both to avoid user surprise and to let build tools know they
> are not stepping on each other's toes.  (BTW, the above errors are not
> specifically related to Automake at all.)

Hm, yeah - maybe.  I don't know if we thoroughly document all
the sort of filenames that are created with -save-temps though.

Richard.