Re: [PATCH 10/21] PR jit/63854: Fix leak of worklist within jit-recording.c
On Wed, Nov 19, 2014 at 9:02 PM, David Malcolm dmalc...@redhat.com wrote: On Wed, 2014-11-19 at 09:59 -0700, Jeff Law wrote: On 11/19/14 03:46, David Malcolm wrote: Fix this leak: 160 bytes in 5 blocks are definitely lost in loss record 154 of 228 at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x5D75D4F: xrealloc (xmalloc.c:177) by 0x4DE1710: void va_heap::reservegcc::jit::recording::block*(vecgcc::jit::recording::block*, va_heap, vl_embed*, unsigned int, bool) (vec.h:310) by 0x4DDFAB5: vecgcc::jit::recording::block*, va_heap, vl_ptr::reserve(unsigned int, bool) (vec.h:1428) by 0x4DDFBFC: vecgcc::jit::recording::block*, va_heap, vl_ptr::reserve_exact(unsigned int) (vec.h:1448) by 0x4DDE588: vecgcc::jit::recording::block*, va_heap, vl_ptr::create(unsigned int) (vec.h:1463) by 0x4DD9B9F: gcc::jit::recording::function::validate() (jit-recording.c:2191) by 0x4DD7AD3: gcc::jit::recording::context::validate() (jit-recording.c:1005) by 0x4DD7660: gcc::jit::recording::context::compile() (jit-recording.c:848) by 0x4DD5BD2: gcc_jit_context_compile (libgccjit.c:2014) by 0x401CA4: test_jit (harness.h:190) by 0x401D88: main (harness.h:232) gcc/jit/ChangeLog: PR jit/63854 * jit-recording.c (recording::function::validate): Convert worklist from vec to autovec to fix a leak. JIT space, yours to approve :-) We haven't formalized that yet, but it'd be silly to do anything else. FWIW, I added myself to the MAINTAINERS file as JIT maintainer as part of a change you reviewed as: https://gcc.gnu.org/ml/jit/2014-q4/msg00029.html Is there a governance distinction here, between patch review vs decisions of the steering committee? i.e. do changes to the maintainers part of the MAINTAINERS file require higher-level approval? Yes, reviewers and maintainers are appointed by the steering commitee only. Richard. Presumably I should continue to send (non-trivial) jit patches to this list and wait for review before committing to trunk? Anyway so formally, this is OK for the trunk. Thanks.
Re: [PATCH 10/21] PR jit/63854: Fix leak of worklist within jit-recording.c
On 11/20/14 09:01, Richard Biener wrote: Is there a governance distinction here, between patch review vs decisions of the steering committee? i.e. do changes to the maintainers part of the MAINTAINERS file require higher-level approval? Yes, reviewers and maintainers are appointed by the steering commitee only. Right. I've already raised appointing David as the JIT maintainer to the steering committee. I just need to count the votes and take appropriate action. Similarly for the MPX runtime and Ilya as the MPX maintainer, Bernd as the nvptx maintainer. If there's other maintainers that need to get appointed, nobody should hesitate to contact one of the SC members to get the nomination in front of the committee. jeff
[PATCH 10/21] PR jit/63854: Fix leak of worklist within jit-recording.c
Fix this leak:
160 bytes in 5 blocks are definitely lost in loss record 154 of 228
at 0x4A0645D: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x5D75D4F: xrealloc (xmalloc.c:177)
by 0x4DE1710: void
va_heap::reservegcc::jit::recording::block*(vecgcc::jit::recording::block*,
va_heap, vl_embed*, unsigned int, bool) (vec.h:310)
by 0x4DDFAB5: vecgcc::jit::recording::block*, va_heap,
vl_ptr::reserve(unsigned int, bool) (vec.h:1428)
by 0x4DDFBFC: vecgcc::jit::recording::block*, va_heap,
vl_ptr::reserve_exact(unsigned int) (vec.h:1448)
by 0x4DDE588: vecgcc::jit::recording::block*, va_heap,
vl_ptr::create(unsigned int) (vec.h:1463)
by 0x4DD9B9F: gcc::jit::recording::function::validate()
(jit-recording.c:2191)
by 0x4DD7AD3: gcc::jit::recording::context::validate() (jit-recording.c:1005)
by 0x4DD7660: gcc::jit::recording::context::compile() (jit-recording.c:848)
by 0x4DD5BD2: gcc_jit_context_compile (libgccjit.c:2014)
by 0x401CA4: test_jit (harness.h:190)
by 0x401D88: main (harness.h:232)
gcc/jit/ChangeLog:
PR jit/63854
* jit-recording.c (recording::function::validate): Convert
worklist from vec to autovec to fix a leak.
---
gcc/jit/jit-recording.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index 8daa8f2..8cce277 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -2187,8 +2187,7 @@ recording::function::validate ()
{
/* Iteratively walk the graph of blocks, marking their m_is_reachable
flag, starting at the initial block. */
- vecblock * worklist;
- worklist.create (m_blocks.length ());
+ auto_vecblock * worklist (m_blocks.length ());
worklist.safe_push (m_blocks[0]);
while (worklist.length () 0)
{
--
1.8.5.3
Re: [PATCH 10/21] PR jit/63854: Fix leak of worklist within jit-recording.c
On 11/19/14 03:46, David Malcolm wrote: Fix this leak: 160 bytes in 5 blocks are definitely lost in loss record 154 of 228 at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x5D75D4F: xrealloc (xmalloc.c:177) by 0x4DE1710: void va_heap::reservegcc::jit::recording::block*(vecgcc::jit::recording::block*, va_heap, vl_embed*, unsigned int, bool) (vec.h:310) by 0x4DDFAB5: vecgcc::jit::recording::block*, va_heap, vl_ptr::reserve(unsigned int, bool) (vec.h:1428) by 0x4DDFBFC: vecgcc::jit::recording::block*, va_heap, vl_ptr::reserve_exact(unsigned int) (vec.h:1448) by 0x4DDE588: vecgcc::jit::recording::block*, va_heap, vl_ptr::create(unsigned int) (vec.h:1463) by 0x4DD9B9F: gcc::jit::recording::function::validate() (jit-recording.c:2191) by 0x4DD7AD3: gcc::jit::recording::context::validate() (jit-recording.c:1005) by 0x4DD7660: gcc::jit::recording::context::compile() (jit-recording.c:848) by 0x4DD5BD2: gcc_jit_context_compile (libgccjit.c:2014) by 0x401CA4: test_jit (harness.h:190) by 0x401D88: main (harness.h:232) gcc/jit/ChangeLog: PR jit/63854 * jit-recording.c (recording::function::validate): Convert worklist from vec to autovec to fix a leak. JIT space, yours to approve :-) We haven't formalized that yet, but it'd be silly to do anything else. Anyway so formally, this is OK for the trunk. jeff
Re: [PATCH 10/21] PR jit/63854: Fix leak of worklist within jit-recording.c
On Wed, 2014-11-19 at 09:59 -0700, Jeff Law wrote: On 11/19/14 03:46, David Malcolm wrote: Fix this leak: 160 bytes in 5 blocks are definitely lost in loss record 154 of 228 at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x5D75D4F: xrealloc (xmalloc.c:177) by 0x4DE1710: void va_heap::reservegcc::jit::recording::block*(vecgcc::jit::recording::block*, va_heap, vl_embed*, unsigned int, bool) (vec.h:310) by 0x4DDFAB5: vecgcc::jit::recording::block*, va_heap, vl_ptr::reserve(unsigned int, bool) (vec.h:1428) by 0x4DDFBFC: vecgcc::jit::recording::block*, va_heap, vl_ptr::reserve_exact(unsigned int) (vec.h:1448) by 0x4DDE588: vecgcc::jit::recording::block*, va_heap, vl_ptr::create(unsigned int) (vec.h:1463) by 0x4DD9B9F: gcc::jit::recording::function::validate() (jit-recording.c:2191) by 0x4DD7AD3: gcc::jit::recording::context::validate() (jit-recording.c:1005) by 0x4DD7660: gcc::jit::recording::context::compile() (jit-recording.c:848) by 0x4DD5BD2: gcc_jit_context_compile (libgccjit.c:2014) by 0x401CA4: test_jit (harness.h:190) by 0x401D88: main (harness.h:232) gcc/jit/ChangeLog: PR jit/63854 * jit-recording.c (recording::function::validate): Convert worklist from vec to autovec to fix a leak. JIT space, yours to approve :-) We haven't formalized that yet, but it'd be silly to do anything else. FWIW, I added myself to the MAINTAINERS file as JIT maintainer as part of a change you reviewed as: https://gcc.gnu.org/ml/jit/2014-q4/msg00029.html Is there a governance distinction here, between patch review vs decisions of the steering committee? i.e. do changes to the maintainers part of the MAINTAINERS file require higher-level approval? Presumably I should continue to send (non-trivial) jit patches to this list and wait for review before committing to trunk? Anyway so formally, this is OK for the trunk. Thanks.
