[Patch, fortran, GCC-5/6, PR62536, v1] ICE (segfault) for invalid END BLOCK statement

[Patch, fortran, GCC-5/6, PR62536, v1] ICE (segfault) for invalid END BLOCK statement

[Andre Vehreschild]

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

Hi all,

attached are two patches one for trunk and one for the gcc-5-branch,
which fix an ICE when BLOCK statements are not closed correctly (too
many END BLOCKs). Unfortunately did the patch I tailored for gcc-5 not
work on trunk.

GCC 5: The ICE is prevented by making sure that gfc_current_ns is not
set to NULL, when too many closing END BLOCK statements are found. This
may lead to gfortran be in the wrong namespace, but gfortran is already
in error. Therefore this does not matter any more. We just need to exit
cleanly.

GCC 6/trunk: Here the ICE is prevented by making sure, that unnesting
of BLOCK namespaces is only done, when the END encountered is not the
one of a BLOCK. This prevents gfortran from removing a correctly
initialized and for further analysis required namespace.

Bootstrapped and regtested ok on x86_64-linux-gnu/F23. 

Ok for trunk and gcc-5-branch, respectively?

Regards,
	Andre
-- 
Andre Vehreschild * Email: vehre ad gmx dot de 

[-- Attachment #2: pr62536_gcc5_1.patch --]
[-- Type: text/x-patch, Size: 1538 bytes --]

diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 2708413..adea3f3 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -6454,9 +6454,16 @@ cleanup:
 	  prev_ns = ns;
 	  ns = ns->sibling;
 	}
-  
-      gfc_free_namespace (gfc_current_ns);
-      gfc_current_ns = parent_ns;
+
+      if (parent_ns)
+	{
+	  /* Free the current namespace only when the parent one exists.  This
+	     prevents an ICE when more END BLOCK then BLOCK statements are
+	     present.  It does not mean any further harm, because we already
+	     have errored.  */
+	  gfc_free_namespace (gfc_current_ns);
+	  gfc_current_ns = parent_ns;
+	}
     }
 
   return MATCH_ERROR;
diff --git a/gcc/testsuite/gfortran.dg/block_14.f08 b/gcc/testsuite/gfortran.dg/block_14.f08
new file mode 100644
index 0000000..4e5903b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/block_14.f08
@@ -0,0 +1,21 @@
+! { dg-do compile }
+!
+! Contributed by Tobias Burnus  <burnus@gcc.gnu.org>
+! Check fix for PR62536 works as expected.
+
+function f2 (x)
+implicit none
+  integer f2, x
+  block
+   block named ! { dg-error "Unclassifiable statement" }
+    integer a ! should be SAVEd
+    a = a + x ! should increment by y every time
+    f2 = a
+   end block named ! { dg-error "Syntax error in END BLOCK statement" }
+  end block ! { dg-error "Expected block name of " }
+  return
+endfunction ! { dg-error "Expecting END BLOCK statement" }
+
+end ! { dg-error "END BLOCK statement expected" }
+! { dg-excess-errors "Previous error cause unexpected end of file." }
+

[-- Attachment #3: pr62536_gcc5_1.txt --]
[-- Type: text/plain, Size: 462 bytes --]

gcc/testsuite/ChangeLog:

2016-01-27  Andre Vehreschild  <vehre@gcc.gnu.org>

	PR fortran/62536
	* gfortran.dg/block_14.f08: New test.


gcc/fortran/ChangeLog:

2016-01-27  Andre Vehreschild  <vehre@gcc.gnu.org>

	PR fortran/62536
	* decl.c: Prevent setting gfc_current_ns to NULL when block statement's
	nesting is incomplete.  There is already an error conditon, so having
	gfc_current_ns pointing to an eventually wrong namespace does not matter
	that much.


[-- Attachment #4: pr62536_gcc6_1.patch --]
[-- Type: text/x-patch, Size: 2363 bytes --]

diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 7c0e011..df81369 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -6327,6 +6327,7 @@ gfc_match_end (gfc_statement *st)
   gfc_namespace *parent_ns, *ns, *prev_ns;
   gfc_namespace **nsp;
   bool abreviated_modproc_decl;
+  bool got_matching_end = false;
 
   old_loc = gfc_current_locus;
   if (gfc_match ("end") != MATCH_YES)
@@ -6510,6 +6511,8 @@ gfc_match_end (gfc_statement *st)
 		 ? "END PROCEDURE" : gfc_ascii_statement(*st), &old_loc);
       goto cleanup;
     }
+  else
+    got_matching_end = true;
 
   old_loc = gfc_current_locus;
   /* If we're at the end, make sure a block name wasn't required.  */
@@ -6581,7 +6584,7 @@ cleanup:
   /* If we are missing an END BLOCK, we created a half-ready namespace.
      Remove it from the parent namespace's sibling list.  */
 
-  while (state == COMP_BLOCK)
+  while (state == COMP_BLOCK && !got_matching_end)
     {
       parent_ns = gfc_current_ns->parent;
 
@@ -6601,7 +6604,7 @@ cleanup:
 	  prev_ns = ns;
 	  ns = ns->sibling;
 	}
