[Patch, RFC] avoid potential recursion, collect error and warning summaries.

[Patch, RFC] avoid potential recursion, collect error and warning summaries.

[Iain Sandoe]

Hello All,

1. I have a note in a patchlet from Tristan that

as.c (close_output_file)
could cause recursion if output_file_close () calls as_fatal or xexit  
() on error conditions.

2. the [presumably intended] error reporting in write.c is unreachable  
in the general case, since write_object_file () is not called when  
there are errors.

3. There's another bit of error reporting later in as.c.

====

So this patch coalesces the error reporting, moves it to the at exit  
action, and avoids the potential recursion on close errors.

Unfortunately, it can't be lightly applied - since the reporting of  
error summary causes a lot of test-cases to need amendment.  Thus I've  
guarded that section of the code, and ports can enable it when they  
have time to go through their test-cases and amend as necessary.

thoughts?

Iain

gas:

	* write.c (write_object_file): Remove warning/error summaries.
	* as.c (TC_WANTS_ERROR_SUMMARY): New.
	(close_output_file): Allow for potential errors in the close routine.
	Report warning and error summaries here.
	(main): Don't report warnings as errors here.
	* config/obj-macho.h (TC_WANTS_ERROR_SUMMARY): Define.

Index: gas/as.c
===================================================================
RCS file: /cvs/src/src/gas/as.c,v
retrieving revision 1.101
diff -u -p -r1.101 as.c
--- gas/as.c	17 Jan 2012 00:07:03 -0000	1.101
+++ gas/as.c	20 Feb 2012 21:29:51 -0000
@@ -992,12 +992,51 @@ dump_statistics (void)
  #endif
  }

+#ifndef TC_WANTS_ERROR_SUMMARY
+#define TC_WANTS_ERROR_SUMMARY 0
+#endif
+
  static void
  close_output_file (void)
  {
-  output_file_close (out_file_name);
-  if (!keep_it)
-    unlink_if_ordinary (out_file_name);
+static int closing = 0;
+
+  /* Avoid recursion if xexit is called from output_file_close().  */
+  if (! closing)
+    {
+      int n_warns, n_errs;
+
+      closing = 1;
+      output_file_close (out_file_name);
+
+      n_warns = had_warnings ();
+      n_errs = had_errors ();
+
+      /* Do error messages by hand, to avoid recursion via  
as_fatal().  */
+      if (flag_always_generate_output
+	  && (n_errs || (n_warns && flag_fatal_warnings)))
+	/* The -Z flag indicates that an object file should be generated,
+	     regardless of warnings and errors.  */
+	fprintf (stderr, _("Summary: %d error%s, %d warning%s, "
+			   "(potentially bad) object file retained\n"),
+		 n_errs, n_errs == 1 ? "" : "s",
+		 n_warns, n_warns == 1 ? "" : "s");
+      else if (n_errs)
+        {
+          if (TC_WANTS_ERROR_SUMMARY)
+	    fprintf (stderr, _("Summary: %d error%s, %d warning%s, "
+			       "no object file generated\n"),
+		     n_errs, n_errs == 1 ? "" : "s",
+		     n_warns, n_warns == 1 ? "" : "s");
+	  unlink_if_ordinary (out_file_name);
+	}
+      else if (flag_fatal_warnings && n_warns)
+        {
+	  fprintf (stderr, _("Summary: %d warnings, and treating warnings as"
+			     " errors, no object file generated\n"), n_warns);
+	  unlink_if_ordinary (out_file_name);
+        }
+  }
  }

  /* The interface between the macro code and gas expression  
handling.  */
@@ -1294,9 +1333,6 @@ main (int argc, char ** argv)
    listing_print (listing_filename, argv_orig);
  #endif

-  if (flag_fatal_warnings && had_warnings () > 0 && had_errors () == 0)
-    as_bad (_("%d warnings, treating warnings as errors"),  
had_warnings ());
-
    if (had_errors () > 0 && ! flag_always_generate_output)
      keep_it = 0;

Index: gas/write.c
===================================================================
RCS file: /cvs/src/src/gas/write.c,v
retrieving revision 1.147
diff -u -p -r1.147 write.c
--- gas/write.c	17 Jan 2012 00:20:30 -0000	1.147
+++ gas/write.c	20 Feb 2012 21:29:52 -0000
@@ -1767,29 +1767,6 @@ write_object_file (void)
    fragS *fragP;			/* Track along all frags.  */
  #endif

-  /* Do we really want to write it?  */
-  {
-    int n_warns, n_errs;
-    n_warns = had_warnings ();
-    n_errs = had_errors ();
-    /* The -Z flag indicates that an object file should be generated,
-       regardless of warnings and errors.  */
-    if (flag_always_generate_output)
-      {
-	if (n_warns || n_errs)
-	  as_warn (_("%d error%s, %d warning%s, generating bad object file"),
-		   n_errs, n_errs == 1 ? "" : "s",
-		   n_warns, n_warns == 1 ? "" : "s");
-      }
-    else
-      {
-	if (n_errs)
-	  as_fatal (_("%d error%s, %d warning%s, no object file generated"),
-		    n_errs, n_errs == 1 ? "" : "s",
-		    n_warns, n_warns == 1 ? "" : "s");
-      }
-  }
-
    /* From now on, we don't care about sub-segments.  Build one frag  
chain
       for each segment. Linked thru fr_next.  */

Index: gas/config/obj-macho.h
===================================================================
RCS file: /cvs/src/src/gas/config/obj-macho.h,v
retrieving revision 1.9
diff -u -p -r1.9 obj-macho.h
--- gas/config/obj-macho.h	20 Feb 2012 20:11:32 -0000	1.9
+++ gas/config/obj-macho.h	20 Feb 2012 21:29:52 -0000
@@ -84,4 +84,6 @@ extern int obj_mach_o_allow_local_subtra
  #define OBJ_PROCESS_STAB(SEG,W,S,T,O,D)	 
obj_mach_o_process_stab(W,S,T,O,D)
  extern void obj_mach_o_process_stab (int, const char *,int, int, int);

+#define TC_WANTS_ERROR_SUMMARY 1
+
  #endif /* _OBJ_MACH_O_H */

Re: [Patch, RFC] avoid potential recursion, collect error and warning summaries.

[Alan Modra]

On Mon, Feb 20, 2012 at 09:41:03PM +0000, Iain Sandoe wrote:
> Unfortunately, it can't be lightly applied - since the reporting of
> error summary causes a lot of test-cases to need amendment.  Thus
> I've guarded that section of the code, and ports can enable it when
> they have time to go through their test-cases and amend as
> necessary.

I don't like this.  There are quite a lot of generic gas tests that
fail if you define TC_WANTS_ERROR_SUMMARY.  So they will at least need
modifying, and likely duplicating, to suit two possible gas error
behaviours.

If you really think this summary output is desirable, then you will
think it worth spending your time fixing the gas testsuite.  On all
targets.  Anything less is just pushing the job onto Nick, me, or
some other global maintainer since most targets don't really have
active maintainers.

I'm more inclined to recognize we currently have some dead code in
write_object_file and just delete it.

-- 
Alan Modra
Australia Development Lab, IBM