Re: [Patch, fortran] PR104272 - finalizer gets called during allocate
Hi Paul, On 4/5/23 08:53, Paul Richard Thomas via Gcc-patches wrote: Hi All, This is a first in my recent experience - a very old bug that produces too many finalizations! It results from a bit of a fix up, where class objects are allocated using the derived typespec, rather than a source or mold expression. This occurs upstream in resolve.cc, where the default initializer is assigned to expr3 but no means are provided to identify what it is. The patch applies a signaling bit-field to the ext field of gfc_code, which then suppresses the deallocation of allocatable components in the allocate expression. I have checked that this does not cause memory leaks, even though the number of builtin_frees in class_result_8.f90 goes down by one. can you have a look again at the logic in the hunk touching trans-stmt.cc (gfc_trans_allocate)? I haven't checked in detail, but it seems possible that you get a stale tmp in the gfc_prepend_expr_to_block if (code->ext.alloc.expr3_not_explicit == 0). Wouldn't it make more sense to move this condition before the braces as part of the overall condition? OK for mainline? Otherwise this LGTM. Thanks for the patch! Harald Paul Fortran: Fix and excess finalization during allocation [PR104272] 2023-04-04 Paul Thomas gcc/fortran PR fortran/104272 * gfortran.h : Add expr3_not_explicit bit field to gfc_code. * resolve.cc (resolve_allocate_expr): Set bit field when the default initializer is applied to expr3. * trans-stmt.cc (gfc_trans_allocate): If expr3_not_explicit is set, do not deallocate expr3. gcc/testsuite/ PR fortran/104272 * gfortran.dg/class_result_8.f90 : Number of builtin_frees down from 6 to 5 without memory leaks. * gfortran.dg/finalize_52.f90: New test
Re: [Patch, fortran] PR104272 - finalizer gets called during allocate
On 4/5/23 20:50, Harald Anlauf via Gcc-patches wrote: can you have a look again at the logic in the hunk touching trans-stmt.cc (gfc_trans_allocate)? I haven't checked in detail, but it seems possible that you get a stale tmp in the gfc_prepend_expr_to_block if (code->ext.alloc.expr3_not_explicit == 0). Oops, I meant if (code->ext.alloc.expr3_not_explicit != 0) Wouldn't it make more sense to move this condition before the braces as part of the overall condition? OK for mainline? Otherwise this LGTM. Thanks for the patch! Harald
Re: [Patch, fortran] PR104272 - finalizer gets called during allocate
Hi Harald, Quite right - good spot. There was an 'else' that turned out to be unnecessary. Thanks Paul On Wed, 5 Apr 2023 at 19:50, Harald Anlauf wrote: > Hi Paul, > > On 4/5/23 08:53, Paul Richard Thomas via Gcc-patches wrote: > > Hi All, > > > > This is a first in my recent experience - a very old bug that produces > too > > many finalizations! It results from a bit of a fix up, where class > objects > > are allocated using the derived typespec, rather than a source or mold > > expression. This occurs upstream in resolve.cc, where the default > > initializer is assigned to expr3 but no means are provided to identify > what > > it is. The patch applies a signaling bit-field to the ext field of > > gfc_code, which then suppresses the deallocation of allocatable > components > > in the allocate expression. I have checked that this does not cause > memory > > leaks, even though the number of builtin_frees in class_result_8.f90 goes > > down by one. > > can you have a look again at the logic in the hunk touching > trans-stmt.cc (gfc_trans_allocate)? I haven't checked in detail, > but it seems possible that you get a stale tmp in the > gfc_prepend_expr_to_block if (code->ext.alloc.expr3_not_explicit == 0). > Wouldn't it make more sense to move this condition before the braces > as part of the overall condition? > > > OK for mainline? > > Otherwise this LGTM. > > Thanks for the patch! > > Harald > > > Paul > > > > Fortran: Fix and excess finalization during allocation [PR104272] > > > > 2023-04-04 Paul Thomas > > > > gcc/fortran > > PR fortran/104272 > > * gfortran.h : Add expr3_not_explicit bit field to gfc_code. > > * resolve.cc (resolve_allocate_expr): Set bit field when the > > default initializer is applied to expr3. > > * trans-stmt.cc (gfc_trans_allocate): If expr3_not_explicit is > > set, do not deallocate expr3. > > > > gcc/testsuite/ > > PR fortran/104272 > > * gfortran.dg/class_result_8.f90 : Number of builtin_frees down > > from 6 to 5 without memory leaks. > > * gfortran.dg/finalize_52.f90: New test > > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
