| Message ID | FFF95760268D324AB6DD9426E83C8DF70B2E0F21@ARLEXCHMBX01.lst.link.l-3com.com |
|---|---|
| State | New |
| Headers | show |
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
=================================================================== --- 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);