[Fortran] Prevent segfault with dump-*-original for implicit class expressions.

Hi Mikael, hi all,

Mikael, thanks for the comments. I have worked in your suggestions and the
patch now looks like the attached one. 

To get the testsuite accepting a dump-fortran-original, I had to insert a
dg-prune-output. Without the prune output, dejagnu was reporting excess errors.
The prune removes all output after the first Namespace: that starts the
fortran-dump. I am not that happy with it, because ICEs in the dump will not be
detected that way. But I could not find a better solution yet. Does anyone know
of a better way to solve this? Best would be to have the fortran dump go to a
file like all other dumps, but that is not scope of this patch.

Bootstraps and regtests ok on x86_64-linux-gnu/F21.

OK for trunk now?

Regards,
	Andre

On Sat, 25 Apr 2015 23:13:34 +0200
Mikael Morin <mikael.morin@sfr.fr> wrote:

> Hello,
> 
> Le 13/03/2015 11:33, Andre Vehreschild a écrit :
> > Hi all,
> > 
> > this is another patch preventing a segfault. This time the segfault
> > occurred, when -fdump-(fortran|tree)-original was given with the program
> > having an implicit class set. The issue is that the _data component is
> > assumed to be present in a BT_CLASS w/o checking and trying to access the
> > unlimited polymorphic flag there. The patch fixes this by redirecting the
> > access to the flag to the correct position whether the _data component is
> > present or not.
> > 
> > Building a testcase for this is difficult for me. May be I am just blocked
> > in the head there. The issue occurred when trying to dump the
> > (fortran|tree)-original of the testcase gfortran.dg/implicit_class_1.f90. So
> > one could argue to add the flag to that testcase, but does it pay in
> > contrast to the additional effort each time the testsuite is executed? I
> > have added the test now, not being happy with it, but having no clue how to
> > do it better.
> > 
> > Bootstraps and regtests ok on x86_64-linux-gnu/F20.
> > 
> > Ok, for trunk?
> > 
> 
> Comments below:
> 
>  b/gcc/fortran/symbol.c
> > index 32eea21..3379f47 100644
> > --- a/gcc/fortran/symbol.c
> > +++ b/gcc/fortran/symbol.c
> 
> (skipped)
> 
> The beginning looks good.
> I suggest using 'ts->u.derived->attr.is_class' instead of
> 'strcmp (ts->u.derived.components->name, "_data") == 0'.
> No strong opinion, your choice.
> 
> 
> > @@ -4576,13 +4579,14 @@ gfc_type_compatible (gfc_typespec *ts1,
> > gfc_typespec *ts2) if (is_derived1 && is_derived2)
> >      return gfc_compare_derived_types (ts1->u.derived, ts2->u.derived);
> >  
> > -  if (is_derived1 && is_class2)
> > +  if (is_derived1 && is_class2 && ts2->u.derived->components)
> >      return gfc_compare_derived_types (ts1->u.derived,
> >  				      ts2->u.derived->components->ts.u.derived);
> > -  if (is_class1 && is_derived2)
> > +  if (is_class1 && is_derived2 && ts1->u.derived->components)
> >      return gfc_type_is_extension_of
> > (ts1->u.derived->components->ts.u.derived, ts2->u.derived);
> > -  else if (is_class1 && is_class2)
> > +  else if (is_class1 && is_class2 && ts1->u.derived->components
> > +	   && ts2->u.derived->components)
> >      return gfc_type_is_extension_of
> > (ts1->u.derived->components->ts.u.derived,
> > ts2->u.derived->components->ts.u.derived); else
> 
> The above change don't seem right.
> In the case where the class container is missing, you want to use
> "ts->u.derived" instead of "ts->u.derived->components->ts.u.derived",
> not skip the procedure call entirely.
> 
> > diff --git a/gcc/testsuite/gfortran.dg/implicit_class_1.f90
> > b/gcc/testsuite/gfortran.dg/implicit_class_1.f90 index 329f57a..fff1f2b
> > 100644 --- a/gcc/testsuite/gfortran.dg/implicit_class_1.f90
> > +++ b/gcc/testsuite/gfortran.dg/implicit_class_1.f90
> > @@ -4,6 +4,10 @@
> >  !
> >  ! Contributed by Reinhold Bader <Reinhold.Bader@lrz.de>
> >  
> > +! Add dump-tree-original to check, if the patch preventing a gfortran
> > +! segfault is working correctly.
> > +! { dg-options "-fdump-tree-original" }
> > +
> -fdump-tree-original doesn't trigger any bug here.
> So use -fdump-fortran-original (I'm not sure the testsuite will like it).
> 
> Mikael
>

Hello,

Le 28/04/2015 12:43, Andre Vehreschild a écrit :
> To get the testsuite accepting a dump-fortran-original, I had to insert a
> dg-prune-output. Without the prune output, dejagnu was reporting excess errors.
> The prune removes all output after the first Namespace: that starts the
> fortran-dump. I am not that happy with it, because ICEs in the dump will not be
> detected that way. But I could not find a better solution yet. Does anyone know
> of a better way to solve this? Best would be to have the fortran dump go to a
> file like all other dumps, but that is not scope of this patch.
> 
Without a better idea, the scope of the dg-prune-output should be
limited at least.
As the dump ends with a "-------" line, you can use the
"Namespace:.*-{42}" regular expression.
OK with that change.  Thanks.

Mikael