diff mbox

[Fortran,OOP] PR 61767: ICE in generate_finalization_wrapper at fortran/class.c:1491

Message ID CAKwh3qjNHZAGSkMw_wvwduyEWHVyknPDorbrG0-AxyDiJ3pmXw@mail.gmail.com
State New
Headers show

Commit Message

Janus Weil Dec. 8, 2016, 12:56 p.m. UTC
Hi all,

the attached patch fixes an ice-on-valid problem with finalization.
The ICE turned out to be caused by a bug in 'has_finalizer_component':
According to the documentation, this function is supposed to detect
whether a derived type has any nonpointer nonallocatable components
that have a finalizer. However it triggered also on pointer components
with a finalizer. Fixing this makes the ICE go away.

The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2016-12-08  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/61767
    * class.c (has_finalizer_component): Fix this function to detect only
    non-pointer non-allocatable components which have a finalizer.

2016-12-08  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/61767
    * gfortran.dg/finalize_31.f90: New test.

Comments

Andre Vehreschild Dec. 8, 2016, 1:10 p.m. UTC | #1
Hi Janus,

so when I interpret the testcase correctly, than the finalizer should not be
called, right? So adding a call abort() in the Finalize and allocating and
deallocating M in the main program should do no harm, but make the testcase IMO
more feasible. What do you think?

- Andre


On Thu, 8 Dec 2016 13:56:29 +0100
Janus Weil <janus@gcc.gnu.org> wrote:

> Hi all,
> 
> the attached patch fixes an ice-on-valid problem with finalization.
> The ICE turned out to be caused by a bug in 'has_finalizer_component':
> According to the documentation, this function is supposed to detect
> whether a derived type has any nonpointer nonallocatable components
> that have a finalizer. However it triggered also on pointer components
> with a finalizer. Fixing this makes the ICE go away.
> 
> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
> 
> Cheers,
> Janus
> 
> 
> 2016-12-08  Janus Weil  <janus@gcc.gnu.org>
> 
>     PR fortran/61767
>     * class.c (has_finalizer_component): Fix this function to detect only
>     non-pointer non-allocatable components which have a finalizer.
> 
> 2016-12-08  Janus Weil  <janus@gcc.gnu.org>
> 
>     PR fortran/61767
>     * gfortran.dg/finalize_31.f90: New test.
Janus Weil Dec. 8, 2016, 3:41 p.m. UTC | #2
Hi Andre,

> so when I interpret the testcase correctly, than the finalizer should not be
> called, right? So adding a call abort() in the Finalize and allocating and
> deallocating M in the main program should do no harm, but make the testcase IMO
> more feasible. What do you think?

thanks for the comment. Indeed it can not hurt to extend it into a
runtime test. New version attached (according to your suggestions). Ok
with this?

Cheers,
Janus



> On Thu, 8 Dec 2016 13:56:29 +0100
> Janus Weil <janus@gcc.gnu.org> wrote:
>
>> Hi all,
>>
>> the attached patch fixes an ice-on-valid problem with finalization.
>> The ICE turned out to be caused by a bug in 'has_finalizer_component':
>> According to the documentation, this function is supposed to detect
>> whether a derived type has any nonpointer nonallocatable components
>> that have a finalizer. However it triggered also on pointer components
>> with a finalizer. Fixing this makes the ICE go away.
>>
>> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>>
>> Cheers,
>> Janus
>>
>>
>> 2016-12-08  Janus Weil  <janus@gcc.gnu.org>
>>
>>     PR fortran/61767
>>     * class.c (has_finalizer_component): Fix this function to detect only
>>     non-pointer non-allocatable components which have a finalizer.
>>
>> 2016-12-08  Janus Weil  <janus@gcc.gnu.org>
>>
>>     PR fortran/61767
>>     * gfortran.dg/finalize_31.f90: New test.
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de
Andre Vehreschild Dec. 9, 2016, 12:36 p.m. UTC | #3
Hi Janus,

yes, that was what I had in mind for the testcase. Now it looks ok to me.

- Andre

On Thu, 8 Dec 2016 16:41:23 +0100
Janus Weil <janus@gcc.gnu.org> wrote:

