PATCH RFA: Make collect2 send stdout to stdout and stderr to stderr

PATCH RFA: Make collect2 send stdout to stdout and stderr to stderr

[Ian Lance Taylor]

It's been a longstanding pet peeve of mine that collect2 combines
linker output to both stdout and stderr and sends it to stderr.  This
is just wrong.  It leads to irritations like the fact that gcc
--target-help dumps the assembler help information to stdout and the
linker help information to stderr.  It makes it harder to use
-Wl,--verbose.

This patch fixes this.  It will make collect2 a tiny bit slower, as it
will now create and remove two temporary files instead of one.  I
don't think this will be a serious slowdown compared to overall link
time.  In the normal case nothing will be written to either file.

It might be slightly more efficient to use pipes rather than temporary
files.  The reason this is not done is that we want to send the linker
output to the caller's stdout and stderr only if the link succeeds.
If the link fails, we want to scan the linker output looking for
references which can be satisfied by repo files, rather than pass it
on.  But when using a pipe we can't know, in general, whether the
linker succeeds unless we read all the output from the pipe, since
otherwise the linker might block writing to the pipe.  So we would
have to be prepared to buffer data in memory until the linker is
complete.  In the normal case the linker will produce no output and
this will be more efficient.  In pathological cases we will use up a
lot of memory buffer linking output.  And the code will be generally
more complex.

Anyhow, this patch passes the obvious tests.  I'm running a bootstrap
and testsuite run now.  OK to commit if those pass?

Ian


2005-03-30  Ian Lance Taylor  <ian@c2micro.com>

	* collect2.c (lderrout): New variable.
	(collect_exit): Dump ldout to stdout.  Dump and unlink lderrout,
	if it is set, to stderr.
	(handler): Unlink lderrout if it is set.
	(dump_file): Add "to" parameter.  Change all callers.
	(main): Initialize lderrout.
	(collect_execute): Add errname parameter.  Change all callers.
	Rename redir parameter to outname.  Never pass
	PEX_STDERR_TO_STDOUT to pex_run.
	* collect2.h (collect_execute, dump_file): Update declarations.
	* tlink.c (tlink_execute): Add errname parameter.  Change all
	callers.
	(do_tlink): Check lderrout as well as ldout.


Index: collect2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/collect2.c,v
retrieving revision 1.171
diff -p -u -r1.171 collect2.c
--- collect2.c	29 Mar 2005 19:39:35 -0000	1.171
+++ collect2.c	30 Mar 2005 05:47:45 -0000
@@ -184,7 +184,8 @@ static const char *o_file;		/* <xxx>.o f
 #ifdef COLLECT_EXPORT_LIST
 static const char *export_file;	        /* <xxx>.x for AIX export list.  */
 #endif
-const char *ldout;			/* File for ld errors.  */
+const char *ldout;			/* File for ld stdout.  */
+const char *lderrout;			/* File for ld stderr.  */
 static const char *output_file;		/* Output file for ld.  */
 static const char *nm_file_name;	/* pathname of nm */
 #ifdef LDD_SUFFIX
@@ -308,10 +309,16 @@ collect_exit (int status)
 
   if (ldout != 0 && ldout[0])
     {
-      dump_file (ldout);
+      dump_file (ldout, stdout);
       maybe_unlink (ldout);
     }
 
+  if (lderrout != 0 && lderrout[0])
+    {
+      dump_file (lderrout, stderr);
+      maybe_unlink (lderrout);
+    }
+
   if (status != 0 && output_file != 0 && output_file[0])
     maybe_unlink (output_file);
 
@@ -398,6 +405,9 @@ handler (int signo)
   if (ldout != 0 && ldout[0])
     maybe_unlink (ldout);
 
+  if (lderrout != 0 && lderrout[0])
+    maybe_unlink (lderrout);
+
 #ifdef COLLECT_EXPORT_LIST
   if (export_file != 0 && export_file[0])
     maybe_unlink (export_file);
@@ -447,7 +457,7 @@ extract_string (const char **pp)
 }
 \f
 void
