On Wed, Nov 19, 2014 at 05:46:17AM -0500, David Malcolm wrote:
> This fixes various leaks of vec buffers seen via valgrind within jit
> (both recording and playback).
>
> Various vec<> within jit::recording are converted to auto_vec<>.
>
> Various playback::wrapper subclasses containing vec<> gain a finalizer
> so they can release the vec when they are collected.
It seems kind of tempting to just use a virtual dtor as the finalizer,
but I'm not sure how to make that work nicely with your overriding of
operator new.
Trev
>
> gcc/jit/ChangeLog:
> PR jit/63854
> * jit-playback.c (gcc::jit::playback::compound_type::set_fields):
> Convert param from const vec<playback::field *> & to
> const auto_vec<playback::field *> *.
> (gcc::jit::playback::context::new_function_type): Convert param
> "param_types" from vec<type *> * to const auto_vec<type *> *.
> (gcc::jit::playback::context::new_function): Convert param
> "params" from vec<param *> * to const auto_vec<param *> *.
> (gcc::jit::playback::context::build_call): Convert param "args"
> from vec<rvalue *> to const auto_vec<rvalue *> *.
> (gcc::jit::playback::context::new_call): Likewise.
> (gcc::jit::playback::context::new_call_through_ptr): Likewise.
> (wrapper_finalizer): New function.
> (gcc::jit::playback::wrapper::operator new): Call the finalizer
> variant of ggc_internal_cleared_alloc, supplying
> wrapper_finalizer.
> (gcc::jit::playback::function::finalizer): New.
> (gcc::jit::playback::block::finalizer): New.
> (gcc::jit::playback::source_file::finalizer): New.
> (gcc::jit::playback::source_line::finalizer): New.
>
> * jit-playback.h
> (gcc::jit::playback::context::new_function_type): Convert param
> "param_types" from vec<type *> * to const auto_vec<type *> *.
> (gcc::jit::playback::context::new_function): Convert param
> "params" from vec<param *> * to const auto_vec<param *> *.
> (gcc::jit::playback::context::new_call): Convert param
> "args" from vec<rvalue *> to const auto_vec<rvalue *> *.
> (gcc::jit::playback::context::new_call_through_ptr): Likewise.
> (gcc::jit::playback::context::build_call): Likewise.
> (gcc::jit::playback::context): Convert fields "m_functions",
> "m_source_files", "m_cached_locations" from vec to auto_vec.
> (gcc::jit::playback::wrapper::finalizer): New virtual function.
> (gcc::jit::playback::compound_type::set_fields): Convert param fro
> const vec<playback::field *> & to
> const auto_vec<playback::field *> *.
> (gcc::jit::playback::function::finalizer): New.
> (gcc::jit::playback::block::finalizer): New.
> (gcc::jit::playback::source_file::finalizer): New.
> (gcc::jit::playback::source_line::finalizer): New.
>
> * jit-recording.c
> (gcc::jit::recording::function_type::replay_into): Convert local
> from a vec into an auto_vec.
> (gcc::jit::recording::fields::replay_into): Likewise.
> (gcc::jit::recording::function::replay_into): Likewise.
> (gcc::jit::recording::call::replay_into): Likewise.
> (gcc::jit::recording::call_through_ptr::replay_into): Likewise.
>
> * jit-recording.h (gcc::jit::recording::context): Convert fields
> "m_mementos", "m_compound_types", "m_functions" from vec<> to
> auto_vec <>.
> (gcc::jit::recording::function_type::get_param_types): Convert
> return type from vec<type *> to const vec<type *> &.
> (gcc::jit::recording::function_type): Convert field
> "m_param_types" from a vec<> to an auto_vec<>.
> (gcc::jit::recording::fields): Likewise for field "m_fields".
> (gcc::jit::recording::function::get_params): Convert return type
> from vec <param *> to const vec<param *> &.
> (gcc::jit::recording::function): Convert fields "m_params",
> "m_locals", "m_blocks" from vec<> to auto_vec<>.
> (gcc::jit::recording::block): Likewise for field "m_statements".
> vec<> to auto_vec<>.
> (gcc::jit::recording::call): Likewise for field "m_args".
> (gcc::jit::recording::call_through_ptr): Likewise.
> ---
> gcc/jit/jit-playback.c | 73
> +++++++++++++++++++++++++++++++++++++++++--------
> gcc/jit/jit-playback.h | 27 ++++++++++++------
> gcc/jit/jit-recording.c | 16 +++++------
> gcc/jit/jit-recording.h | 26 +++++++++---------
> 4 files changed, 100 insertions(+), 42 deletions(-)
>
> diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
> index 285a3ef..8fdfa29 100644
> --- a/gcc/jit/jit-playback.c
> +++ b/gcc/jit/jit-playback.c
> @@ -285,15 +285,15 @@ new_compound_type (location *loc,
> }
>
> void
> -playback::compound_type::set_fields (const vec<playback::field *> &fields)
> +playback::compound_type::set_fields (const auto_vec<playback::field *>
> *fields)
> {
> /* Compare with c/c-decl.c: finish_struct. */
> tree t = as_tree ();
>
> tree fieldlist = NULL;
> - for (unsigned i = 0; i < fields.length (); i++)
> + for (unsigned i = 0; i < fields->length (); i++)
> {
> - field *f = fields[i];
> + field *f = (*fields)[i];
> DECL_CONTEXT (f->as_tree ()) = t;
> fieldlist = chainon (f->as_tree (), fieldlist);
> }
> @@ -309,7 +309,7 @@ playback::compound_type::set_fields (const
> vec<playback::field *> &fields)
> playback::type *
> playback::context::
> new_function_type (type *return_type,
> - vec<type *> *param_types,
> + const auto_vec<type *> *param_types,
> int is_variadic)
> {
> int i;
> @@ -361,7 +361,7 @@ new_function (location *loc,
> enum gcc_jit_function_kind kind,
> type *return_type,
> const char *name,
> - vec<param *> *params,
> + const auto_vec<param *> *params,
> int is_variadic,
> enum built_in_function builtin_id)
> {
> @@ -770,12 +770,12 @@ playback::rvalue *
> playback::context::
> build_call (location *loc,
> tree fn_ptr,
> - vec<rvalue *> args)
> + const auto_vec<rvalue *> *args)
> {
> vec<tree, va_gc> *tree_args;
> - vec_alloc (tree_args, args.length ());
> - for (unsigned i = 0; i < args.length (); i++)
> - tree_args->quick_push (args[i]->as_tree ());
> + vec_alloc (tree_args, args->length ());
> + for (unsigned i = 0; i < args->length (); i++)
> + tree_args->quick_push ((*args)[i]->as_tree ());
>
> if (loc)
> set_tree_location (fn_ptr, loc);
> @@ -806,7 +806,7 @@ playback::rvalue *
> playback::context::
> new_call (location *loc,
> function *func,
> - vec<rvalue *> args)
> + const auto_vec<rvalue *> *args)
> {
> tree fndecl;
>
> @@ -828,7 +828,7 @@ playback::rvalue *
> playback::context::
> new_call_through_ptr (location *loc,
> rvalue *fn_ptr,
> - vec<rvalue *> args)
> + const auto_vec<rvalue *> *args)
> {
> gcc_assert (fn_ptr);
> tree t_fn_ptr = fn_ptr->as_tree ();
> @@ -1079,6 +1079,18 @@ get_address (location *loc)
> return new rvalue (get_context (), ptr);
> }
>
> +/* The wrapper subclasses are GC-managed, but can own non-GC memory.
> + Provide this finalization hook for calling then they are collected,
> + which calls the finalizer vfunc. This allows them to call "release"
> + on any vec<> within them. */
> +
> +static void
> +wrapper_finalizer (void *ptr)
> +{
> + playback::wrapper *wrapper = reinterpret_cast <playback::wrapper *> (ptr);
> + wrapper->finalizer ();
> +}
> +
> /* gcc::jit::playback::wrapper subclasses are GC-managed:
> allocate them using ggc_internal_cleared_alloc. */
>
> @@ -1086,7 +1098,8 @@ void *
> playback::wrapper::
> operator new (size_t sz)
> {
> - return ggc_internal_cleared_alloc (sz MEM_STAT_INFO);
> + return ggc_internal_cleared_alloc (sz, wrapper_finalizer, 0, 1);
> +
> }
>
> /* Constructor for gcc:jit::playback::function. */
> @@ -1128,6 +1141,15 @@ gt_ggc_mx ()
> gt_ggc_m_9tree_node (m_inner_block);
> }
>
> +/* Don't leak vec's internal buffer (in non-GC heap) when we are
> + GC-ed. */
> +
> +void
> +playback::function::finalizer ()
> +{
> + m_blocks.release ();
> +}
> +
> /* Get the return type of a playback function, in tree form. */
>
> tree
> @@ -1262,6 +1284,15 @@ postprocess ()
> }
> }
>
> +/* Don't leak vec's internal buffer (in non-GC heap) when we are
> + GC-ed. */
> +
> +void
> +playback::block::finalizer ()
> +{
> + m_stmts.release ();
> +}
> +
> /* Add an eval of the rvalue to the function's statement list. */
>
> void
> @@ -2024,6 +2055,15 @@ playback::source_file::source_file (tree filename) :
> {
> }
>
> +/* Don't leak vec's internal buffer (in non-GC heap) when we are
> + GC-ed. */
> +
> +void
> +playback::source_file::finalizer ()
> +{
> + m_source_lines.release ();
> +}
> +
> /* Construct a playback::source_line for the given line
> within this source file, if one doesn't exist already. */
>
> @@ -2056,6 +2096,15 @@ playback::source_line::source_line (source_file *file,
> int line_num) :
> {
> }
>
> +/* Don't leak vec's internal buffer (in non-GC heap) when we are
> + GC-ed. */
> +
> +void
> +playback::source_line::finalizer ()
> +{
> + m_locations.release ();
> +}
> +
> /* Construct a playback::location for the given column
> within this line of a specific source file, if one doesn't exist
> already. */
> diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
> index dcb19bf..30e9229 100644
> --- a/gcc/jit/jit-playback.h
> +++ b/gcc/jit/jit-playback.h
> @@ -71,7 +71,7 @@ public:
>
> type *
> new_function_type (type *return_type,
> - vec<type *> *param_types,
> + const auto_vec<type *> *param_types,
> int is_variadic);
>
> param *
> @@ -84,7 +84,7 @@ public:
> enum gcc_jit_function_kind kind,
> type *return_type,
> const char *name,
> - vec<param *> *params,
> + const auto_vec<param *> *params,
> int is_variadic,
> enum built_in_function builtin_id);
>
> @@ -128,12 +128,12 @@ public:
> rvalue *
> new_call (location *loc,
> function *func,
> - vec<rvalue *> args);
> + const auto_vec<rvalue *> *args);
>
> rvalue *
> new_call_through_ptr (location *loc,
> rvalue *fn_ptr,
> - vec<rvalue *> args);
> + const auto_vec<rvalue *> *args);
>
> rvalue *
> new_cast (location *loc,
> @@ -214,7 +214,7 @@ private:
> rvalue *
> build_call (location *loc,
> tree fn_ptr,
> - vec<rvalue *> args);
> + const auto_vec<rvalue *> *args);
>
> tree
> build_cast (location *loc,
> @@ -240,14 +240,14 @@ private:
> char *m_path_s_file;
> char *m_path_so_file;
>
> - vec<function *> m_functions;
> + auto_vec<function *> m_functions;
> tree m_char_array_type_node;
> tree m_const_char_ptr;
>
> /* Source location handling. */
> - vec<source_file *> m_source_files;
> + auto_vec<source_file *> m_source_files;
>
> - vec<std::pair<tree, location *> > m_cached_locations;
> + auto_vec<std::pair<tree, location *> > m_cached_locations;
> };
>
> /* A temporary wrapper object.
> @@ -263,6 +263,10 @@ public:
> /* Allocate in the GC heap. */
> void *operator new (size_t sz);
>
> + /* Some wrapper subclasses contain vec<> and so need to
> + release them when they are GC-ed. */
> + virtual void finalizer () { }
> +
> };
>
> class type : public wrapper
> @@ -297,7 +301,7 @@ public:
> : type (inner)
> {}
>
> - void set_fields (const vec<field *> &fields);
> + void set_fields (const auto_vec<field *> *fields);
> };
>
> class field : public wrapper
> @@ -319,6 +323,7 @@ public:
> function(context *ctxt, tree fndecl, enum gcc_jit_function_kind kind);
>
> void gt_ggc_mx ();
> + void finalizer ();
>
> tree get_return_type_as_tree () const;
>
> @@ -366,6 +371,8 @@ public:
> block (function *func,
> const char *name);
>
> + void finalizer ();
> +
> tree as_label_decl () const { return m_label_decl; }
>
> void
> @@ -500,6 +507,7 @@ class source_file : public wrapper
> {
> public:
> source_file (tree filename);
> + void finalizer ();
>
> source_line *
> get_source_line (int line_num);
> @@ -520,6 +528,7 @@ class source_line : public wrapper
> {
> public:
> source_line (source_file *file, int line_num);
> + void finalizer ();
>
> location *
> get_location (recording::location *rloc, int column_num);
> diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
> index 8cce277..8069afc 100644
> --- a/gcc/jit/jit-recording.c
> +++ b/gcc/jit/jit-recording.c
> @@ -1602,7 +1602,7 @@ void
> recording::function_type::replay_into (replayer *r)
> {
> /* Convert m_param_types to a vec of playback type. */
> - vec <playback::type *> param_types;
> + auto_vec <playback::type *> param_types;
> int i;
> recording::type *type;
> param_types.create (m_param_types.length ());
> @@ -1859,11 +1859,11 @@ recording::fields::fields (compound_type
> *struct_or_union,
> void
> recording::fields::replay_into (replayer *)
> {
> - vec<playback::field *> playback_fields;
> + auto_vec<playback::field *> playback_fields;
> playback_fields.create (m_fields.length ());
> for (unsigned i = 0; i < m_fields.length (); i++)
> playback_fields.safe_push (m_fields[i]->playback_field ());
> - m_struct_or_union->playback_compound_type ()->set_fields (playback_fields);
> + m_struct_or_union->playback_compound_type ()->set_fields
> (&playback_fields);
> }
>
> /* Override the default implementation of
> @@ -2032,7 +2032,7 @@ void
> recording::function::replay_into (replayer *r)
> {
> /* Convert m_params to a vec of playback param. */
> - vec <playback::param *> params;
> + auto_vec <playback::param *> params;
> int i;
> recording::param *param;
> params.create (m_params.length ());
> @@ -2848,14 +2848,14 @@ recording::call::call (recording::context *ctxt,
> void
> recording::call::replay_into (replayer *r)
> {
> - vec<playback::rvalue *> playback_args;
> + auto_vec<playback::rvalue *> playback_args;
> playback_args.create (m_args.length ());
> for (unsigned i = 0; i< m_args.length (); i++)
> playback_args.safe_push (m_args[i]->playback_rvalue ());
>
> set_playback_obj (r->new_call (playback_location (r, m_loc),
> m_func->playback_function (),
> - playback_args));
> + &playback_args));
> }
>
> /* Implementation of recording::memento::make_debug_string for
> @@ -2925,14 +2925,14 @@ recording::call_through_ptr::call_through_ptr
> (recording::context *ctxt,
> void
> recording::call_through_ptr::replay_into (replayer *r)
> {
> - vec<playback::rvalue *> playback_args;
> + auto_vec<playback::rvalue *> playback_args;
> playback_args.create (m_args.length ());
> for (unsigned i = 0; i< m_args.length (); i++)
> playback_args.safe_push (m_args[i]->playback_rvalue ());
>
> set_playback_obj (r->new_call_through_ptr (playback_location (r, m_loc),
> m_fn_ptr->playback_rvalue (),
> - playback_args));
> + &playback_args));
> }
>
> /* Implementation of recording::memento::make_debug_string for
> diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
> index bb1a2ee..4ea8ef1 100644
> --- a/gcc/jit/jit-recording.h
> +++ b/gcc/jit/jit-recording.h
> @@ -242,11 +242,11 @@ private:
> bool m_bool_options[GCC_JIT_NUM_BOOL_OPTIONS];
>
> /* Recorded API usage. */
> - vec<memento *> m_mementos;
> + auto_vec<memento *> m_mementos;
>
> /* Specific recordings, for use by dump_to_file. */
> - vec<compound_type *> m_compound_types;
> - vec<function *> m_functions;
> + auto_vec<compound_type *> m_compound_types;
> + auto_vec<function *> m_functions;
>
> type *m_basic_types[NUM_GCC_JIT_TYPES];
> type *m_FILE_type;
> @@ -622,7 +622,7 @@ public:
> void replay_into (replayer *);
>
> type * get_return_type () const { return m_return_type; }
> - vec<type *> get_param_types () const { return m_param_types; }
> + const vec<type *> &get_param_types () const { return m_param_types; }
> int is_variadic () const { return m_is_variadic; }
>
> string * make_debug_string_with_ptr ();
> @@ -633,7 +633,7 @@ public:
>
> private:
> type *m_return_type;
> - vec<type *> m_param_types;
> + auto_vec<type *> m_param_types;
> int m_is_variadic;
> };
>
> @@ -749,7 +749,7 @@ private:
>
> private:
> compound_type *m_struct_or_union;
> - vec<field *> m_fields;
> + auto_vec<field *> m_fields;
> };
>
> class union_ : public compound_type
> @@ -897,7 +897,7 @@ public:
>
> type *get_return_type () const { return m_return_type; }
> string * get_name () const { return m_name; }
> - vec<param *> get_params () const { return m_params; }
> + const vec<param *> &get_params () const { return m_params; }
>
> /* Get the given param by index.
> Implements the post-error-checking part of
> @@ -920,11 +920,11 @@ private:
> enum gcc_jit_function_kind m_kind;
> type *m_return_type;
> string *m_name;
> - vec<param *> m_params;
> + auto_vec<param *> m_params;
> int m_is_variadic;
> enum built_in_function m_builtin_id;
> - vec<local *> m_locals;
> - vec<block *> m_blocks;
> + auto_vec<local *> m_locals;
> + auto_vec<block *> m_blocks;
> };
>
> class block : public memento
> @@ -1011,7 +1011,7 @@ private:
> function *m_func;
> int m_index;
> string *m_name;
> - vec<statement *> m_statements;
> + auto_vec<statement *> m_statements;
> bool m_has_been_terminated;
> bool m_is_reachable;
>
> @@ -1222,7 +1222,7 @@ private:
>
> private:
> function *m_func;
> - vec<rvalue *> m_args;
> + auto_vec<rvalue *> m_args;
> };
>
> class call_through_ptr : public rvalue
> @@ -1241,7 +1241,7 @@ private:
>
> private:
> rvalue *m_fn_ptr;
> - vec<rvalue *> m_args;
> + auto_vec<rvalue *> m_args;
> };
>
> class array_access : public lvalue
> --
> 1.8.5.3
>
