On Wed, Feb 3, 2021 at 1:15 AM Martin Sebor <mse...@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. > > > > >