On 06/05/24, ltaylorsimp...@gmail.com wrote:
> 
> 
> > -----Original Message-----
> > From: Anton Johansson <a...@rev.ng>
> > Sent: Monday, May 6, 2024 1:31 PM
> > To: qemu-devel@nongnu.org
> > Cc: a...@rev.ng; ltaylorsimp...@gmail.com; bc...@quicinc.com
> > Subject: [PATCH 3/4] target/hexagon: idef-parser fix leak of init_list
> > 
> > gen_inst_init_args() is called for instructions using a predicate as an
> rvalue.
> > Upon first call, the list of arguments which might need initialization
> init_list is
> > freed to indicate that they have been processed. For instructions without
> an
> > rvalue predicate,
> > gen_inst_init_args() isn't called and init_list will never be freed.
> > 
> > Free init_list from free_instruction() if it hasn't already been freed.
> > 
> > Signed-off-by: Anton Johansson <a...@rev.ng>
> > ---
> >  target/hexagon/idef-parser/parser-helpers.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/target/hexagon/idef-parser/parser-helpers.c
> > b/target/hexagon/idef-parser/parser-helpers.c
> > index 95f2b43076..bae01c2bb8 100644
> > --- a/target/hexagon/idef-parser/parser-helpers.c
> > +++ b/target/hexagon/idef-parser/parser-helpers.c
> > @@ -2121,6 +2121,13 @@ void free_instruction(Context *c)
> >          g_string_free(g_array_index(c->inst.strings, GString*, i), TRUE);
> >      }
> >      g_array_free(c->inst.strings, TRUE);
> > +    /*
> > +     * Free list of arguments that might need initialization, if they
> haven't
> > +     * already been free'd.
> > +     */
> > +    if (c->inst.init_list) {
> > +        g_array_free(c->inst.init_list, TRUE);
> > +    }
> >      /* Free INAME token value */
> >      g_string_free(c->inst.name, TRUE);
> >      /* Free variables and registers */
> 
> Why not do this in gen_inst_init_args just before this?
>    /* Free argument init list once we have initialized everything */
>     g_array_free(c->inst.init_list, TRUE);
>     c->inst.init_list = NULL;

Thanks for the reviews Taylor! I'm not sure I understand what you mean
here, we already free init_list in gen_inst_init_args, since
gen_inst_init_args won't be called for all instructions we need to also
free from a common place.

//Anton

Reply via email to