Re: [Fortran, Patch, PR119349, v1] Fix regression of polymorphic dummy sourced from array constructors.
Hi Jerry,
thanks for the review and the kind words. Committed as gcc-15-8481-g0f344846a62
Thanks again,
Andre
On Thu, 20 Mar 2025 11:42:35 -0700
Jerry D wrote:
> On 3/20/25 9:20 AM, Andre Vehreschild wrote:
> > Hi all,
> >
> > attached patch fixes a 15-regression where an element of an actual
> > temporary array, i.e., elemental([ e1, e2...]) passed to the formal
> > polymorphic dummy leads to a double free of the derived types components.
> > This patch prevents this by preventing the deallocation of the array
> > constructors temporary, when the formal is polymorphic. ...
> >
> > Folks its so hard to explain this in prose. I rewrote above paragraph the
> > third time now. And I still don't understand on re-reading. So here is some
> > pseudo code:
> >
> > struct derived {
> >char *c; // This is the component suffering from double-free
> > };
> >
> > derived[2] atmp = [ derived(""), derived("")]
> >
> > forall a in atmp
> >derived t_a = a; // <- Copy of a, but no deep copy, i.e. t_a.c == a.c
> >class_temp = class_derived(a); // set _vtype left out for brevity
> >call elemental_function(class_temp);
> >if (class_temp._data.c != NULL)
> > free(class_temp._data.c); // and set it to NULL
> >if (t_a.c != NULL)
> > free(t_a.c); // BOOM, this is freeing the same c
> > end
> >
> > Generating the last if-block and the free is what this patch prevents for
> > polymorphic dummys that stem from an array construction. And only for those.
> >
> > Sorry, I am having a hard time explaining things today. So I hope the code
> > above will do.
> >
> > Regtested ok on x86_64-pc-linux-gnu / F41. Ok for mainline?
>
> OK and thanks for the fix. Your prose is fine and your comment in the
> code suffices.
>
> Thanks for the patch.
>
> Jerry
> >
> > Regards,
> > Andre
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
>
--
Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Fortran, Patch, PR119380, v1] Fix freeing procedure pointers in components
Hi Paul, well, I had those might complicated patches bit my mightily. So let's hope for the best :-) Thanks for the review. Committed with your proposed change in the testcase as gcc-15-8642-ga5c69abf138 Thanks again, Andre On Fri, 21 Mar 2025 10:40:11 + Paul Richard Thomas wrote: > Hi Andre, > > Gosh, that's a mighty complicated patch :-) I suggest changing the comment > in the test case: > > s/Check that components of procedure pointer aren't freeed./Do not free > procedure pointer components/ or some such. > > OK for mainline and, I propose, 14-branch. > > Regards and thanks > > Paul > > > On Fri, 21 Mar 2025 at 09:38, Andre Vehreschild wrote: > > > Hi all, > > > > attached patch fixes freeing of procedure pointers that are stored in a > > derived > > type's component. GFortran did that already for polymorphic types but > > missed > > out on the others. > > > > Regtested ok on x86_64-pc-linux-gnu / F41. Ok for mainline? > > > > Regards, > > Andre > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > > -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Fortran, Patch, PR119380, v1] Fix freeing procedure pointers in components
Hi Andre, Gosh, that's a mighty complicated patch :-) I suggest changing the comment in the test case: s/Check that components of procedure pointer aren't freeed./Do not free procedure pointer components/ or some such. OK for mainline and, I propose, 14-branch. Regards and thanks Paul On Fri, 21 Mar 2025 at 09:38, Andre Vehreschild wrote: > Hi all, > > attached patch fixes freeing of procedure pointers that are stored in a > derived > type's component. GFortran did that already for polymorphic types but > missed > out on the others. > > Regtested ok on x86_64-pc-linux-gnu / F41. Ok for mainline? > > Regards, > Andre > -- > Andre Vehreschild * Email: vehre ad gmx dot de >
[Fortran, Patch, PR119380, v1] Fix freeing procedure pointers in components
Hi all,
attached patch fixes freeing of procedure pointers that are stored in a derived
type's component. GFortran did that already for polymorphic types but missed
out on the others.
Regtested ok on x86_64-pc-linux-gnu / F41. Ok for mainline?
Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
From 9a77974f8120564846f672f28650100d158f365d Mon Sep 17 00:00:00 2001
From: Andre Vehreschild
Date: Fri, 21 Mar 2025 09:13:29 +0100
Subject: [PATCH] Fortran: Fix freeing procedure pointer components [PR119380]
PR fortran/119380
gcc/fortran/ChangeLog:
* trans-array.cc (structure_alloc_comps): Prevent freeing of
procedure pointer components.
gcc/testsuite/ChangeLog:
* gfortran.dg/proc_ptr_comp_54.f90: New test.
---
gcc/fortran/trans-array.cc| 2 +-
.../gfortran.dg/proc_ptr_comp_54.f90 | 30 +++
2 files changed, 31 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gfortran.dg/proc_ptr_comp_54.f90
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index e9eacf20128..960613167f7 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -10109,7 +10109,7 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl, tree dest,
else
{
attr = &c->attr;
- if (attr->pointer)
+ if (attr->pointer || attr->proc_pointer)
continue;
}
diff --git a/gcc/testsuite/gfortran.dg/proc_ptr_comp_54.f90 b/gcc/testsuite/gfortran.dg/proc_ptr_comp_54.f90
new file mode 100644
index 000..73abc590e9e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/proc_ptr_comp_54.f90
@@ -0,0 +1,30 @@
+!{ dg-do run }
+
+! Check that components of procedure pointer aren't freeed.
+! Contributed by Damian Rouson
+
+ implicit none
+
+ type foo_t
+integer, allocatable :: i_
+procedure(f), pointer, nopass :: f_
+procedure(c), pointer, nopass :: c_
+ end type
+
+ class(foo_t), allocatable :: ff
+
+ associate(foo => foo_t(1,f))
+ end associate
+
+contains
+
+ function f()
+logical, allocatable :: f
+f = .true.
+ end function
+
+ function c()
+class(foo_t), allocatable :: c
+allocate(c)
+ end function
+end
--
2.49.0
Re: [Fortran, Patch, PR119349, v1] Fix regression of polymorphic dummy sourced from array constructors.
Hi Paul,
thanks for the (additional) review. The patch has been merged already as
gcc-15-8481-g0f344846a62.
But I totally agree, that conv_procedure_call is calling (pun intended) for a
refactoring.
Thanks again,
Andre
On Fri, 21 Mar 2025 14:34:13 +
Paul Richard Thomas wrote:
> Hi Andre,
>
> I am reasonably familiar with the mess that is gfc_conv_procedure_call :-)
> So in spite of you having a hard time explaining things today, I see your
> patch as verging on 'obvious' and is certainly the best that can be done
> without refactoring the whole thing.
>
> OK fo mainline.
>
> Thanks for the patch
>
> Paul
>
>
> On Thu, 20 Mar 2025 at 16:36, Andre Vehreschild wrote:
>
> > Hi all,
> >
> > attached patch fixes a 15-regression where an element of an actual
> > temporary array, i.e., elemental([ e1, e2...]) passed to the formal
> > polymorphic
> > dummy leads to a double free of the derived types components. This patch
> > prevents this by preventing the deallocation of the array constructors
> > temporary, when the formal is polymorphic. ...
> >
> > Folks its so hard to explain this in prose. I rewrote above paragraph the
> > third
> > time now. And I still don't understand on re-reading. So here is some
> > pseudo
> > code:
> >
> > struct derived {
> > char *c; // This is the component suffering from double-free
> > };
> >
> > derived[2] atmp = [ derived(""), derived("")]
> >
> > forall a in atmp
> > derived t_a = a; // <- Copy of a, but no deep copy, i.e. t_a.c == a.c
> > class_temp = class_derived(a); // set _vtype left out for brevity
> > call elemental_function(class_temp);
> > if (class_temp._data.c != NULL)
> > free(class_temp._data.c); // and set it to NULL
> > if (t_a.c != NULL)
> > free(t_a.c); // BOOM, this is freeing the same c
> > end
> >
> > Generating the last if-block and the free is what this patch prevents for
> > polymorphic dummys that stem from an array construction. And only for
> > those.
> >
> > Sorry, I am having a hard time explaining things today. So I hope the code
> > above will do.
> >
> > Regtested ok on x86_64-pc-linux-gnu / F41. Ok for mainline?
> >
> > Regards,
> > Andre
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
> >
--
Andre Vehreschild * Email: vehre ad gmx dot de
