Re: RFA: fix PR 9350

[Tom Tromey <tromey@redhat.com>]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> This is actually something that I learnt only relatively recently,
Joel> maybe a year or two ago: If you put something on the cleanup queue,
Joel> you should perform the cleanup when you're done, or you run
Joel> the risk of having a memory leak.

Yeah.  Yesterday I was contemplating writing a gcc plugin to detect
this error.  But back to reality...

Joel> Do we have a different scenario in your example that causes
Joel> a memory leak?

Yeah, this patch reveals a number of leak styles.

In syms_from_objfile, we installed a cleanup but then discarded it.
This is another cleanup oddity -- because they are handled linearly,
code must either be careful to create them in the right order so that
a sequence of discard_ and do_ calls can be run at the end; or the
code must duplicate the action.

In update_global_location_list, we simply were not installing any
cleanup for the local VEC.

In varobj_invalidate, the freeing was only done on one branch of an
`if', though the condition of the `if' unconditionally allocated
memory.

>> -      do_cleanups (ui_out_chain);
>> +      do_cleanups (old_chain);

Joel> Ooops, does it look like you're using uiout after it has been
Joel> deleted?  (I have seen the same issue a few more time later
Joel> in your patch)

No, it just looks that way because ui_out_chain had a funny name.

The old code looked like:

-      ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "value");

So ui_out_chain was just used for finalizing a tuple.
`uiout' itself is still valid; the additional cleanup we run (via
`old_chain') is to finalize `stb'.

Joel> Perhaps this function would benefit from having only one place
Joel> where the result is returned, thus requiring only one call to
Joel> do_cleanups? At first sight, it seems relatively easy to achieve
Joel> in this case. That's an open question - I'm fine with just fixing
Joel> the above by moving the do_cleanups to just before the return.

I only did it this way because it was the prevailing style in the
function.  I will switch it and retest and resubmit.

Tom