-dump_file (const char *name)
+dump_file (const char *name, FILE *to)
 {
   FILE *stream = fopen (name, "r");
 
@@ -467,7 +477,7 @@ dump_file (const char *name)
 	  word = obstack_finish (&temporary_obstack);
 
 	  if (*word == '.')
-	    ++word, putc ('.', stderr);
+	    ++word, putc ('.', to);
 	  p = word;
 	  if (!strncmp (p, USER_LABEL_PREFIX, strlen (USER_LABEL_PREFIX)))
 	    p += strlen (USER_LABEL_PREFIX);
@@ -484,25 +494,25 @@ dump_file (const char *name)
 	  if (result)
 	    {
 	      int diff;
-	      fputs (result, stderr);
+	      fputs (result, to);
 
 	      diff = strlen (word) - strlen (result);
 	      while (diff > 0 && c == ' ')
-		--diff, putc (' ', stderr);
+		--diff, putc (' ', to);
 	      while (diff < 0 && c == ' ')
 		++diff, c = getc (stream);
 
 	      free (result);
 	    }
 	  else
-	    fputs (word, stderr);
+	    fputs (word, to);
 
-	  fflush (stderr);
+	  fflush (to);
 	  obstack_free (&temporary_obstack, temporary_firstobj);
 	}
       if (c == EOF)
 	break;
-      putc (c, stderr);
+      putc (c, to);
     }
   fclose (stream);
 }
@@ -990,6 +1000,7 @@ main (int argc, char **argv)
   export_file = make_temp_file (".x");
 #endif
   ldout = make_temp_file (".ld");
+  lderrout = make_temp_file (".le");
   *c_ptr++ = c_file_name;
   *c_ptr++ = "-x";
   *c_ptr++ = "c";