> Hi Andre,
> 
> > so when I interpret the testcase correctly, than the finalizer should not be
> > called, right? So adding a call abort() in the Finalize and allocating and
> > deallocating M in the main program should do no harm, but make the testcase
> > IMO more feasible. What do you think?  
> 
> thanks for the comment. Indeed it can not hurt to extend it into a
> runtime test. New version attached (according to your suggestions). Ok
> with this?
> 
> Cheers,
> Janus
> 
> 
> 
> > On Thu, 8 Dec 2016 13:56:29 +0100
> > Janus Weil <janus@gcc.gnu.org> wrote:
> >  
> >> Hi all,
> >>
> >> the attached patch fixes an ice-on-valid problem with finalization.
> >> The ICE turned out to be caused by a bug in 'has_finalizer_component':
> >> According to the documentation, this function is supposed to detect
> >> whether a derived type has any nonpointer nonallocatable components
> >> that have a finalizer. However it triggered also on pointer components
> >> with a finalizer. Fixing this makes the ICE go away.
> >>
> >> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
> >>
> >> Cheers,
> >> Janus
> >>
> >>
> >> 2016-12-08  Janus Weil  <janus@gcc.gnu.org>
> >>
> >>     PR fortran/61767
> >>     * class.c (has_finalizer_component): Fix this function to detect only
> >>     non-pointer non-allocatable components which have a finalizer.
> >>
> >> 2016-12-08  Janus Weil  <janus@gcc.gnu.org>
> >>
> >>     PR fortran/61767
> >>     * gfortran.dg/finalize_31.f90: New test.  
> >
> >
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
Janus Weil Dec. 9, 2016, 1:22 p.m. UTC | #4
Hi Andre,

> yes, that was what I had in mind for the testcase. Now it looks ok to me.

thanks for reviewing, committed as r243483.

Cheers,
Janus



> On Thu, 8 Dec 2016 16:41:23 +0100
> Janus Weil <janus@gcc.gnu.org> wrote:
>
>> Hi Andre,
>>
>> > so when I interpret the testcase correctly, than the finalizer should not be
>> > called, right? So adding a call abort() in the Finalize and allocating and
>> > deallocating M in the main program should do no harm, but make the testcase
>> > IMO more feasible. What do you think?
>>
>> thanks for the comment. Indeed it can not hurt to extend it into a
>> runtime test. New version attached (according to your suggestions). Ok
>> with this?
>>
>> Cheers,
>> Janus
>>
>>
>>
>> > On Thu, 8 Dec 2016 13:56:29 +0100
>> > Janus Weil <janus@gcc.gnu.org> wrote:
>> >
>> >> Hi all,
>> >>
>> >> the attached patch fixes an ice-on-valid problem with finalization.
>> >> The ICE turned out to be caused by a bug in 'has_finalizer_component':
>> >> According to the documentation, this function is supposed to detect
>> >> whether a derived type has any nonpointer nonallocatable components
>> >> that have a finalizer. However it triggered also on pointer components
>> >> with a finalizer. Fixing this makes the ICE go away.
>> >>
>> >> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>> >>
>> >> Cheers,
>> >> Janus
>> >>
>> >>
>> >> 2016-12-08  Janus Weil  <janus@gcc.gnu.org>
>> >>
>> >>     PR fortran/61767
>> >>     * class.c (has_finalizer_component): Fix this function to detect only
>> >>     non-pointer non-allocatable components which have a finalizer.
>> >>
>> >> 2016-12-08  Janus Weil  <janus@gcc.gnu.org>
>> >>
>> >>     PR fortran/61767
>> >>     * gfortran.dg/finalize_31.f90: New test.
>> >
>> >
>> > --
>> > Andre Vehreschild * Email: vehre ad gmx dot de
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de
diff mbox

Patch

Index: gcc/fortran/class.c
===================================================================
--- gcc/fortran/class.c	(revision 243433)
+++ gcc/fortran/class.c	(working copy)
@@ -841,20 +841,19 @@  has_finalizer_component (gfc_symbol *derived)
    gfc_component *c;
 
   for (c = derived->components; c; c = c->next)
-    {
-      if (c->ts.type == BT_DERIVED && c->ts.u.derived->f2k_derived
-	  && c->ts.u.derived->f2k_derived->finalizers)
-	return true;
+    if (c->ts.type == BT_DERIVED && !c->attr.pointer && !c->attr.allocatable)
+      {
+	if (c->ts.u.derived->f2k_derived
+	    && c->ts.u.derived->f2k_derived->finalizers)
+	  return true;
 
-      /* Stop infinite recursion through this function by inhibiting
-	 calls when the derived type and that of the component are
-	 the same.  */
-      if (c->ts.type == BT_DERIVED
-	  && !gfc_compare_derived_types (derived, c->ts.u.derived)
-	  && !c->attr.pointer && !c->attr.allocatable
-	  && has_finalizer_component (c->ts.u.derived))
-	return true;
-    }
+	/* Stop infinite recursion through this function by inhibiting
+	  calls when the derived type and that of the component are
+	  the same.  */
+	if (!gfc_compare_derived_types (derived, c->ts.u.derived)
+	    && has_finalizer_component (c->ts.u.derived))
+	  return true;
+      }
   return false;
 }