patch fortran, pr 59746, internal compiler error : segmentation fault

patch fortran, pr 59746, internal compiler error : segmentation fault

[jimmie.davis]

The code would only remove a variable from a common block if it was just defined in the previous statement. The PR showed a case where a pre-existing variable was put in the common block.  When it was not removed, the common block list was wrong and segfaulted (or infinite looped) when used later on.

I changed it so it would remove a variable from a common block if it was just defined, or if it previously existed and was put in the common block in the last statement.

This patch works with the example given in the PR and has no regressions in the testsuite.    

tested i686/linux.


--bud davis

2013-03-06  Bud Davis <jmdavis@link.com>

	PR fortran/59746
	* symbol.c (gfc_restore_last_undo_checkpoint): Delete a common block
	symbol if it was put in the list.

 
 Index: gcc/gcc/testsuite/gfortran.dg/pr59746.f90
===================================================================
--- gcc/gcc/testsuite/gfortran.dg/pr59746.f90	(revision 0)
+++ gcc/gcc/testsuite/gfortran.dg/pr59746.f90	(revision 0)
@@ -0,0 +1,18 @@
+! { dg-do compile }
+!pr59746
+      CALL RCCFL(NVE,IR,NU3,VE(1,1,1,I))
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+!  the PR only contained the two above.
+!  success is no segfaults or infinite loops.  
+!  let's check some combinations
+     CALL ABC (INTG)
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     CALL DEF (NT1)
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     CALL GHI (NRESL)
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     END
Index: gcc/gcc/fortran/symbol.c
===================================================================
--- gcc/gcc/fortran/symbol.c	(revision 208437)
+++ gcc/gcc/fortran/symbol.c	(working copy)
@@ -3069,9 +3069,9 @@
 
   FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
     {
-      if (p->gfc_new)
+      if (p->gfc_new || (p->attr.in_common && !p->old_symbol->attr.in_common) )
 	{
-	  /* Symbol was new.  */
+	  /* Symbol was new. Or was old and just put in common */
 	  if (p->attr.in_common && p->common_block && p->common_block->head)
 	    {
 	      /* If the symbol was added to any common block, it
@@ -3115,6 +3115,9 @@
 	  /* The derived type is saved in the symtree with the first
 	     letter capitalized; the all lower-case version to the
 	     derived type contains its associated generic function.  */
+         }
+         if (p->gfc_new) 
+         {
 	  if (p->attr.flavor == FL_DERIVED)
 	    gfc_delete_symtree (&p->ns->sym_root, gfc_get_string ("%c%s",
                         (char) TOUPPER ((unsigned char) p->name[0]),
@@ -3125,7 +3128,9 @@
 	  gfc_release_symbol (p);
 	}
       else
-	restore_old_symbol (p);
+        {
+	  restore_old_symbol (p);
+        }
     }
 
   latest_undo_chgset->syms.truncate (0);

Re: patch fortran, pr 59746, internal compiler error : segmentation fault

[Mikael Morin]

Hello,

Le 09/03/2014 13:59, jimmie.davis@l-3com.com a écrit :
> 
> 
> The code would only remove a variable from a common block if it was just defined in the previous statement. The PR showed a case where a pre-existing variable was put in the common block.  When it was not removed, the common block list was wrong and segfaulted (or infinite looped) when used later on.
> 
> I changed it so it would remove a variable from a common block if it was just defined, or if it previously existed and was put in the common block in the last statement.
> 
> This patch works with the example given in the PR and has no regressions in the testsuite.    
> 
> tested i686/linux.
> 
> 
> --bud davis
> 
[...]
> Index: gcc/gcc/fortran/symbol.c
> ===================================================================
> --- gcc/gcc/fortran/symbol.c	(revision 208437)
> +++ gcc/gcc/fortran/symbol.c	(working copy)
> @@ -3069,9 +3069,9 @@
>  
>    FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
>      {
> -      if (p->gfc_new)
> +      if (p->gfc_new || (p->attr.in_common && !p->old_symbol->attr.in_common) )
>  	{
> -	  /* Symbol was new.  */
> +	  /* Symbol was new. Or was old and just put in common */
>  	  if (p->attr.in_common && p->common_block && p->common_block->head)
Please merge the two ifs; it seems they have exactly the same scope
after the patch.

>  	    {
>  	      /* If the symbol was added to any common block, it
> @@ -3115,6 +3115,9 @@
>  	  /* The derived type is saved in the symtree with the first
>  	     letter capitalized; the all lower-case version to the
>  	     derived type contains its associated generic function.  */
This comment applies to the TOUPPER thing below...

> +         }
> +         if (p->gfc_new) 
> +         {
... so it should be put here. Also the indentation is wrong.

>  	  if (p->attr.flavor == FL_DERIVED)
>  	    gfc_delete_symtree (&p->ns->sym_root, gfc_get_string ("%c%s",
>                          (char) TOUPPER ((unsigned char) p->name[0]),
> @@ -3125,7 +3128,9 @@
>  	  gfc_release_symbol (p);
>  	}
>        else
> -	restore_old_symbol (p);
> +        {
> +	  restore_old_symbol (p);
> +        }
>      }
This is unnecessary.

Otherwise looks goodish.  This is not a regression, but I suppose it
will be acceptable.

Mikael

RE: patch fortran, pr 59746, internal compiler error : segmentation fault

[jimmie.davis]

Comments from Mikael:
 
#1.  Please merge the two ifs; it seems they have exactly the same scope
after the patch.
 
#2.  This comment applies to the TOUPPER thing below...
 .. so it should be put here. Also the indentation is wrong.
 
#3.This is unnecessary.
 
===========================

All corrected in the patch below.  
This patch is not very important.   If we are in 'regressions fix only' mode, then this needs to sit until stage 1 happens....It is not worth any kind of breakage.

Reran the testsuite.  No new failures.


regards,
Bud Davis


Index: gcc/gcc/fortran/symbol.c
===================================================================
--- gcc/gcc/fortran/symbol.c	(revision 208437)
+++ gcc/gcc/fortran/symbol.c	(working copy)
@@ -3069,65 +3069,65 @@
 
   FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
     {
-      if (p->gfc_new)
-	{
-	  /* Symbol was new.  */
-	  if (p->attr.in_common && p->common_block && p->common_block->head)
-	    {
-	      /* If the symbol was added to any common block, it
-		 needs to be removed to stop the resolver looking
-		 for a (possibly) dead symbol.  */
+      /* Symbol was new. Or was old and just put in common */
+      if ((p->gfc_new || 
+          (p->attr.in_common && !p->old_symbol->attr.in_common )) && 
+           p->attr.in_common && p->common_block && p->common_block->head)
+        {
+          /* If the symbol was added to any common block, it
+             needs to be removed to stop the resolver looking
+             for a (possibly) dead symbol.  */
 
-	      if (p->common_block->head == p && !p->common_next)
-		{
-		  gfc_symtree st, *st0;
-		  st0 = find_common_symtree (p->ns->common_root,
-					     p->common_block);
-		  if (st0)
-		    {
-		      st.name = st0->name;
-		      gfc_delete_bbt (&p->ns->common_root, &st, compare_symtree);
-		      free (st0);
-		    }
-		}
+          if (p->common_block->head == p && !p->common_next)
+            {
+              gfc_symtree st, *st0;
+              st0 = find_common_symtree (p->ns->common_root,
+                                         p->common_block);
+              if (st0)
+                {
+                  st.name = st0->name;
+		  gfc_delete_bbt (&p->ns->common_root, &st, compare_symtree);
+		  free (st0);
+                }
+            }
 
-	      if (p->common_block->head == p)
-	        p->common_block->head = p->common_next;
-	      else
-		{
-		  gfc_symbol *cparent, *csym;
+	    if (p->common_block->head == p)
+	      p->common_block->head = p->common_next;
+	    else
+              {
+                gfc_symbol *cparent, *csym;
 
-		  cparent = p->common_block->head;
-		  csym = cparent->common_next;
+                cparent = p->common_block->head;
+                csym = cparent->common_next;
 
-		  while (csym != p)
-		    {
-		      cparent = csym;
-		      csym = csym->common_next;
-		    }
+                while (csym != p)
+                  {
+                    cparent = csym;
+                    csym = csym->common_next;
+                  }
 
-		  gcc_assert(cparent->common_next == p);
+                  gcc_assert(cparent->common_next == p);
+                  cparent->common_next = csym->common_next;
+              }
+        }
+        if (p->gfc_new) 
+          {
+            /* The derived type is saved in the symtree with the first
+               letter capitalized; the all lower-case version to the
+               derived type contains its associated generic function.  */
 
-		  cparent->common_next = csym->common_next;
-		}
-	    }
+            if (p->attr.flavor == FL_DERIVED)
+              gfc_delete_symtree (&p->ns->sym_root, gfc_get_string ("%c%s",
+                                  (char) TOUPPER ((unsigned char) p->name[0]),
+                                  &p->name[1]));
+            else
+	      gfc_delete_symtree (&p->ns->sym_root, p->name);
 
-	  /* The derived type is saved in the symtree with the first
-	     letter capitalized; the all lower-case version to the
-	     derived type contains its associated generic function.  */
-	  if (p->attr.flavor == FL_DERIVED)
-	    gfc_delete_symtree (&p->ns->sym_root, gfc_get_string ("%c%s",
-                        (char) TOUPPER ((unsigned char) p->name[0]),
-                        &p->name[1]));
-	  else
-	    gfc_delete_symtree (&p->ns->sym_root, p->name);
-
-	  gfc_release_symbol (p);
-	}
-      else
-	restore_old_symbol (p);
+	    gfc_release_symbol (p);
+	  }
+        else
+          restore_old_symbol (p);
     }
-
   latest_undo_chgset->syms.truncate (0);
   latest_undo_chgset->tbps.truncate (0);
 

Re: patch fortran, pr 59746, internal compiler error : segmentation fault

[Mikael Morin]

Le 09/03/2014 20:47, jimmie.davis@l-3com.com a écrit :
> Comments from Mikael:
>  
> #1.  Please merge the two ifs; it seems they have exactly the same scope
> after the patch.
>  
> #2.  This comment applies to the TOUPPER thing below...
>  .. so it should be put here. Also the indentation is wrong.
>  
> #3.This is unnecessary.
>  
> ===========================
> 
> All corrected in the patch below.  
> This patch is not very important.   If we are in 'regressions fix only' mode, then this needs to sit until stage 1 happens....It is not worth any kind of breakage.
Alright, don't forget to ping the patch during next stage1 then.

Next review iteration:
> Index: gcc/gcc/fortran/symbol.c
> ===================================================================
> --- gcc/gcc/fortran/symbol.c	(revision 208437)
> +++ gcc/gcc/fortran/symbol.c	(working copy)
> @@ -3069,65 +3069,65 @@
>  
>    FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
>      {
> -      if (p->gfc_new)
> -	{
> -	  /* Symbol was new.  */
> -	  if (p->attr.in_common && p->common_block && p->common_block->head)
> -	    {
> -	      /* If the symbol was added to any common block, it
> -		 needs to be removed to stop the resolver looking
> -		 for a (possibly) dead symbol.  */
> +      /* Symbol was new. Or was old and just put in common */
> +      if ((p->gfc_new || 
> +          (p->attr.in_common && !p->old_symbol->attr.in_common )) && 
> +           p->attr.in_common && p->common_block && p->common_block->head)
write this instead:
if (p->attr.in_common && p->common_block && p->common_block->head
    && (p->gfc_new || !p->old_symbol->attr.in_common))

[p->attr.in_common is less likely than p->gfc_new which happens all the
time; we may as well short-circuit the latter.]


For the rest of the patch, there are indentation issues, any leading 8
spaces block should be replaced with a tab.
You can use "contrib/check_GNU_style.sh your_patch" to check common
style issues)

> +           )
> +        {
> +          /* If the symbol was added to any common block, it
> +             needs to be removed to stop the resolver looking
> +             for a (possibly) dead symbol.  */
>  
> -	      if (p->common_block->head == p && !p->common_next)
> -		{
> -		  gfc_symtree st, *st0;
> -		  st0 = find_common_symtree (p->ns->common_root,
> -					     p->common_block);
> -		  if (st0)
> -		    {
> -		      st.name = st0->name;
> -		      gfc_delete_bbt (&p->ns->common_root, &st, compare_symtree);
> -		      free (st0);
> -		    }
> -		}
> +          if (p->common_block->head == p && !p->common_next)
> +            {
> +              gfc_symtree st, *st0;
> +              st0 = find_common_symtree (p->ns->common_root,
> +                                         p->common_block);
> +              if (st0)
> +                {
> +                  st.name = st0->name;
> +		  gfc_delete_bbt (&p->ns->common_root, &st, compare_symtree);
> +		  free (st0);
> +                }
> +            }
>  
> -	      if (p->common_block->head == p)
> -	        p->common_block->head = p->common_next;
> -	      else
> -		{
> -		  gfc_symbol *cparent, *csym;


> +	    if (p->common_block->head == p)
I think the code block starting here is indented too much.

> +	      p->common_block->head = p->common_next;
> +	    else
> +              {
> +                gfc_symbol *cparent, *csym;
>  
> -		  cparent = p->common_block->head;
> -		  csym = cparent->common_next;
> +                cparent = p->common_block->head;
> +                csym = cparent->common_next;
>  
> -		  while (csym != p)
> -		    {
> -		      cparent = csym;
> -		      csym = csym->common_next;
> -		    }
> +                while (csym != p)
> +                  {
> +                    cparent = csym;
> +                    csym = csym->common_next;
> +                  }
>  
> -		  gcc_assert(cparent->common_next == p);
> +                  gcc_assert(cparent->common_next == p);
> +                  cparent->common_next = csym->common_next;
> +              }
> +        }


> +        if (p->gfc_new) 
Same here, the code after this shouldn't need reindenting.

> +          {
> +            /* The derived type is saved in the symtree with the first
> +               letter capitalized; the all lower-case version to the
> +               derived type contains its associated generic function.  */
>  
> -		  cparent->common_next = csym->common_next;
> -		}
> -	    }
> +            if (p->attr.flavor == FL_DERIVED)
> +              gfc_delete_symtree (&p->ns->sym_root, gfc_get_string ("%c%s",
> +                                  (char) TOUPPER ((unsigned char) p->name[0]),
> +                                  &p->name[1]));
> +            else
> +	      gfc_delete_symtree (&p->ns->sym_root, p->name);
>  
> -	  /* The derived type is saved in the symtree with the first
> -	     letter capitalized; the all lower-case version to the
> -	     derived type contains its associated generic function.  */
> -	  if (p->attr.flavor == FL_DERIVED)
> -	    gfc_delete_symtree (&p->ns->sym_root, gfc_get_string ("%c%s",
> -                        (char) TOUPPER ((unsigned char) p->name[0]),
> -                        &p->name[1]));
> -	  else
> -	    gfc_delete_symtree (&p->ns->sym_root, p->name);
> -
> -	  gfc_release_symbol (p);
> -	}
> -      else
> -	restore_old_symbol (p);
> +	    gfc_release_symbol (p);
> +	  }
> +        else
> +          restore_old_symbol (p);
>      }
> -
>    latest_undo_chgset->syms.truncate (0);
>    latest_undo_chgset->tbps.truncate (0);
>  
> 

Mikael

RE: patch fortran, pr 59746, internal compiler error : segmentation fault

[jimmie.davis]

________________________________________
From: Mikael Morin [mikael.morin@sfr.fr]
Sent: Sunday, March 09, 2014 3:40 PM
To: Davis, Bud @ SSG - Link; gcc-patches@gcc.gnu.org; fortran@gcc.gnu.org
Subject: Re: patch fortran, pr 59746, internal compiler error : segmentation fault

   
> -      if (p->gfc_new)
 
>write this instead:
>if (p->attr.in_common && p->common_block && p->common_block->head
>    && (p->gfc_new || !p->old_symbol->attr.in_common))

>[p->attr.in_common is less likely than p->gfc_new which happens all the
>time; we may as well short-circuit the latter.]

Done.

>For the rest of the patch, there are indentation issues, any leading 8
>spaces block should be replaced with a tab.

Done.  All sequences of 8 spaces are replaced with a tab.

>You can use "contrib/check_GNU_style.sh your_patch" to check common
>style issues)

All issues fixed, except for test case lines, the 'dg error' lines are over 80 columns.  A lot of the tests do this, I am guessing this violation is acceptable.

 
> +         if (p->common_block->head == p)
>I think the code block starting here is indented too much.

Fixed.

> +        if (p->gfc_new)
>Same here, the code after this shouldn't need reindenting.
 
Fixed.

>Mikael

Below is the revised patch.  Still passes the testsuite with no new failures.


regards,
Bud Davis





Index: gcc/gcc/testsuite/gfortran.dg/pr59746.f90
===================================================================
--- gcc/gcc/testsuite/gfortran.dg/pr59746.f90	(revision 0)
+++ gcc/gcc/testsuite/gfortran.dg/pr59746.f90	(revision 0)
@@ -0,0 +1,18 @@
+! { dg-do compile }
+!pr59746
+      CALL RCCFL (NVE,IR,NU3,VE (1,1,1,I))
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+!  the PR only contained the two above.
+!  success is no segfaults or infinite loops.
+!  let's check some combinations
+     CALL ABC (INTG)
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     CALL DEF (NT1)
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     CALL GHI (NRESL)
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+     END
Index: gcc/gcc/fortran/symbol.c
===================================================================
--- gcc/gcc/fortran/symbol.c	(revision 208437)
+++ gcc/gcc/fortran/symbol.c	(working copy)
@@ -3069,56 +3069,56 @@
 
   FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
     {
-      if (p->gfc_new)
+      /* Symbol was new.  Or was old and just put in common.  */
+      if ( p->attr.in_common && p->common_block && p->common_block->head
+	   && (p->gfc_new || !p->old_symbol->attr.in_common))
 	{
-	  /* Symbol was new.  */
-	  if (p->attr.in_common && p->common_block && p->common_block->head)
-	    {
-	      /* If the symbol was added to any common block, it
-		 needs to be removed to stop the resolver looking
-		 for a (possibly) dead symbol.  */
+	  /* If the symbol was added to any common block, it
+	  needs to be removed to stop the resolver looking
+	  for a (possibly) dead symbol.  */
 
-	      if (p->common_block->head == p && !p->common_next)
+	  if (p->common_block->head == p && !p->common_next)
+	    {
+	      gfc_symtree st, *st0;
+	      st0 = find_common_symtree (p->ns->common_root,
+					 p->common_block);
+	      if (st0)
 		{
-		  gfc_symtree st, *st0;
-		  st0 = find_common_symtree (p->ns->common_root,
-					     p->common_block);
-		  if (st0)
-		    {
-		      st.name = st0->name;
-		      gfc_delete_bbt (&p->ns->common_root, &st, compare_symtree);
-		      free (st0);
-		    }
+		  st.name = st0->name;
+		  gfc_delete_bbt (&p->ns->common_root, &st, compare_symtree);
+		  free (st0);
 		}
+	    }
 
-	      if (p->common_block->head == p)
-	        p->common_block->head = p->common_next;
-	      else
-		{
-		  gfc_symbol *cparent, *csym;
+	  if (p->common_block->head == p)
+	    p->common_block->head = p->common_next;
+	  else
+	    {
+	      gfc_symbol *cparent, *csym;
 
-		  cparent = p->common_block->head;
-		  csym = cparent->common_next;
+	      cparent = p->common_block->head;
+	      csym = cparent->common_next;
 
-		  while (csym != p)
-		    {
-		      cparent = csym;
-		      csym = csym->common_next;
-		    }
-
-		  gcc_assert(cparent->common_next == p);
-
-		  cparent->common_next = csym->common_next;
+	      while (csym != p)
+		{
+		  cparent = csym;
+		  csym = csym->common_next;
 		}
-	    }
 
+	      gcc_assert (cparent->common_next == p);
+	      cparent->common_next = csym->common_next;
+	    }
+	}
+      if (p->gfc_new)
+	{
 	  /* The derived type is saved in the symtree with the first
 	     letter capitalized; the all lower-case version to the
 	     derived type contains its associated generic function.  */
+
 	  if (p->attr.flavor == FL_DERIVED)
 	    gfc_delete_symtree (&p->ns->sym_root, gfc_get_string ("%c%s",
-                        (char) TOUPPER ((unsigned char) p->name[0]),
-                        &p->name[1]));
+				(char) TOUPPER ((unsigned char) p->name[0]),
+				&p->name[1]));
 	  else
 	    gfc_delete_symtree (&p->ns->sym_root, p->name);
 
@@ -3127,7 +3127,6 @@
       else
 	restore_old_symbol (p);
     }
-
   latest_undo_chgset->syms.truncate (0);
   latest_undo_chgset->tbps.truncate (0);
 

 

Re: patch fortran, pr 59746, internal compiler error : segmentation fault

[Mikael Morin]

Hello,

Le 10/03/2014 03:15, jimmie.davis@l-3com.com a écrit :
> Index: gcc/gcc/fortran/symbol.c
> ===================================================================
> --- gcc/gcc/fortran/symbol.c	(revision 208437)
> +++ gcc/gcc/fortran/symbol.c	(working copy)
> @@ -3069,56 +3069,56 @@
>  
>    FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
>      {
> -      if (p->gfc_new)
> +      /* Symbol was new.  Or was old and just put in common.  */
Now the comment needs updating as "just put in common" also applies to
the "new" case. Or you can also remove it ("just put in common" is
somewhat redundant with the other comment anyway).

> +      if ( p->attr.in_common && p->common_block && p->common_block->head
> +	   && (p->gfc_new || !p->old_symbol->attr.in_common))
>  	{
> -	  /* Symbol was new.  */
> -	  if (p->attr.in_common && p->common_block && p->common_block->head)
> -	    {
> -	      /* If the symbol was added to any common block, it
> -		 needs to be removed to stop the resolver looking
> -		 for a (possibly) dead symbol.  */
> +	  /* If the symbol was added to any common block, it
> +	  needs to be removed to stop the resolver looking
> +	  for a (possibly) dead symbol.  */
"needs" should be aligned with "If" like it was before; same for "for".

Now we are in pretty good shape.

The ICE happens with invalid code after reporting an error, correct?
Then I agree, this should rather wait for stage 1.

Thanks
Mikael

Re: patch fortran, pr 59746, internal compiler error : segmentation fault

[Tobias Burnus]

Given that we are now in stage 1: Mikael and Bud, what's the status of 
this patch?

http://gcc.gnu.org/ml/fortran/2014-03/msg00098.html

Tobias

On January 13, 2014 22:18, Mikael Morin wrote:
> Hello,
>
> Le 10/03/2014 03:15, jimmie.davis@l-3com.com a écrit :
>> Index: gcc/gcc/fortran/symbol.c
>> ===================================================================
>> --- gcc/gcc/fortran/symbol.c	(revision 208437)
>> +++ gcc/gcc/fortran/symbol.c	(working copy)
>> @@ -3069,56 +3069,56 @@
>>   
>>     FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
>>       {
>> -      if (p->gfc_new)
>> +      /* Symbol was new.  Or was old and just put in common.  */
> Now the comment needs updating as "just put in common" also applies to
> the "new" case. Or you can also remove it ("just put in common" is
> somewhat redundant with the other comment anyway).
>
>> +      if ( p->attr.in_common && p->common_block && p->common_block->head
>> +	   && (p->gfc_new || !p->old_symbol->attr.in_common))
>>   	{
>> -	  /* Symbol was new.  */
>> -	  if (p->attr.in_common && p->common_block && p->common_block->head)
>> -	    {
>> -	      /* If the symbol was added to any common block, it
>> -		 needs to be removed to stop the resolver looking
>> -		 for a (possibly) dead symbol.  */
>> +	  /* If the symbol was added to any common block, it
>> +	  needs to be removed to stop the resolver looking
>> +	  for a (possibly) dead symbol.  */
> "needs" should be aligned with "If" like it was before; same for "for".
>
> Now we are in pretty good shape.
>
> The ICE happens with invalid code after reporting an error, correct?
> Then I agree, this should rather wait for stage 1.
>
> Thanks
> Mikael
>

Re: patch fortran, pr 59746, internal compiler error : segmentation fault

[Mikael Morin]

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

Hello,

I'm unburrying the patch from the thread starting at:
https://gcc.gnu.org/ml/gcc-patches/2014-03/msg00439.html

I provide the patch in two flavors read-only (without whitespace 
changes) and  write-only (with them).
This has been tested on x86_64-unknown-linux-gnu.  OK for trunk?

Mikael




[-- Attachment #2: pr59746_4.CL --]
[-- Type: text/plain, Size: 304 bytes --]

2015-07-29  Bud Davis  <jmdavis@link.com>
	    Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/59746
	* symbol.c (gfc_restore_last_undo_checkpoint): Delete a common block
	symbol if it was put in the list.

2015-07-29  Bud Davis  <jmdavis@link.com>

	PR fortran/59746
	* gfortran.dg/common_22.f90: New.


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

*** /tmp/ro4P6U_symbol.c	2015-07-29 20:08:48.675970662 +0200
--- gcc/fortran/symbol.c	2015-07-29 19:48:25.580979685 +0200
*************** gfc_restore_last_undo_checkpoint (void)
*** 3168,3177 ****
  
    FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
      {
!       if (p->gfc_new)
! 	{
! 	  /* Symbol was new.  */
! 	  if (p->attr.in_common && p->common_block && p->common_block->head)
  	    {
  	      /* If the symbol was added to any common block, it
  		 needs to be removed to stop the resolver looking
--- 3168,3177 ----
  
    FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
      {
!       /* Symbol was new. Or was old and just put in common */
!       if ((p->gfc_new
! 	   || (p->attr.in_common && !p->old_symbol->attr.in_common ))
! 	  && p->attr.in_common && p->common_block && p->common_block->head)
  	{
  	  /* If the symbol was added to any common block, it
  	     needs to be removed to stop the resolver looking
*************** gfc_restore_last_undo_checkpoint (void)
*** 3206,3216 ****
  		    }
  
  		  gcc_assert(cparent->common_next == p);
- 
  		  cparent->common_next = csym->common_next;
  		}
  	    }
! 
  	  /* The derived type is saved in the symtree with the first
  	     letter capitalized; the all lower-case version to the
  	     derived type contains its associated generic function.  */
--- 3206,3216 ----
  		}
  
  	      gcc_assert(cparent->common_next == p);
  	      cparent->common_next = csym->common_next;
  	    }
  	}
!       if (p->gfc_new)
! 	{
  	  /* The derived type is saved in the symtree with the first
  	     letter capitalized; the all lower-case version to the
  	     derived type contains its associated generic function.  */
*** /dev/null	2015-07-28 11:36:43.193098438 +0200
--- gcc/testsuite/gfortran.dg/common_22.f90	2015-07-29 19:59:59.864974563 +0200
***************
*** 0 ****
--- 1,24 ----
+ ! { dg-do compile }
+ !
+ ! PR fortran/59746
+ ! Check that symbols present in common block are properly cleaned up
+ ! upon error.
+ !
+ ! Contributed by Bud Davis  <jmdavis@link.com>
+ 
+       CALL RCCFL (NVE,IR,NU3,VE (1,1,1,I))
+       COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+       COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+ !  the PR only contained the two above.
+ !  success is no segfaults or infinite loops.
+ !  let's check some combinations
+      CALL ABC (INTG)
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      CALL DEF (NT1)
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      CALL GHI (NRESL)
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      END



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: pr59746_4_c.diff --]
[-- Type: text/x-patch; name="pr59746_4_c.diff", Size: 4523 bytes --]

Index: fortran/symbol.c
===================================================================
*** fortran/symbol.c	(révision 226157)
--- fortran/symbol.c	(copie de travail)
***************
*** 3168,3216 ****
  
    FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
      {
!       if (p->gfc_new)
  	{
! 	  /* Symbol was new.  */
! 	  if (p->attr.in_common && p->common_block && p->common_block->head)
! 	    {
! 	      /* If the symbol was added to any common block, it
! 		 needs to be removed to stop the resolver looking
! 		 for a (possibly) dead symbol.  */
  
! 	      if (p->common_block->head == p && !p->common_next)
  		{
! 		  gfc_symtree st, *st0;
! 		  st0 = find_common_symtree (p->ns->common_root,
! 					     p->common_block);
! 		  if (st0)
! 		    {
! 		      st.name = st0->name;
! 		      gfc_delete_bbt (&p->ns->common_root, &st, compare_symtree);
! 		      free (st0);
! 		    }
  		}
  
! 	      if (p->common_block->head == p)
! 	        p->common_block->head = p->common_next;
! 	      else
! 		{
! 		  gfc_symbol *cparent, *csym;
! 
! 		  cparent = p->common_block->head;
! 		  csym = cparent->common_next;
! 
! 		  while (csym != p)
! 		    {
! 		      cparent = csym;
! 		      csym = csym->common_next;
! 		    }
  
! 		  gcc_assert(cparent->common_next == p);
  
! 		  cparent->common_next = csym->common_next;
  		}
- 	    }
  
  	  /* The derived type is saved in the symtree with the first
  	     letter capitalized; the all lower-case version to the
  	     derived type contains its associated generic function.  */
--- 3168,3216 ----
  
    FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
      {
!       /* Symbol was new. Or was old and just put in common */
!       if ((p->gfc_new
! 	   || (p->attr.in_common && !p->old_symbol->attr.in_common ))
! 	  && p->attr.in_common && p->common_block && p->common_block->head)
  	{
! 	  /* If the symbol was added to any common block, it
! 	     needs to be removed to stop the resolver looking
! 	     for a (possibly) dead symbol.  */
  
! 	  if (p->common_block->head == p && !p->common_next)
! 	    {
! 	      gfc_symtree st, *st0;
! 	      st0 = find_common_symtree (p->ns->common_root,
! 					 p->common_block);
! 	      if (st0)
  		{
! 		  st.name = st0->name;
! 		  gfc_delete_bbt (&p->ns->common_root, &st, compare_symtree);
! 		  free (st0);
  		}
+ 	    }
  
! 	  if (p->common_block->head == p)
! 	    p->common_block->head = p->common_next;
! 	  else
! 	    {
! 	      gfc_symbol *cparent, *csym;
  
! 	      cparent = p->common_block->head;
! 	      csym = cparent->common_next;
  
! 	      while (csym != p)
! 		{
! 		  cparent = csym;
! 		  csym = csym->common_next;
  		}
  
+ 	      gcc_assert(cparent->common_next == p);
+ 	      cparent->common_next = csym->common_next;
+ 	    }
+ 	}
+       if (p->gfc_new)
+ 	{
  	  /* The derived type is saved in the symtree with the first
  	     letter capitalized; the all lower-case version to the
  	     derived type contains its associated generic function.  */
Index: testsuite/gfortran.dg/common_22.f90
===================================================================
*** testsuite/gfortran.dg/common_22.f90	(révision 0)
--- testsuite/gfortran.dg/common_22.f90	(copie de travail)
***************
*** 0 ****
--- 1,24 ----
+ ! { dg-do compile }
+ !
+ ! PR fortran/59746
+ ! Check that symbols present in common block are properly cleaned up
+ ! upon error.
+ !
+ ! Contributed by Bud Davis  <jmdavis@link.com>
+ 
+       CALL RCCFL (NVE,IR,NU3,VE (1,1,1,I))
+       COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+       COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+ !  the PR only contained the two above.
+ !  success is no segfaults or infinite loops.
+ !  let's check some combinations
+      CALL ABC (INTG)
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      CALL DEF (NT1)
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      CALL GHI (NRESL)
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      COMMON /CCFILE/ INTG,NT1,NT2,NT3,NVM,NVE,NFRLE,NRESF,NRESL !{ dg-error "Unexpected COMMON" }
+      END

*ping* Re: patch fortran, pr 59746, internal compiler error : segmentation fault

[Mikael Morin]

Le 29/07/2015 20:35, Mikael Morin a écrit :
> Hello,
>
> I'm unburrying the patch from the thread starting at:
> https://gcc.gnu.org/ml/gcc-patches/2014-03/msg00439.html
>
> I provide the patch in two flavors read-only (without whitespace
> changes) and  write-only (with them).
> This has been tested on x86_64-unknown-linux-gnu.  OK for trunk?
>

Ping: https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02494.html

Re: *ping* Re: patch fortran, pr 59746, internal compiler error : segmentation fault

[Paul Richard Thomas]

Hi Mikael,

This is OK for trunk.

Thanks for resurrecting the patch.

Cheers

Paul

On 6 August 2015 at 12:11, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Le 29/07/2015 20:35, Mikael Morin a écrit :
>>
>> Hello,
>>
>> I'm unburrying the patch from the thread starting at:
>> https://gcc.gnu.org/ml/gcc-patches/2014-03/msg00439.html
>>
>> I provide the patch in two flavors read-only (without whitespace
>> changes) and  write-only (with them).
>> This has been tested on x86_64-unknown-linux-gnu.  OK for trunk?
>>
>
> Ping: https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02494.html



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx