Possible patch for PR fortran/66056

Possible patch for PR fortran/66056

[Louis Krupp]

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

The problem:  Statement labels within a type declaration are put in the statement label tree belonging to the type declaration's namespace's (instead of the current namespace).  When the line is otherwise empty and an error is issued, gfc_free_st_label tries to delete the label from the label tree belonging to the current namespace and then frees the label structure, leaving an invalid statement label pointer in the type declaration's namespace's label tree.  When that namespace is cleaned up, bad things can happen.

The attached patch stores a namespace pointer in the statement label structure so that if a label is deleted early for some reason, it will be deleted from the proper namespace.

Louis




[-- Attachment #2: empty_label_typedecl.f90 --]
[-- Type: application/octet-stream, Size: 190 bytes --]

! { dg-do compile }
! { dg-options "-Werror" }
subroutine s
  type t
  1 ! { dg-error "empty statement" }
  end type
end subroutine
! { dg-excess-errors "warnings being treated as errors" }

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

Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 229302)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -1291,6 +1291,8 @@ typedef struct gfc_st_label
   tree backend_decl;
 
   locus where;
+
+  gfc_namespace *ns;
 }
 gfc_st_label;
 
Index: gcc/fortran/io.c
===================================================================
--- gcc/fortran/io.c	(revision 229302)
+++ gcc/fortran/io.c	(working copy)
@@ -28,7 +28,7 @@ along with GCC; see the file COPYING3.  If not see
 
 gfc_st_label
 format_asterisk = {0, NULL, NULL, -1, ST_LABEL_FORMAT, ST_LABEL_FORMAT, NULL,
-		   0, {NULL, NULL}};
+		   0, {NULL, NULL}, NULL};
 
 typedef struct
 {
Index: gcc/fortran/symbol.c
===================================================================
--- gcc/fortran/symbol.c	(revision 229302)
+++ gcc/fortran/symbol.c	(working copy)
@@ -2195,7 +2195,7 @@ gfc_free_st_label (gfc_st_label *label)
   if (label == NULL)
     return;
 
-  gfc_delete_bbt (&gfc_current_ns->st_labels, label, compare_st_labels);
+  gfc_delete_bbt (&label->ns->st_labels, label, compare_st_labels);
 
   if (label->format != NULL)
     gfc_free_expr (label->format);
@@ -2260,6 +2260,7 @@ gfc_get_st_label (int labelno)
   lp->value = labelno;
   lp->defined = ST_LABEL_UNKNOWN;
   lp->referenced = ST_LABEL_UNKNOWN;
+  lp->ns = ns;
 
   gfc_insert_bbt (&ns->st_labels, lp, compare_st_labels);
 

Re: Possible patch for PR fortran/66056

[FX]

> The problem:  Statement labels within a type declaration are put in the statement label tree belonging to the type declaration's namespace's (instead of the current namespace).  When the line is otherwise empty and an error is issued, gfc_free_st_label tries to delete the label from the label tree belonging to the current namespace and then frees the label structure, leaving an invalid statement label pointer in the type declaration's namespace's label tree.  When that namespace is cleaned up, bad things can happen.

Seems OK.
Please post patches with full ChangeLog entry, stating how they were tested (“bootstraped and regtested on x86_64-linux”, for example). And of course, do an actual full regression-test :)

If that was done, then OK to commit.

Thanks,
FX

Re: Possible patch for PR fortran/66056

[Louis Krupp]

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

A patch with ChangeLog diffs is attached.

I ran "make && make install && make check-fortran" at the top level on x86_64-pc-linux-gnu.  (I always do that before I post a patch and again before I actually check anything in.  I sometimes forget to include files when I finally commit, but I'm always confident that the ones I remember are good :)

Louis

---- On Mon, 26 Oct 2015 01:51:04 -0700 FX<fxcoudert@gmail.com> wrote ---- 
 > > The problem:  Statement labels within a type declaration are put in the statement label tree belonging to the type declaration's namespace's (instead of the current namespace).  When the line is otherwise empty and an error is issued, gfc_free_st_label tries to delete the label from the label tree belonging to the current namespace and then frees the label structure, leaving an invalid statement label pointer in the type declaration's namespace's label tree.  When that namespace is cleaned up, bad things can happen. 
 >  
 > Seems OK. 
 > Please post patches with full ChangeLog entry, stating how they were tested (“bootstraped and regtested on x86_64-linux”, for example). And of course, do an actual full regression-test :) 
 >  
 > If that was done, then OK to commit. 
 >  
 > Thanks, 
 > FX


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3002 bytes --]

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(revision 229307)
+++ gcc/fortran/ChangeLog	(working copy)
@@ -1,3 +1,14 @@
+2015-10-26  Louis Krupp  <louis.krupp@zoho.com>
+
+	PR fortran/66056
+	* fortran.h: Include namespace pointer in statement label
+	structure.
+	* symbol.c (gfc_get_st_label): Store pointer to namespace
+	that owns the statement label tree in each label.
+	(gfc_free_st_label): Use namespace owning statement label
+	tree when deleting statement label.
+	* io.c: Initialize format_asterisk with NULL namespace pointer.
+
 2015-01-25  Paul Thomas  <pault@gcc.gnu.org>
 
 	PR fortran/67171
Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 229307)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -1291,6 +1291,8 @@ typedef struct gfc_st_label
   tree backend_decl;
 
   locus where;
+
+  gfc_namespace *ns;
 }
 gfc_st_label;
 
Index: gcc/fortran/io.c
===================================================================
--- gcc/fortran/io.c	(revision 229307)
+++ gcc/fortran/io.c	(working copy)
@@ -28,7 +28,7 @@ along with GCC; see the file COPYING3.  If not see
 
 gfc_st_label
 format_asterisk = {0, NULL, NULL, -1, ST_LABEL_FORMAT, ST_LABEL_FORMAT, NULL,
-		   0, {NULL, NULL}};
+		   0, {NULL, NULL}, NULL};
 
 typedef struct
 {
Index: gcc/fortran/symbol.c
===================================================================
--- gcc/fortran/symbol.c	(revision 229307)
+++ gcc/fortran/symbol.c	(working copy)
@@ -2195,7 +2195,7 @@ gfc_free_st_label (gfc_st_label *label)
   if (label == NULL)
     return;
 
-  gfc_delete_bbt (&gfc_current_ns->st_labels, label, compare_st_labels);
+  gfc_delete_bbt (&label->ns->st_labels, label, compare_st_labels);
 
   if (label->format != NULL)
     gfc_free_expr (label->format);
@@ -2260,6 +2260,7 @@ gfc_get_st_label (int labelno)
   lp->value = labelno;
   lp->defined = ST_LABEL_UNKNOWN;
   lp->referenced = ST_LABEL_UNKNOWN;
+  lp->ns = ns;
 
   gfc_insert_bbt (&ns->st_labels, lp, compare_st_labels);
 
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 229307)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2015-10-26  Louis Krupp  <louis.krupp@zoho.com>
+
+	PR fortran/66056
+	* gfortran.dg/empty_label_typedecl.f90: New test
+
 2015-01-25  Paul Thomas  <pault@gcc.gnu.org>
 
 	PR fortran/67171
Index: gcc/testsuite/gfortran.dg/empty_label_typedecl.f90
===================================================================
--- gcc/testsuite/gfortran.dg/empty_label_typedecl.f90	(revision 0)
+++ gcc/testsuite/gfortran.dg/empty_label_typedecl.f90	(working copy)
@@ -0,0 +1,8 @@
+! { dg-do compile }
+! { dg-options "-Werror" }
+subroutine s
+  type t
+  1 ! { dg-error "empty statement" }
+  end type
+end subroutine
+! { dg-excess-errors "warnings being treated as errors" }

Re: Possible patch for PR fortran/66056

[FX]

> I ran "make && make install && make check-fortran" at the top level on x86_64-pc-linux-gnu.  (I always do that before I post a patch and again before I actually check anything in.  I sometimes forget to include files when I finally commit, but I'm always confident that the ones I remember are good :)

OK to commit.

FX