| Message ID | 5265611B.7030906@net-b.de |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 21, 2013 at 07:15:07PM +0200, Tobias Burnus wrote: > - c->tb = tb; > + if (num == 1) > + c->tb = tb; > + else > + { > + c->tb = XCNEW (gfc_typebound_proc); > + c->tb->where = gfc_current_locus; > + *c->tb = *tb; I haven't convinced myself yet, but is this leaking memory? Lines 5006-7 are tb = XCNEW (gfc_typebound_proc); tb->where = gfc_current_locus; In the num = 1 case, c->tb = tb and I presume during teardown c->tb is freed and by extension tb. But, in the else case, are you copying the contents from *tb into *c->tb and the tb is never freed?
Steve Kargl wrote: > On Mon, Oct 21, 2013 at 07:15:07PM +0200, Tobias Burnus wrote: >> - c->tb = tb; >> + if (num == 1) >> + c->tb = tb; >> + else >> + { >> + c->tb = XCNEW (gfc_typebound_proc); >> + c->tb->where = gfc_current_locus; >> + *c->tb = *tb; > I haven't convinced myself yet, but is this leaking memory? > > Lines 5006-7 are > > tb = XCNEW (gfc_typebound_proc); > tb->where = gfc_current_locus; > > In the num = 1 case, c->tb = tb and I presume during teardown > c->tb is freed and by extension tb. But, in the else case, > are you copying the contents from *tb into *c->tb and the tb > is never freed? Well, as the double-free PR shows, free_components frees the memory: "free (p->tb);". And as it walks all components … Tobias
On Mon, Oct 21, 2013 at 09:04:22PM +0200, Tobias Burnus wrote: > Steve Kargl wrote: > > On Mon, Oct 21, 2013 at 07:15:07PM +0200, Tobias Burnus wrote: > >> - c->tb = tb; > >> + if (num == 1) > >> + c->tb = tb; > >> + else > >> + { > >> + c->tb = XCNEW (gfc_typebound_proc); (see below) > >> + c->tb->where = gfc_current_locus; > >> + *c->tb = *tb; > > I haven't convinced myself yet, but is this leaking memory? > > > > Lines 5006-7 are > > > > tb = XCNEW (gfc_typebound_proc); > > tb->where = gfc_current_locus; > > > > In the num = 1 case, c->tb = tb and I presume during teardown > > c->tb is freed and by extension tb. But, in the else case, > > are you copying the contents from *tb into *c->tb and the tb > > is never freed? > > Well, as the double-free PR shows, free_components frees the memory: > "free (p->tb);". And as it walks all components ? Argh. I misread the code. Instead of 'c->tb = XCNEW(...' above, I "saw" 'tb = XCNEW(...'. So, I thought you might be leaking a local copy of tb. With that misunderstanding cleared up, the patch looks fine.
2013-10-21 Tobias Burnus <burnus@net-b.de> PR fortran/58803 * decl.c (match_ppc_decl): Prevent later double free. 2013-10-21 Tobias Burnus <burnus@net-b.de> PR fortran/58803 * gfortran.dg/proc_ptr_comp_38.f90: New. diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c index 3a8175f..9c9fd4f 100644 --- a/gcc/fortran/decl.c +++ b/gcc/fortran/decl.c @@ -5055,7 +5055,14 @@ match_ppc_decl (void) if (!gfc_add_proc (&c->attr, name, NULL)) return MATCH_ERROR; - c->tb = tb; + if (num == 1) + c->tb = tb; + else + { + c->tb = XCNEW (gfc_typebound_proc); + c->tb->where = gfc_current_locus; + *c->tb = *tb; + } /* Set interface. */ if (proc_if != NULL) diff --git a/gcc/testsuite/gfortran.dg/proc_ptr_comp_38.f90 b/gcc/testsuite/gfortran.dg/proc_ptr_comp_38.f90 new file mode 100644 index 0000000..2a71ca0 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/proc_ptr_comp_38.f90 @@ -0,0 +1,12 @@ +! { dg-do compile } +! +! PR fortran/58803 +! +! Contributed by Vittorio Zecca +! +! Was before ICEing due to a double free +! + type t + procedure(real), pointer, nopass :: f1, f2 + end type + end
As written in the PR: The problem is that in free_components one frees: 2054 free_components (gfc_component *p) ... 2058 for (; p; p = q) 2059 { 2060 q = p->next; 2061 2062 gfc_free_array_spec (p->as); 2063 gfc_free_expr (p->initializer); 2064 free (p->tb); 2065 2066 free (p); 2067 } Here: p->name == "f1" p->tb == (gfc_typebound_proc *) 0x17e0070 when one then cycles to p = q (i.e. to p->next), one has: p->name == "f2" p->tb == (gfc_typebound_proc *) 0x17e0070 The patch is rather straight forward. Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias