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

login
register
mail settings
Submitter jimmie.davis@l-3com.com
Date March 9, 2014, 12:59 p.m.
Message ID <FFF95760268D324AB6DD9426E83C8DF70B2E0F21@ARLEXCHMBX01.lst.link.l-3com.com>
Download mbox | patch
Permalink /patch/328324/
State New
Headers show

Comments

jimmie.davis@l-3com.com - March 9, 2014, 12:59 p.m.
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
Mikael Morin - March 9, 2014, 5:31 p.m.
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

Patch

===================================================================
--- 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);