This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: patch fortran, pr 59746, internal compiler error : segmentation fault
- From: Mikael Morin <mikael dot morin at sfr dot fr>
- To: jimmie dot davis at l-3com dot com, gcc-patches at gcc dot gnu dot org, fortran at gcc dot gnu dot org
- Date: Sun, 09 Mar 2014 18:31:32 +0100
- Subject: Re: patch fortran, pr 59746, internal compiler error : segmentation fault
- Authentication-results: sourceware.org; auth=none
- References: <FFF95760268D324AB6DD9426E83C8DF70B2E0F21 at ARLEXCHMBX01 dot lst dot link dot l-3com dot com>
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