[Patch, fortran] PR80477 - [OOP] Polymorphic function result generates memory leak

[Paul Richard Thomas]

Several attempts, including mine, were made to fix this bug since it
was posted. They were all attacking the wrong place. Instead of
providing the free of the class _data as part of the call to
'add_a_type' it should be included in the post block of the argument
processing in the call to 'assign_a_type'. The comment in the patch
says the rest.

Bootstrapped and regtested on FC28/x86_64 - OK for trunk?

Paul

2017-07-27 Paul Thomas <***@gcc.gnu.org>

PR fortran/80477
* trans-expr.c (gfc_conv_procedure_call): Allocatable class
results being passed to a derived type formal argument must
have the _data component deallocated after use.

2017-07-27 Paul Thomas <***@gcc.gnu.org>

PR fortran/80477
* gfortran.dg/class_result_7.f90: New test.

[Janus Weil]

Hi Paul,

Post by Paul Richard Thomas
Several attempts, including mine, were made to fix this bug since it
was posted. They were all attacking the wrong place. Instead of
providing the free of the class _data as part of the call to
'add_a_type' it should be included in the post block of the argument
processing in the call to 'assign_a_type'. The comment in the patch
says the rest.
Bootstrapped and regtested on FC28/x86_64 - OK for trunk?

great that you managed to solve this one! The patch looks very good to
me, but I'm afraid two details may be missing:

1) If the type has allocatable components, those need to be freed first.
2) If the type has a finalizer, that needs to be called as well.

I believe that both points can be fixed by calling the _final
component of the vtab before freeing the class data. Should not be
hard to add, I hope (gfc_add_finalizer_call might be useful).

Thanks for your efforts ...

Cheers,
Janus

[Paul Richard Thomas]

Hi Janus,

Yes, you are correct about the need to call the _final component. OK -
will do :-)

PR86481? This seems to be the same problem in a different place.

I am starting to wonder if the gfc_se structure doesn't need an
additional block component; se.final, in addition to se.post. This
kind of difficulty in sequencing has come up many times before. If the
deallocations were available in the final block, this could be added
to the post block of the caller. This would be a neat solution to the
cases where finalization should be implemented but is not....

Thanks

Paul

Post by Janus Weil
Hi Paul,

Post by Paul Richard Thomas
Several attempts, including mine, were made to fix this bug since it
was posted. They were all attacking the wrong place. Instead of
providing the free of the class _data as part of the call to
'add_a_type' it should be included in the post block of the argument
processing in the call to 'assign_a_type'. The comment in the patch
says the rest.
Bootstrapped and regtested on FC28/x86_64 - OK for trunk?

great that you managed to solve this one! The patch looks very good to
1) If the type has allocatable components, those need to be freed first.
2) If the type has a finalizer, that needs to be called as well.
I believe that both points can be fixed by calling the _final
component of the vtab before freeing the class data. Should not be
hard to add, I hope (gfc_add_finalizer_call might be useful).
Thanks for your efforts ...
Cheers,
Janus

[Janus Weil]

Hi Paul,

Post by Paul Richard Thomas
Yes, you are correct about the need to call the _final component. OK -
will do :-)

Thanks. If it turns out to be more work than anticipated, you can
certainly go ahead and commit your patch as is, before going any
further. But I certainly hope it will be a cheap addition.

Post by Paul Richard Thomas
PR86481? This seems to be the same problem in a different place.

PR 65347 is related as well.

Post by Paul Richard Thomas
I am starting to wonder if the gfc_se structure doesn't need an
additional block component; se.final, in addition to se.post. This
kind of difficulty in sequencing has come up many times before. If the
deallocations were available in the final block, this could be added
to the post block of the caller. This would be a neat solution to the
cases where finalization should be implemented but is not....

I feel like I don't have enough knowledge of this structure and its
usage to comment on this. I never really managed to get comfy with
gfc_se, gfc_ss & co :(

Cheers,
Janus

Post by Paul Richard Thomas

Post by Janus Weil
Hi Paul,

Post by Paul Richard Thomas
Several attempts, including mine, were made to fix this bug since it
was posted. They were all attacking the wrong place. Instead of
providing the free of the class _data as part of the call to
'add_a_type' it should be included in the post block of the argument
processing in the call to 'assign_a_type'. The comment in the patch
says the rest.
Bootstrapped and regtested on FC28/x86_64 - OK for trunk?

great that you managed to solve this one! The patch looks very good to
1) If the type has allocatable components, those need to be freed first.
2) If the type has a finalizer, that needs to be called as well.
I believe that both points can be fixed by calling the _final
component of the vtab before freeing the class data. Should not be
hard to add, I hope (gfc_add_finalizer_call might be useful).
Thanks for your efforts ...
Cheers,
Janus

--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

[Janus Weil]

Hi Paul,

Post by Janus Weil

Post by Paul Richard Thomas
Yes, you are correct about the need to call the _final component. OK -
will do :-)

Thanks. If it turns out to be more work than anticipated, you can
certainly go ahead and commit your patch as is, before going any
further. But I certainly hope it will be a cheap addition.

it seems you have not committed this patch yet? Please do! It does fix
a serious problem. The finalization stuff can be taken care of later
...

Cheers,
Janus

[Paul Richard Thomas]

Hi Janus,

I have suffered a temporary outage due to daytime work becoming
nightime too. I was investigating this bug a bit further. It seems to
me that the namespace of the procedures is not being resolved,
otherwise resolve_types would have been called. I could commit the
patch tonight and work some more on the PR afterwards.

Cheers

Paul

Post by Janus Weil
Hi Paul,

Post by Janus Weil

Post by Paul Richard Thomas
Yes, you are correct about the need to call the _final component. OK -
will do :-)

Thanks. If it turns out to be more work than anticipated, you can
certainly go ahead and commit your patch as is, before going any
further. But I certainly hope it will be a cheap addition.

it seems you have not committed this patch yet? Please do! It does fix
a serious problem. The finalization stuff can be taken care of later
...
Cheers,
Janus

[Dominique d'Humières]

Hi!

Post by Janus Weil
great that you managed to solve this one! The patch looks very good to
1) If the type has allocatable components, those need to be freed first.
…

PR86481?

Cheers

Dominique