[PR fortran/66528] unbalanced IF/ENDIF with -fmax-errors=1 causes invalid free

[PR fortran/66528] unbalanced IF/ENDIF with -fmax-errors=1 causes invalid free

[Manuel López-Ibáñez]

[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]

The problem is that diagnostic_action_after_output tries to delete the
active pretty-printer which tries to delete its output_buffer, which
is normally dynamically allocated via placement-new, but the
output_buffer used by the error_buffer of Fortran is statically
allocated. Being statically allocated simplifies a lot pushing/poping
several instances of error_buffer.

The solution I found is to reset the active output_buffer back to the
default one before calling diagnostic_action_after_output. This is a
bit ugly, because this function does use the output_buffer, however,
at the point that Fortran calls it, both are in an equivalent state,
thus there is no visible difference.


Bootstrapped and regression tested on x86_64-linux-gnu.

2015-06-24  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    PR fortran/66528
    * gfortran.dg/maxerrors.f90: New test.

gcc/fortran/ChangeLog:

2015-06-24  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    PR fortran/66528
    * error.c (gfc_warning_check): Restore the default output_buffer
    before calling diagnostic_action_after_output.
    (gfc_error_check): Likewise.
    (gfc_diagnostics_init): Add comment.

[-- Attachment #2: fix-pr66528.diff --]
[-- Type: text/plain, Size: 2430 bytes --]

Index: gcc/testsuite/gfortran.dg/maxerrors.f90
===================================================================
--- gcc/testsuite/gfortran.dg/maxerrors.f90	(revision 0)
+++ gcc/testsuite/gfortran.dg/maxerrors.f90	(revision 0)
@@ -0,0 +1,12 @@
+! { dg-do compile }
+! { dg-options "-fmax-errors=1" }
+! PR66528
+! { dg-prune-output "compilation terminated" }
+program main
+  read (*,*) n
+  if (n<0) then
+    print *,foo
+  end ! { dg-error "END IF statement expected" }
+    print *,bar
+end program main
+
Index: gcc/fortran/error.c
===================================================================
--- gcc/fortran/error.c	(revision 224844)
+++ gcc/fortran/error.c	(working copy)
@@ -1247,24 +1247,23 @@ gfc_clear_warning (void)
    If so, print the warning.  */
 
 void
 gfc_warning_check (void)
 {
-  /* This is for the new diagnostics machinery.  */
   if (! gfc_output_buffer_empty_p (pp_warning_buffer))
     {
       pretty_printer *pp = global_dc->printer;
       output_buffer *tmp_buffer = pp->buffer;
       pp->buffer = pp_warning_buffer;
       pp_really_flush (pp);
       warningcount += warningcount_buffered;
       werrorcount += werrorcount_buffered;
       gcc_assert (warningcount_buffered + werrorcount_buffered == 1);
+      pp->buffer = tmp_buffer;
       diagnostic_action_after_output (global_dc, 
 				      warningcount_buffered 
 				      ? DK_WARNING : DK_ERROR);
-      pp->buffer = tmp_buffer;
     }
 }
 
 
 /* Issue an error.  */
@@ -1379,12 +1378,12 @@ gfc_error_check (void)
       output_buffer *tmp_buffer = pp->buffer;
       pp->buffer = pp_error_buffer;
       pp_really_flush (pp);
       ++errorcount;
       gcc_assert (gfc_output_buffer_empty_p (pp_error_buffer));
-      diagnostic_action_after_output (global_dc, DK_ERROR);
       pp->buffer = tmp_buffer;
+      diagnostic_action_after_output (global_dc, DK_ERROR);
       return true;
     }
 
   return false;
 }
@@ -1470,10 +1469,12 @@ gfc_diagnostics_init (void)
   diagnostic_format_decoder (global_dc) = gfc_format_decoder;
   global_dc->caret_chars[0] = '1';
   global_dc->caret_chars[1] = '2';
   pp_warning_buffer = new (XNEW (output_buffer)) output_buffer ();
   pp_warning_buffer->flush_p = false;
+  /* pp_error_buffer is statically allocated.  This simplifies memory
+     management when using gfc_push/pop_error. */
   pp_error_buffer = &(error_buffer.buffer);
   pp_error_buffer->flush_p = false;
 }
 
 void

Re: [PR fortran/66528] unbalanced IF/ENDIF with -fmax-errors=1 causes invalid free

[Steve Kargl]

On Wed, Jun 24, 2015 at 08:36:45PM +0200, Manuel López-Ibáñez wrote:
> The problem is that diagnostic_action_after_output tries to delete the
> active pretty-printer which tries to delete its output_buffer, which
> is normally dynamically allocated via placement-new, but the
> output_buffer used by the error_buffer of Fortran is statically
> allocated. Being statically allocated simplifies a lot pushing/poping
> several instances of error_buffer.
> 
> The solution I found is to reset the active output_buffer back to the
> default one before calling diagnostic_action_after_output. This is a
> bit ugly, because this function does use the output_buffer, however,
> at the point that Fortran calls it, both are in an equivalent state,
> thus there is no visible difference.
> 
> 
> Bootstrapped and regression tested on x86_64-linux-gnu.
> 
> 2015-06-24  Manuel López-Ibáñez  <manu@gcc.gnu.org>
> 
>     PR fortran/66528
>     * gfortran.dg/maxerrors.f90: New test.
> 
> gcc/fortran/ChangeLog:
> 
> 2015-06-24  Manuel López-Ibáñez  <manu@gcc.gnu.org>
> 
>     PR fortran/66528
>     * error.c (gfc_warning_check): Restore the default output_buffer
>     before calling diagnostic_action_after_output.
>     (gfc_error_check): Likewise.
>     (gfc_diagnostics_init): Add comment.

Patch looks ok to me.

-- 
Steve