-  
+
       gfc_free_namespace (gfc_current_ns);
       gfc_current_ns = parent_ns;
       gfc_state_stack = gfc_state_stack->previous;
diff --git a/gcc/testsuite/gfortran.dg/block_15.f08 b/gcc/testsuite/gfortran.dg/block_15.f08
new file mode 100644
index 0000000..fd1ca55
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/block_15.f08
@@ -0,0 +1,20 @@
+! { dg-do compile }
+!
+! Contributed by Tobias Burnus  <burnus@gcc.gnu.org>
+! Check fix for PR62536 works as expected.
+
+function f2 (x)
+implicit none
+  integer f2, x
+  block
+   block named ! { dg-error "Unclassifiable statement" }
+    integer a ! should be SAVEd
+    a = a + x ! should increment by y every time
+    f2 = a
+   end block named ! { dg-error "Syntax error in END BLOCK statement" }
+  end block
+  return
+endfunction
+
+end
+
diff --git a/gcc/testsuite/gfortran.dg/block_end_error_1.f90 b/gcc/testsuite/gfortran.dg/block_end_error_1.f90
index e7dcdfa..4bbe89b 100644
--- a/gcc/testsuite/gfortran.dg/block_end_error_1.f90
+++ b/gcc/testsuite/gfortran.dg/block_end_error_1.f90
@@ -6,5 +6,5 @@ subroutine s
    block
    end block named ! { dg-error "Syntax error in END BLOCK statement" }
    return
-endsubroutine
+endsubroutine ! { dg-error "Expecting END BLOCK statement" }
 ! { dg-prune-output "Unexpected end of file" }

[-- Attachment #5: pr62536_gcc6_1.txt --]
[-- Type: text/plain, Size: 453 bytes --]

gcc/fortran/ChangeLog:

2016-01-27  Andre Vehreschild  <vehre@gcc.gnu.org>

	PR fortran/62536
	* decl.c (gfc_match_end): Only unnest and remove BLOCK namespaces
	when the END encountered does not match a BLOCK's end.

gcc/testsuite/ChangeLog:

2016-01-27  Andre Vehreschild  <vehre@gcc.gnu.org>

	PR fortran/62536
	* gfortran.dg/block_15.f08: New test.
	* gfortran.dg/block_end_error_1.f90: Need to catch additional error
	on incorrectly closed BLOCK.

Re: [Patch, fortran, GCC-5/6, PR62536, v1] ICE (segfault) for invalid END BLOCK statement

[Jerry DeLisle]

On 01/27/2016 08:25 AM, Andre Vehreschild wrote:
> Hi all,
> 
> attached are two patches one for trunk and one for the gcc-5-branch,
> which fix an ICE when BLOCK statements are not closed correctly (too
> many END BLOCKs). Unfortunately did the patch I tailored for gcc-5 not
> work on trunk.
> 
> GCC 5: The ICE is prevented by making sure that gfc_current_ns is not
> set to NULL, when too many closing END BLOCK statements are found. This
> may lead to gfortran be in the wrong namespace, but gfortran is already
> in error. Therefore this does not matter any more. We just need to exit
> cleanly.
> 
> GCC 6/trunk: Here the ICE is prevented by making sure, that unnesting
> of BLOCK namespaces is only done, when the END encountered is not the
> one of a BLOCK. This prevents gfortran from removing a correctly
> initialized and for further analysis required namespace.
> 
> Bootstrapped and regtested ok on x86_64-linux-gnu/F23. 
> 
> Ok for trunk and gcc-5-branch, respectively?
> 

OK and thanks for fixes. Much appreciated?

Jerry

Re: [Patch, fortran, GCC-5/6, PR62536, v1] ICE (segfault) for invalid END BLOCK statement

[Andre Vehreschild]

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

Hi Jerry, hi all,

thanks for the fast review. Committed as

r232918 for gcc-5-branch, and

r232919 for trunk.

Thanks again Jerry and regards,

	Andre

On Wed, 27 Jan 2016 15:19:32 -0800
Jerry DeLisle <jvdelisle@charter.net> wrote:

> On 01/27/2016 08:25 AM, Andre Vehreschild wrote:
> > Hi all,
> > 
> > attached are two patches one for trunk and one for the gcc-5-branch,
> > which fix an ICE when BLOCK statements are not closed correctly (too
> > many END BLOCKs). Unfortunately did the patch I tailored for gcc-5 not
> > work on trunk.
> > 
> > GCC 5: The ICE is prevented by making sure that gfc_current_ns is not
> > set to NULL, when too many closing END BLOCK statements are found. This
> > may lead to gfortran be in the wrong namespace, but gfortran is already
> > in error. Therefore this does not matter any more. We just need to exit
> > cleanly.
> > 
> > GCC 6/trunk: Here the ICE is prevented by making sure, that unnesting
> > of BLOCK namespaces is only done, when the END encountered is not the
> > one of a BLOCK. This prevents gfortran from removing a correctly
> > initialized and for further analysis required namespace.
> > 
> > Bootstrapped and regtested ok on x86_64-linux-gnu/F23. 
> > 
> > Ok for trunk and gcc-5-branch, respectively?
> >   
> 
> OK and thanks for fixes. Much appreciated?
> 
> Jerry


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 

[-- Attachment #2: submit5.diff --]
[-- Type: text/x-patch, Size: 1675 bytes --]

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(Revision 232916)
+++ gcc/fortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,11 @@
+2016-01-28  Andre Vehreschild  <vehre@gcc.gnu.org>
+
+	PR fortran/62536
+	* decl.c: Prevent setting gfc_current_ns to NULL when block statement's
+	nesting is incomplete.  There is already an error conditon, so having
+	gfc_current_ns pointing to an eventually wrong namespace does not matter
+	that much.
+
 2016-01-27  Andre Vehreschild  <vehre@gcc.gnu.org>
 
 	PR fortran/p69268
Index: gcc/fortran/decl.c
===================================================================
--- gcc/fortran/decl.c	(Revision 232916)
+++ gcc/fortran/decl.c	(Arbeitskopie)
@@ -6454,9 +6454,16 @@
 	  prev_ns = ns;
 	  ns = ns->sibling;
 	}
-  
-      gfc_free_namespace (gfc_current_ns);
-      gfc_current_ns = parent_ns;
+
+      if (parent_ns)
+	{
+	  /* Free the current namespace only when the parent one exists.  This
+	     prevents an ICE when more END BLOCK then BLOCK statements are
+	     present.  It does not mean any further harm, because we already
+	     have errored.  */
+	  gfc_free_namespace (gfc_current_ns);
+	  gfc_current_ns = parent_ns;
+	}
     }
 
   return MATCH_ERROR;
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(Revision 232916)
+++ gcc/testsuite/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,8 @@
+2016-01-28  Andre Vehreschild  <vehre@gcc.gnu.org>
+
+	PR fortran/62536
+	* gfortran.dg/block_14.f08: New test.
+
 2016-01-27  Marek Polacek  <polacek@redhat.com>
 
 	Backport from mainline

