Updating the subject since this is really just a driver change (and
therefore needs a review from those who can approve patches there).

Thanks,
Alex

-----Original Message-----
From: Jit <jit-bounces@gcc.gnu.org> On Behalf Of Alex Coplan
Sent: 09 July 2020 21:13
To: gcc-patches@gcc.gnu.org; jit@gcc.gnu.org
Cc: nd <nd@arm.com>
Subject: [PATCH] libgccjit: Fix several memory leaks in the driver

Hello,

This patch fixes several memory leaks in the driver, all of which relate
to the handling of static specs. We introduce functions
set_static_spec_{shared,owned}() which are used to enforce proper memory
management when updating the strings in the static_specs table.

This is achieved by making use of the alloc_p field in the table
entries. Similarly to set_spec(), each time we update an entry, we check
whether alloc_p is set, and free the old value if so. We then set
alloc_p correctly based on whether we "own" this memory or whether we're
just taking a pointer to a shared string which we shouldn't free.

The following table shows the number of leaks found by AddressSanitizer
when running a minimal libgccjit program on AArch64. The test program
does the whole libgccjit compilation cycle in a loop (including acquiring
and releasing the context), and the table below shows the number of leaks
for different iterations of that loop.

+--------------+-----+-----+------+---------------+
| # of runs >  | 1   | 2   | 3    | Leaks per run |
+--------------+-----+-----+------+---------------+
| Before patch | 463 | 940 | 1417 | 477           |
+--------------+-----+-----+------+---------------+
| After patch  | 416 | 846 | 1276 | 430           |
+--------------+-----+-----+------+---------------+

Ensuring that we minimize "leaks per run" (ultimately eliminating all of
them) is important in order for long-running applications to be able to
make use of in-process libgccjit.

Testing:
 * Bootstrap and regtest on aarch64-linxu-gnu, x86_64-linux-gnu.
 * Bootstrap and regtest on aarch64-linux-gnu with bootstrap-asan config.
 * Smoke test of libgccjit, ran regressions on a --disable-bootstrap build on
   aarch64-linux-gnu.

OK for master?

Thanks,
Alex

---

gcc/ChangeLog:

2020-07-09  Alex Coplan  <alex.coplan@arm.com>

	* gcc.c (set_static_spec): New.
	(set_static_spec_owned): New.
	(set_static_spec_shared): New.
	(driver::maybe_putenv_COLLECT_LTO_WRAPPER): Use
	set_static_spec_owned() to take ownership of lto_wrapper_file
	such that it gets freed in driver::finalize.
	(driver::maybe_run_linker): Use set_static_spec_shared() to
	ensure that we don't try and free() the static string "ld",
	also ensuring that any previously-allocated string in
	linker_name_spec is freed. Likewise with argv0.
	(driver::finalize): Use set_static_spec_shared() when resetting
	specs that previously had allocated strings; remove if(0)
	around call to free().