@@ -1525,7 +1536,8 @@ do_wait (const char *prog, struct pex_ob
 /* Execute a program, and wait for the reply.  */
 
 struct pex_obj *
-collect_execute (const char *prog, char **argv, const char *redir)
+collect_execute (const char *prog, char **argv, const char *outname,
+		 const char *errname)
 {
   struct pex_obj *pex;
   const char *errmsg;
@@ -1560,10 +1572,8 @@ collect_execute (const char *prog, char 
   if (pex == NULL)
     fatal_perror ("pex_init failed");
 
-  errmsg = pex_run (pex,
-		    (PEX_LAST | PEX_SEARCH
-		     | (redir ? PEX_STDERR_TO_STDOUT : 0)),
-		    argv[0], argv, redir, NULL, &err);
+  errmsg = pex_run (pex, PEX_LAST | PEX_SEARCH, argv[0], argv, outname,
+		    errname, &err);
   if (errmsg != NULL)
     {
       if (err != 0)
@@ -1583,7 +1593,7 @@ fork_execute (const char *prog, char **a
 {
   struct pex_obj *pex;
 
-  pex = collect_execute (prog, argv, NULL);
+  pex = collect_execute (prog, argv, NULL, NULL);
   do_wait (prog, pex);
 }
 \f
Index: collect2.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/collect2.h,v
retrieving revision 1.11
diff -p -u -r1.11 collect2.h
--- collect2.h	29 Mar 2005 19:39:35 -0000	1.11
+++ collect2.h	30 Mar 2005 05:47:45 -0000
@@ -23,17 +23,19 @@ Software Foundation, 59 Temple Place - S
 
 extern void do_tlink (char **, char **);
 
-extern struct pex_obj *collect_execute (const char *, char **, const char *);
+extern struct pex_obj *collect_execute (const char *, char **, const char *,
+					const char *);
 
 extern void collect_exit (int) ATTRIBUTE_NORETURN;
 
 extern int collect_wait (const char *, struct pex_obj *);
 
-extern void dump_file (const char *);
+extern void dump_file (const char *, FILE *);
 
 extern int file_exists (const char *);
 
 extern const char *ldout;
+extern const char *lderrout;
 extern const char *c_file_name;
 extern struct obstack temporary_obstack;
 extern char *temporary_firstobj;
Index: tlink.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tlink.c,v
retrieving revision 1.60
diff -p -u -r1.60 tlink.c
--- tlink.c	29 Mar 2005 19:39:35 -0000	1.60
+++ tlink.c	30 Mar 2005 05:47:51 -0000
@@ -98,7 +98,7 @@ static symbol * symbol_pop (void);
 static void file_push (file *);
 static file * file_pop (void);
 static void tlink_init (void);
-static int tlink_execute (const char *, char **, const char *);
+static int tlink_execute (const char *, char **, const char *, const char *);
 static char * frob_extension (const char *, const char *);
 static char * obstack_fgets (FILE *, struct obstack *);
 static char * tfgets (FILE *);
@@ -279,11 +279,12 @@ tlink_init (void)
 }
 
 static int
-tlink_execute (const char *prog, char **argv, const char *redir)
+tlink_execute (const char *prog, char **argv, const char *outname,
+	       const char *errname)
 {
   struct pex_obj *pex;
 
-  pex = collect_execute (prog, argv, redir);
+  pex = collect_execute (prog, argv, outname, errname);
   return collect_wait (prog, pex);
 }
 
@@ -527,7 +528,7 @@ recompile_files (void)
 	fprintf (stderr, _("collect: recompiling %s\n"), f->main);
 
       if (chdir (f->dir) != 0
-	  || tlink_execute (c_file_name, argv, NULL) != 0
+	  || tlink_execute (c_file_name, argv, NULL, NULL) != 0
 	  || chdir (initial_cwd) != 0)
 	return 0;
 
@@ -729,7 +730,7 @@ scan_linker_output (const char *fname)
 void
 do_tlink (char **ld_argv, char **object_lst ATTRIBUTE_UNUSED)
 {
-  int exit = tlink_execute ("ld", ld_argv, ldout);
+  int exit = tlink_execute ("ld", ld_argv, ldout, lderrout);
 
   tlink_init ();
 
@@ -743,20 +744,26 @@ do_tlink (char **ld_argv, char **object_
 	while (exit && i++ < MAX_ITERATIONS)
 	  {
 	    if (tlink_verbose >= 3)
-	      dump_file (ldout);
+	      {
+		dump_file (ldout, stdout);
+		dump_file (lderrout, stderr);
+	      }
 	    demangle_new_symbols ();
-	    if (! scan_linker_output (ldout))
+	    if (! scan_linker_output (ldout)
+		&& ! scan_linker_output (lderrout))
 	      break;
 	    if (! recompile_files ())
 	      break;
 	    if (tlink_verbose)
 	      fprintf (stderr, _("collect: relinking\n"));
-	    exit = tlink_execute ("ld", ld_argv, ldout);
+	    exit = tlink_execute ("ld", ld_argv, ldout, lderrout);
 	  }
     }
 
-  dump_file (ldout);
+  dump_file (ldout, stdout);
   unlink (ldout);
+  dump_file (lderrout, stderr);
+  unlink (lderrout);
   if (exit)
     {
       error ("ld returned %d exit status", exit);

Re: PATCH RFA: Make collect2 send stdout to stdout and stderr to stderr

[Mike Stump]

On Mar 29, 2005, at 10:06 PM, Ian Lance Taylor wrote:
> 2005-03-30  Ian Lance Taylor  <ian@c2micro.com>
>
>     * collect2.c (lderrout): New variable.
>     (collect_exit): Dump ldout to stdout.  Dump and unlink lderrout,
>     if it is set, to stderr.

Looks good to me.  My only concern was that you scan both streams,  
and it appears you did.

Re: PATCH RFA: Make collect2 send stdout to stdout and stderr to stderr

[Mike Stump]

On Mar 29, 2005, at 10:06 PM, Ian Lance Taylor wrote:
> It's been a longstanding pet peeve of mine that collect2 combines
> linker output to both stdout and stderr and sends it to stderr.

I'll add this for others to ponder, if they wish, I'm sure Ian knew  
about it:

Oh, the only potential downside, would be if a broken linker mixed  
messages between both stdout and stderr, and required for readability  
that they be fully intermixed.  Before, it would be, after the patch,  
the come out in two groups, stdout first, then stderr.  Personally,  
I'd consider that a bug in ld, and just wouldn't worry about it.

Re: PATCH RFA: Make collect2 send stdout to stdout and stderr to stderr

[Mark Mitchell]

Ian Lance Taylor wrote:
> It's been a longstanding pet peeve of mine that collect2 combines
> linker output to both stdout and stderr and sends it to stderr.  This
> is just wrong.  It leads to irritations like the fact that gcc
> --target-help dumps the assembler help information to stdout and the
> linker help information to stderr.  It makes it harder to use
> -Wl,--verbose.
> 

> Anyhow, this patch passes the obvious tests.  I'm running a bootstrap
> and testsuite run now.  OK to commit if those pass?

Yes.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304