OpenE2K/gcc
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

driver: Fix several memory leaks [PR63854]

Browse Source 
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           |
+--------------+-----+-----+------+---------------+

gcc/ChangeLog:

	PR jit/63854
	* 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().
This commit is contained in:
Alex Coplan 2020-08-21 14:26:11 +01:00
parent e7d55c6b81
commit b46584d783
1 changed files with 52 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
gcc/gcc.c
View File

@ -1908,6 +1908,51 @@ init_spec (void)
specs = sl;
}
/* Update the entry for SPEC in the static_specs table to point to VALUE,
ensuring that we free the previous value if necessary. Set alloc_p for the
entry to ALLOC_P: this determines whether we take ownership of VALUE (i.e.
whether we need to free it later on). */
static void
set_static_spec (const char **spec, const char *value, bool alloc_p)
{
struct spec_list *sl = NULL;
for (unsigned i = 0; i < ARRAY_SIZE (static_specs); i++)
{
if (static_specs[i].ptr_spec == spec)
{
sl = static_specs + i;
break;
}
}
gcc_assert (sl);
if (sl->alloc_p)
{
const char *old = *spec;
free (const_cast <char *> (old));
}
*spec = value;
sl->alloc_p = alloc_p;
}
/* Update a static spec to a new string, taking ownership of that
string's memory. */
static void set_static_spec_owned (const char **spec, const char *val)
{
return set_static_spec (spec, val, true);
}
/* Update a static spec to point to a new value, but don't take
ownership of (i.e. don't free) that string. */
static void set_static_spec_shared (const char **spec, const char *val)
{
return set_static_spec (spec, val, false);
}
/* Change the value of spec NAME to SPEC. If SPEC is empty, then the spec is
removed; If the spec starts with a + then SPEC is added to the end of the
@ -8344,7 +8389,7 @@ driver::maybe_putenv_COLLECT_LTO_WRAPPER () const
if (lto_wrapper_file)
{
lto_wrapper_file = convert_white_space (lto_wrapper_file);
lto_wrapper_spec = lto_wrapper_file;
set_static_spec_owned (&lto_wrapper_spec, lto_wrapper_file);
obstack_init (&collect_obstack);
obstack_grow (&collect_obstack, "COLLECT_LTO_WRAPPER=",
sizeof ("COLLECT_LTO_WRAPPER=") - 1);
@ -8851,7 +8896,7 @@ driver::maybe_run_linker (const char *argv0) const
{
char *s = find_a_file (&exec_prefixes, "collect2", X_OK, false);
if (s == NULL)
linker_name_spec = "ld";
set_static_spec_shared (&linker_name_spec, "ld");
}
#if HAVE_LTO_PLUGIN > 0
@ -8875,7 +8920,7 @@ driver::maybe_run_linker (const char *argv0) const
linker_plugin_file_spec = convert_white_space (temp_spec);
}
#endif
lto_gcc_spec = argv0;
set_static_spec_shared (&lto_gcc_spec, argv0);
}
/* Rebuild the COMPILER_PATH and LIBRARY_PATH environment variables
@ -10817,9 +10862,9 @@ driver::finalize ()
just_machine_suffix = 0;
gcc_exec_prefix = 0;
gcc_libexec_prefix = 0;
md_exec_prefix = MD_EXEC_PREFIX;
md_startfile_prefix = MD_STARTFILE_PREFIX;
md_startfile_prefix_1 = MD_STARTFILE_PREFIX_1;
set_static_spec_shared (&md_exec_prefix, MD_EXEC_PREFIX);
set_static_spec_shared (&md_startfile_prefix, MD_STARTFILE_PREFIX);
set_static_spec_shared (&md_startfile_prefix_1, MD_STARTFILE_PREFIX_1);
multilib_dir = 0;
multilib_os_dir = 0;
multiarch_dir = 0;
@ -10843,8 +10888,7 @@ driver::finalize ()
spec_list *sl = &static_specs[i];
if (sl->alloc_p)
{
if (0)
free (const_cast <char *> (*(sl->ptr_spec)));
free (const_cast <char *> (*(sl->ptr_spec)));
sl->alloc_p = false;
}
*(sl->ptr_spec) = sl->default_ptr;