[PATCH] release pointer_query cache when done (PR 98937)

[Richard Biener]

On Wed, Feb 3, 2021 at 1:15 AM Martin Sebor <msebor@gmail.com> wrote:
>
> On 2/2/21 2:29 PM, David Malcolm wrote:
> > On Tue, 2021-02-02 at 12:57 -0700, Martin Sebor via Gcc-patches wrote:
> >> The strlen pass initializes its pointer_query member object with
> >> a cache consisting of a couple of vec's.  Because vec doesn't
> >> implement RAII its memory must be explicitly released to avoid
> >> memory leaks.  The attached patch adds a dtor to
> >> the strlen_dom_walker to do that.
> >>
> >> Tested on x86_64-linux and by verifying that the cache leaks are
> >> gone by compiling gcc.dg/Wstringop-overflow*.c tests under Valgrind.
> >>
> >> I'll plan to commit this change as "obvious" tomorrow unless there
> >> are suggestions for changes.
> >
> > Why not make the vecs within struct pointer_query::cache_type's be
> > auto_vecs?  Then presumably the autogenerated dtor for
> > pointer_query::cache_type would clean things up, as called from the
> > autogenerated dtor for strlen_dom_walker, when cleaning up the
> > var_cache field?
> >
> > Or am I missing something?  (sorry, I'm not familiar with this code)
> > Dave
>
> It would work as long as the cache isn't copied or assigned anywhere.
> I don't think it is either, and GCC compiles and no C tests fail, so
> it should be okay.
>
> But I'm leery of using auto_vec as a class member because of pr90904.
> Looks like auto_vec now has a move ctor and move assignment but still
> no safe copy ctor or assignment.  The cache copy ctor and assignment
> operator could be deleted to avoid accidental copies, but at that
> point it starts to become more involved than just flushing the cache.
>
> If you or someone else has a preference for using auto_vec despite
> this I'll change it.  Otherwise I'd just as soon leave it as is.

I prefer the original patch at this point.

Richard.

> Martin
>
> >
> >> Martin
> >>
> >> PS Valgrind shows a fair number of leaks even with the patch but
> >> none of them due to the pointer_query cache.
> >
> >
>