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

[Mikael Morin <mikael.morin@sfr.fr>]

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