[Bug driver/93019] memory leak in gcc -O2 reported by Valgrind

[Bug driver/93019] memory leak in gcc -O2 reported by Valgrind

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93019

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |eliaz.pitavy at obspm dot fr

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 110063 has been marked as a duplicate of this bug. ***

[Bug driver/93019] memory leak in gcc -O2 reported by Valgrind

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93019

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-05-31
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
           Keywords|                            |memory-hog

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed via the dup.

[Bug driver/93019] memory leak in gcc -O2 reported by Valgrind

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93019

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #1)
> I think this is known for the driver and we're too lazy to fix

For the driver, it is not much a deal but since JIT uses the driver code also,
it becomes an issue.

[Bug driver/93019] memory leak in gcc -O2 reported by Valgrind

[costas.argyris at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93019

Costas Argyris <costas.argyris at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |costas.argyris at gmail dot com

--- Comment #5 from Costas Argyris <costas.argyris at gmail dot com> ---
Created attachment 55464
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55464&action=edit
Valgrind memcheck report as of 2 July 2023

The original valgrind report is now 3.5 years old.    I ran the exact same case
with the latest source code (as of yesterday) and attach the report as a file
because it is actually too large to fit in a comment (there are 95 loss records
now, from the 69 that are seen in the original report).

[Bug driver/93019] memory leak in gcc -O2 reported by Valgrind

[costas.argyris at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93019

--- Comment #6 from Costas Argyris <costas.argyris at gmail dot com> ---
Part of this may be because the driver::finalize function introduced here:

https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9376dd63e6a2d94823f6faf8212c9f37bef5a656

is not called from main:

 int
 main (int argc, char **argv)
 {
-  driver d;
+  driver d (false, /* can_finalize */
+           false); /* debug */

   return d.main (argc, argv);
 }

It only gets called from the jit code it was designed to serve:

+void
+playback::context::
+invoke_embedded_driver (const vec <char *> *argvec)
+{
+  JIT_LOG_SCOPE (get_logger ());
+  driver d (true, /* can_finalize */
+           false); /* debug */
+  int result = d.main (argvec->length (),
+                      const_cast <char **> (argvec->address ()));
+  d.finalize ();
+  if (result)
+    add_error (NULL, "error invoking gcc driver");
+}

What is confusing to me though is that the can_finalize argument to the driver
constructor really seems to be referring only to the environment manager
env_manager component, not the entire driver.    driver::finalize does a lot
more than just call env.restore (), including freeing allocated memory.

I don't see why the inability to call env_manager::restore () should block
anyone from calling driver::finalize (), as the latter starts from env.restore
() but does a lot more later.    In other words, what does environment variable
management have to do with memory management and why is it allowed to block it?
   Can't these two be done independently?

Simply adding a call to driver::finalize in main:

 int
 main (int argc, char **argv)
 {
-  driver d (false, /* can_finalize */
+  driver d (true, /* can_finalize */
            false); /* debug */

-  return d.main (argc, argv);
+  int result = d.main (argc, argv);
+  d.finalize ();
+  return result;
 }


decreases the number of valgrind loss records from 95 to 62.

If can_finalize must stay false in main for whatever reason, then
driver::finalize could simply check if it can call env.restore such that it is
always possible to call driver::finalize regardless of what was passed for
can_finalize, and leave only env_manager::restore depend on that argument, as
it seems to be the only thing that is really relying on it (that would only
require an extra public method on env_manager to get the can_finalize value
that was passed - it is called m_can_restore in the class).

[Bug driver/93019] memory leak in gcc -O2 reported by Valgrind

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93019

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:36be2a0e91c76da4afcd5ddc37e03f5800396387

commit r14-6356-g36be2a0e91c76da4afcd5ddc37e03f5800396387
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Dec 9 10:28:37 2023 +0100

    driver: Fix memory leak [PR93019]

    driver:finalize used by JIT clears the mdswitches pointer; if it was
    allocated before, that leaks the memory.

    2023-12-09  Costas Argyris  <costas.argyris@gmail.com>
                Jakub Jelinek  <jakub@redhat.com>

            PR driver/93019
            * gcc.cc (driver::finalize): Call XDELETEVEC on mdswitches before
            clearing it.

    Signed-off-by: Costas Argyris <costas.argyris@gmail.com>