There can be multiple gcc_options instances, each with
a call to
init_options_struct
matched with a call to
finalize_options_struct
whereas the
opts_obstack
is a singleton. Each gcc_options instance can potentially use the
opts_obstack singleton.
r230264 (aka 25faed340686df8d7bb2242dc8d04285976922b6) fixed
a large memory leak (1.2MB) of the opts_obstack, by making
initialization of the opts_obstack be idempotent
(in init_opts_obstack).
This works if we only have one in-process run of the compiler.
Unfortunately this commit broke much of libgccjit's test suite,
which now fails with memory corruption errors.
The root cause of the breakage is that toplev::finalize cleans up the
opts_obstack using:
obstack_free (opts_obstack, NULL);
Calling obstack_free with NULL leaves an obstack in an uninitialized
state and hence a reinitialization is required;
libiberty/obstacks.texi has:
Note that if @var{object} is a null pointer, the result is an
*uninitialized* obstack. [my emphasis]
Hence opts_obstack reverts to an uninitialized state - but further
calls to initialize it are rejected by the idempotency code in
init_opts_obstack, and we then attempt to allocate from an
uninitialized obstack.
In particular, the obstack's "chunk" field becomes invalid, but isn't
unset. The underlying 64KB chunk(s) are returned to memory_block_pool's
m_blocks linked-list of 64KB free chunks, and they get reused by othe
obstacks e.g. for bitmaps. However, given that opts_obstack fails
to be reinitialized, opts_obstack.chunks points at a freed chunk.
Hence, on the 2nd iteration of a jit testcase, it gets used to
allocate copies of the options, but this out of a chunk that's being
used by a different memory_block_pool user, so chaos ensues: we have
64KB chunks of memory being erroneously shared between different
memory-pool users.
The following patch removes idempotency from init_opts_obstack, and
replaces the call to init_opts_stack from init_options_struct with
an assert that the singleton opts_stack is already initialized,
adding in the necessary per-compile initialization of opts_stack
(we already have per-compile cleanup).
Or to put it another way, previously, we had this pattern of calls:
- for each jit compile:
- toplev:
- multiple calls to init_options_struct matched with calls to
finalize_options_struct; the first call to init_options_struct
idempotently initializes opts_obstack.
- obstack_free (&opts_obstack, NULL);
(leading to corrupt opts_obstack on the 2nd iteration of jit compilation)
and with this patch we instead have this:
- for each jit compile:
- toplev:
- init_opts_obstack
- multiple calls to init_options_struct matched with calls to
finalize_options_struct
- obstack_free (&opts_obstack, NULL);
(I don't like that opts_obstack is global state, but it seems risky
to try to fix that at this stage).
The patch also adds code to reset save_decoded_options and
save_decoded_options_count when freeing opts_obstack, since these
saved options are allocated from out of opts_obstack and hence
also become invalid when it's freed.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu in
conjunction with the followup patch.
Fixes all of the jit test suite, apart from test-threads.c (which is
broken for a different reason). I've also verified manually under
Valgrind that this also keeps the fix for the large leak reported in
the PR that motivated r230264).
OK for trunk? (assuming it bootstraps®rtests by itself)
gcc/ChangeLog:
PR jit/68446
* gcc.c (driver::decode_argv): Add call to
init_opts_obstack before init_options_struct.
* opts.c (init_opts_obstack): Remove idempotency.
(init_options_struct): Replace call to init_opts_obstack
with a gcc_assert to verify that it has already been called.
* toplev.c (toplev::main): Add call to init_opts_obstack before
calls to init_options_struct.
(toplev::finalize): Move cleanup of opts_obstack next to
cleanup of save_decoded_options, clearing the latter, and
save_decoded_options_count.
---
gcc/gcc.c | 1 +
gcc/opts.c | 14 +++++---------
gcc/toplev.c | 7 ++++++-
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 319a073..c191fde 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -7191,6 +7191,7 @@ driver::decode_argv (int argc, const char **argv)
global_init_params ();
finish_params ();
+ init_opts_obstack ();
init_options_struct (&global_options, &global_options_set);
decode_cmdline_options_to_array (argc, argv,
diff --git a/gcc/opts.c b/gcc/opts.c
index 2add158..9437535 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -266,18 +266,12 @@ add_comma_separated_to_vector (void **pvec, const char
*arg)
*pvec = v;
}
-/* Initialize opts_obstack if not initialized. */
+/* Initialize opts_obstack. */
void
init_opts_obstack (void)
{
- static bool opts_obstack_initialized = false;
-
- if (!opts_obstack_initialized)
- {
- opts_obstack_initialized = true;
- gcc_obstack_init (&opts_obstack);
- }
+ gcc_obstack_init (&opts_obstack);
}
/* Initialize OPTS and OPTS_SET before using them in parsing options. */
@@ -287,7 +281,9 @@ init_options_struct (struct gcc_options *opts, struct
gcc_options *opts_set)
{
size_t num_params = get_num_compiler_params ();
- init_opts_obstack ();
+ /* Ensure that opts_obstack has already been initialized by the time
+ that we initialize any gcc_options instances (PR jit/68446). */
+ gcc_assert (opts_obstack.chunk_size > 0);
*opts = global_options_init;
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 8bab3e5..580c439 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -2050,6 +2050,7 @@ toplev::main (int argc, char **argv)
/* One-off initialization of options that does not need to be
repeated when options are added for particular functions. */
init_options_once ();
+ init_opts_obstack ();
/* Initialize global options structures; this must be repeated for
each structure used for parsing options. */
@@ -2131,11 +2132,15 @@ toplev::finalize (void)
finalize_options_struct (&global_options);
finalize_options_struct (&global_options_set);
+ /* save_decoded_options uses opts_obstack, so these must
+ be cleaned up together. */
+ obstack_free (&opts_obstack, NULL);
XDELETEVEC (save_decoded_options);
+ save_decoded_options = NULL;
+ save_decoded_options_count = 0;
/* Clean up the context (and pass_manager etc). */
delete g;
g = NULL;
- obstack_free (&opts_obstack, NULL);
}
--
1.8.5.3
