Thanks for all the explanations. In that case I restrict this patch to just freeing the buffer from within driver::finalize only (I think it should be XDELETEVEC instead of XDELETE, no?).
On Thu, 7 Dec 2023 at 15:42, Jakub Jelinek <ja...@redhat.com> wrote: > On Thu, Dec 07, 2023 at 03:16:29PM +0000, Costas Argyris wrote: > > > Still reachable memory at exit e.g. from valgrind is not a bug. > > > > Indeed, this is coming from a valgrind report here: > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93019 > > > > where it was noted that the driver memory leaks could be > > problematic for JIT. > > In the invoke_embedded_driver JIT case yes, that calls driver::finalize (), > which is why it should be freed before clearing the pointer in there (as > then it is a real leak). > > > So, since using std::vector did reduce the valgrind records > > by one (I only targeted a single variable to begin with) I took > > that as a good sign. > > > > Regarding adding a call to XDELETE (mdswitches), yes, > > that would help in the case where driver::finalize () is actually > > called, which I think is for JIT. I was trying to take care of the > > case where it doesn't get called as well, but from what you say > > I take it that this case is not of interest. > > That is wasted compile time on a non-issue. > > If you see a JIT issue with definitely lost records, that is something > that obviously should be fixed (but even in that area I think we've been a > little bit lazy in the option handling). > The most important is that the actual compiler binaries (cc1, cc1plus, ...) > don't leak memory (in the definitely lost kind) like crazy, we have > --enable-checking=valgrind > for that purpose, where the driver runs cc1/cc1plus etc. under valgrind, > but this is very expensive and slow, so usually it is run once during a > cycle (if at all), on a fast machine could take even in non-bootstrap mode > a weekend to go through the whole testsuite, then one can look at the > leaks. > > Jakub > >
0001-driver-Fix-memory-leak-in-driver-finalize.patch
Description: Binary data
