From: David Malcolm <dmalcolm@redhat.com>
To: gcc-patches@gcc.gnu.org, jit@gcc.gnu.org
Cc: David Malcolm <dmalcolm@redhat.com>
Subject: [PATCH 02/08] PR jit/63854: Fix leak within jit-builtins.c
Date: Wed, 01 Jan 2014 00:00:00 -0000 [thread overview]
Message-ID: <1416965978-15582-3-git-send-email-dmalcolm@redhat.com> (raw)
In-Reply-To: <1416965978-15582-1-git-send-email-dmalcolm@redhat.com>
Running testsuite/jit.dg/test-functions.c under valgrind showed this
leak (amongst others):
520 (320 direct, 200 indirect) bytes in 5 blocks are definitely lost in loss record 144 of 181
at 0x4A06965: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4DF2110: gcc::jit::recording::builtins_manager::make_fn_type(gcc::jit::recording::jit_builtin_type, gcc::jit::recording::jit_builtin_type, bool, int, ...) (jit-builtins.c:405)
by 0x4DEEDD4: gcc::jit::recording::builtins_manager::make_type(gcc::jit::recording::jit_builtin_type) (builtin-types.def:242)
by 0x4DED7B9: gcc::jit::recording::builtins_manager::get_type(gcc::jit::recording::jit_builtin_type) (jit-builtins.c:216)
by 0x4DED5BB: gcc::jit::recording::builtins_manager::make_builtin_function(built_in_function) (jit-builtins.c:177)
by 0x4DED503: gcc::jit::recording::builtins_manager::get_builtin_function(char const*) (jit-builtins.c:163)
by 0x4DD7F57: gcc::jit::recording::context::get_builtin_function(char const*) (jit-recording.c:576)
by 0x4DD33D5: gcc_jit_context_get_builtin_function (libgccjit.c:851)
by 0x402820: create_test_of_builtin_strcmp (test-functions.c:134)
by 0x402B2D: create_use_of_builtins (test-functions.c:231)
by 0x4033E1: create_code (test-functions.c:345)
by 0x40216D: test_jit (harness.h:217)
The recording::function_type instances created by the builtin_manager
were being leaked. These are recording::memento instances, and thus
the context is responsible for freeing them - the bug was that they
weren't getting recorded into the context's list of mementos.
In general, only the recording::context should directly create mementos;
everything else goes through factory methods of recording::context, which
takes care of calling "record" on the new instances (putting them into
the list of "owned" objects).
Fix this by creating a new "new_function_type" factory method of the
context and using it from jit-builtins.c
gcc/jit/ChangeLog:
PR jit/63854
* jit-builtins.c
(gcc::jit::recording::builtins_manager::make_fn_type): Call the
context's new_function_type method, rather than directly creating
a function_type instance.
* jit-recording.c
(gcc::jit::recording::context::new_function_type): New method,
adapted from part of...
(gcc::jit::recording::context::new_function_ptr_type): ...this.
Update to call new_function_type.
* jit-recording.h
(gcc::jit::recording::context::new_function_type): New method.
---
gcc/jit/jit-builtins.c | 9 ++++-----
gcc/jit/jit-recording.c | 33 ++++++++++++++++++++++++++-------
gcc/jit/jit-recording.h | 6 ++++++
3 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/gcc/jit/jit-builtins.c b/gcc/jit/jit-builtins.c
index 07902e8..49d37d8 100644
--- a/gcc/jit/jit-builtins.c
+++ b/gcc/jit/jit-builtins.c
@@ -398,11 +398,10 @@ builtins_manager::make_fn_type (enum jit_builtin_type,
if (!return_type)
goto error;
- result = new function_type (m_ctxt,
- return_type,
- num_args,
- param_types,
- is_variadic);
+ result = m_ctxt->new_function_type (return_type,
+ num_args,
+ param_types,
+ is_variadic);
error:
delete[] param_types;
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index 8069afc..32bf034 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -492,6 +492,27 @@ recording::context::new_union_type (recording::location *loc,
return result;
}
+/* Create a recording::function_type instance and add it to this context's
+ list of mementos.
+
+ Used by new_function_ptr_type and by builtins_manager::make_fn_type. */
+
+recording::function_type *
+recording::context::new_function_type (recording::type *return_type,
+ int num_params,
+ recording::type **param_types,
+ int is_variadic)
+{
+ recording::function_type *fn_type
+ = new function_type (this,
+ return_type,
+ num_params,
+ param_types,
+ is_variadic);
+ record (fn_type);
+ return fn_type;
+}
+
/* Create a recording::type instance and add it to this context's list
of mementos.
@@ -505,13 +526,11 @@ recording::context::new_function_ptr_type (recording::location *, /* unused loc
recording::type **param_types,
int is_variadic)
{
- recording::function_type *fn_type =
- new function_type (this,
- return_type,
- num_params,
- param_types,
- is_variadic);
- record (fn_type);
+ recording::function_type *fn_type
+ = new_function_type (return_type,
+ num_params,
+ param_types,
+ is_variadic);
/* Return a pointer-type to the the function type. */
return fn_type->get_pointer ();
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 4ea8ef1..9b2cfc6 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -88,6 +88,12 @@ public:
new_union_type (location *loc,
const char *name);
+ function_type *
+ new_function_type (type *return_type,
+ int num_params,
+ type **param_types,
+ int is_variadic);
+
type *
new_function_ptr_type (location *loc,
type *return_type,
--
1.8.5.3
next prev parent reply other threads:[~2014-11-26 1:32 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-01 0:00 [PATCH 00/08] More memory leak fixes David Malcolm
2014-01-01 0:00 ` [PATCH 08/08] PR/64003 workaround (uninit memory in i386.md) David Malcolm
2014-01-01 0:00 ` Jeff Law
2014-01-01 0:00 ` David Malcolm
2014-01-01 0:00 ` Jeff Law
2014-01-01 0:00 ` Jeff Law
2014-01-01 0:00 ` [PATCH 01/08] PR jit/63854: Fix leak in tree-ssa-math-opts.c David Malcolm
2014-01-01 0:00 ` Richard Biener
2014-01-01 0:00 ` [PATCH 04/08] PR jit/63854: Remove xstrdup from ipa/cgraph fprintf calls David Malcolm
2014-01-01 0:00 ` [PATCH 07/08] PR jit/63854: Fix leaks in toyvm.c David Malcolm
2014-01-01 0:00 ` [PATCH 03/08] PR jit/63854: Fix leak in real.c for i386:init_ext_80387_constants David Malcolm
2014-01-01 0:00 ` Richard Biener
2014-01-01 0:00 ` David Malcolm [this message]
2014-01-01 0:00 ` [PATCH 06/08] Avoid overuse of name "buffer" in tree-pretty-print.c David Malcolm
2014-01-01 0:00 ` Jeff Law
2014-01-01 0:00 ` [PATCH 05/08] PR jit/63854: Fix double-initialization within tree-pretty-print.c David Malcolm
2014-01-01 0:00 ` Jeff Law
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1416965978-15582-3-git-send-email-dmalcolm@redhat.com \
--to=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jit@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).