[-- Attachment #3: submit6.diff --]
[-- Type: text/x-patch, Size: 2626 bytes --]

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(Revision 232917)
+++ gcc/fortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,9 @@
+2016-01-28  Andre Vehreschild  <vehre@gcc.gnu.org>
+
+	PR fortran/62536
+	* decl.c (gfc_match_end): Only unnest and remove BLOCK namespaces
+	when the END encountered does not match a BLOCK's end.
+
 2016-01-27  Janus Weil  <janus@gcc.gnu.org>
 
 	PR fortran/69484
Index: gcc/fortran/decl.c
===================================================================
--- gcc/fortran/decl.c	(Revision 232917)
+++ gcc/fortran/decl.c	(Arbeitskopie)
@@ -6327,6 +6327,7 @@
   gfc_namespace *parent_ns, *ns, *prev_ns;
   gfc_namespace **nsp;
   bool abreviated_modproc_decl;
+  bool got_matching_end = false;
 
   old_loc = gfc_current_locus;
   if (gfc_match ("end") != MATCH_YES)
@@ -6510,6 +6511,8 @@
 		 ? "END PROCEDURE" : gfc_ascii_statement(*st), &old_loc);
       goto cleanup;
     }
+  else
+    got_matching_end = true;
 
   old_loc = gfc_current_locus;
   /* If we're at the end, make sure a block name wasn't required.  */
@@ -6581,7 +6584,7 @@
   /* If we are missing an END BLOCK, we created a half-ready namespace.
      Remove it from the parent namespace's sibling list.  */
 
-  while (state == COMP_BLOCK)
+  while (state == COMP_BLOCK && !got_matching_end)
     {
       parent_ns = gfc_current_ns->parent;
 
@@ -6601,7 +6604,7 @@
 	  prev_ns = ns;
 	  ns = ns->sibling;
 	}
-  
+
       gfc_free_namespace (gfc_current_ns);
       gfc_current_ns = parent_ns;
       gfc_state_stack = gfc_state_stack->previous;
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(Revision 232917)
+++ gcc/testsuite/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,10 @@
+2016-01-28  Andre Vehreschild  <vehre@gcc.gnu.org>
+
+	PR fortran/62536
+	* gfortran.dg/block_15.f08: New test.
+	* gfortran.dg/block_end_error_1.f90: Need to catch additional error
+	on incorrectly closed BLOCK.
+
 2016-01-28  Ilya Enkovich  <enkovich.gnu@gmail.com>
 
 	* gcc.dg/declare-simd.c: New test.
Index: gcc/testsuite/gfortran.dg/block_end_error_1.f90
===================================================================
--- gcc/testsuite/gfortran.dg/block_end_error_1.f90	(Revision 232917)
+++ gcc/testsuite/gfortran.dg/block_end_error_1.f90	(Arbeitskopie)
@@ -6,5 +6,5 @@
    block
    end block named ! { dg-error "Syntax error in END BLOCK statement" }
    return
-endsubroutine
+endsubroutine ! { dg-error "Expecting END BLOCK statement" }
 ! { dg-prune-output "Unexpected end of file" }