Calling a sub that uses a subsignature leaks memory

[p6rt]

Migrated from rt.perl.org#126183 (status was 'resolved')

Searchable as RT126183$

[p6rt]

From @hoelzro

See subject and the attached script.

To run the attached test script, you'll need to build a patched MoarVM (apply meminfo-moar.patch and run tools/update_ops.p6), a patched NQP (apply meminfo-nqp.patch), and Rakudo using those two patched programs. The patch adds support for a meminfo() op that reports memory using mallinfo(), which I believe is glibc-specific.

The test script prints out memory usage each second; you only really need to care about the first column, which is the amount of memory allocated not using mmap. I have also attached an Octave program I used to plot the data from the script; I have attached one of the plots as well.

I believe the problem is here​:

https://github.com/MoarVM/MoarVM/blob/51dcbe4a41d5e2853ff32c9d9632c53a5ae44340/src/6model/reprs/MVMCallCapture.c#L61

and here​:

https://github.com/MoarVM/MoarVM/blob/51dcbe4a41d5e2853ff32c9d9632c53a5ae44340/src/core/args.c#L62

effective_callsite points to a new callsite object, which is only free'd if it's not equal to the original callsite object. However, further down in args.c, callsite is set to the location of effective_callsite.

[p6rt]

From @hoelzro

memcheck-test.pl

[p6rt]

From @hoelzro

meminfo-moar.patch

diff --git a/src/core/interp.c b/src/core/interp.c
index c1c5f63..37972f4 100644
--- a/src/core/interp.c
+++ b/src/core/interp.c
@@ -2,6 +2,8 @@
 #include <math.h>
 #include "platform/time.h"
 
+#include <malloc.h>
+
 /* Macros for getting things from the bytecode stream. */
 #define GET_REG(pc, idx)    reg_base[*((MVMuint16 *)(pc + idx))]
 #define GET_LEX(pc, idx, f) f->env[*((MVMuint16 *)(pc + idx))]
@@ -5117,6 +5119,21 @@ void MVM_interp_run(MVMThreadContext *tc, void (*initial_invoke)(MVMThreadContex
                 MVM_cross_thread_write_check(tc, obj, blame);
                 goto NEXT;
             }
+            OP(meminfo): {
+                char buffer[256];
+                struct mallinfo mi;
+
+                mi = mallinfo();
+
+                snprintf(buffer, 255, "%d %d %d %d %d %d %d %d %d %d",
+                    mi.arena, mi.ordblks, mi.smblks,
+                    mi.hblks, mi.hblkhd, mi.usmblks,
+                    mi.fsmblks, mi.uordblks, mi.fordblks, mi.keepcost);
+                buffer[255] = '\0';
+                GET_REG(cur_op, 0).s = MVM_string_ascii_decode(tc, tc->instance->VMString, buffer, strlen(buffer));
+                cur_op += 2;
+                goto NEXT;
+            }
 #if MVM_CGOTO
             OP_CALL_EXTOP: {
                 /* Bounds checking? Never heard of that. */
diff --git a/src/core/oplist b/src/core/oplist
index 870ef22..7db22f4 100644
--- a/src/core/oplist
+++ b/src/core/oplist
@@ -841,3 +841,5 @@ prof_allocated   .s r(obj)
 
 # Cross-thread write analysis logging instruction.
 ctw_check        .s r(obj) int16
+
+meminfo        w(str)

[p6rt]

From @hoelzro

meminfo-nqp.patch

diff --git a/src/vm/moar/QAST/QASTOperationsMAST.nqp b/src/vm/moar/QAST/QASTOperationsMAST.nqp
index faa62b4..811bb7d 100644
--- a/src/vm/moar/QAST/QASTOperationsMAST.nqp
+++ b/src/vm/moar/QAST/QASTOperationsMAST.nqp
@@ -2809,6 +2809,8 @@ QAST::MASTOperations.add_core_moarop_mapping('mvmendprofile', 'endprofile');
 # MoarVM-specific GC ops
 QAST::MASTOperations.add_core_moarop_mapping('force_gc', 'force_gc');
 
+QAST::MASTOperations.add_core_moarop_mapping('meminfo', 'meminfo');
+
 sub resolve_condition_op($kind, $negated) {
     return $negated ??
         $kind == $MVM_reg_int64 ?? 'unless_i' !!

[p6rt]

From @hoelzro

plot-arena-size.m

[p6rt]

From @hoelzro

plot.png

[p6rt]

From @hoelzro

So I dug into this a bit, and I put a new struct member on MVMCallCaptureBody objects that indicates whether they own their effective callsite. Setting that flag in MVM_args_save_capture and freeing based on its presence in gc_free plugs the leak; however, building Rakudo with that change cause a segfault. Interestingly enough, setting MVM_SPESH_DISABLE=1 allows Rakudo to build normally.

On 2015-09-25 07​:33​:24, rob@​hoelz.ro wrote​:

See subject and the attached script.

To run the attached test script, you'll need to build a patched MoarVM
(apply meminfo-moar.patch and run tools/update_ops.p6), a patched NQP
(apply meminfo-nqp.patch), and Rakudo using those two patched
programs. The patch adds support for a meminfo() op that reports
memory using mallinfo(), which I believe is glibc-specific.

The test script prints out memory usage each second; you only really
need to care about the first column, which is the amount of memory
allocated not using mmap. I have also attached an Octave program I
used to plot the data from the script; I have attached one of the
plots as well.

I believe the problem is here​:

https://github.com/MoarVM/MoarVM/blob/51dcbe4a41d5e2853ff32c9d9632c53a5ae44340/src/6model/reprs/MVMCallCapture.c#L61

and here​:

https://github.com/MoarVM/MoarVM/blob/51dcbe4a41d5e2853ff32c9d9632c53a5ae44340/src/core/args.c#L62

effective_callsite points to a new callsite object, which is only
free'd if it's not equal to the original callsite object. However,
further down in args.c, callsite is set to the location of
effective_callsite.

[p6rt]

From @hoelzro

Dug into this further; it's the invocation of savecapture in Perl6​::Actions' onlystar method that causes the segfault. Interestingly enough, savecapture is used in a very similar manner in Actions' autogenerate_proto method, which does *not* cause a segfault.

On 2015-09-26 08​:18​:54, rob@​hoelz.ro wrote​:

So I dug into this a bit, and I put a new struct member on
MVMCallCaptureBody objects that indicates whether they own their
effective callsite. Setting that flag in MVM_args_save_capture and
freeing based on its presence in gc_free plugs the leak; however,
building Rakudo with that change cause a segfault. Interestingly
enough, setting MVM_SPESH_DISABLE=1 allows Rakudo to build normally.

On 2015-09-25 07​:33​:24, rob@​hoelz.ro wrote​:

See subject and the attached script.

To run the attached test script, you'll need to build a patched
MoarVM
(apply meminfo-moar.patch and run tools/update_ops.p6), a patched NQP
(apply meminfo-nqp.patch), and Rakudo using those two patched
programs. The patch adds support for a meminfo() op that reports
memory using mallinfo(), which I believe is glibc-specific.

The test script prints out memory usage each second; you only really
need to care about the first column, which is the amount of memory
allocated not using mmap. I have also attached an Octave program I
used to plot the data from the script; I have attached one of the
plots as well.

I believe the problem is here​:

https://github.com/MoarVM/MoarVM/blob/51dcbe4a41d5e2853ff32c9d9632c53a5ae44340/src/6model/reprs/MVMCallCapture.c#L61

and here​:

https://github.com/MoarVM/MoarVM/blob/51dcbe4a41d5e2853ff32c9d9632c53a5ae44340/src/core/args.c#L62

effective_callsite points to a new callsite object, which is only
free'd if it's not equal to the original callsite object. However,
further down in args.c, callsite is set to the location of
effective_callsite.

[p6rt]

From @hoelzro

It seems to be the invocation of onlystar that happens as a result of routine_def; I'm not sure if this is because of a problem with routine_def itself, or just the fact that routine_def is called much much more than other users of onlystar and this is GC oddness.

On 2015-09-27 21​:26​:16, rob@​hoelz.ro wrote​:

Dug into this further; it's the invocation of savecapture in
Perl6​::Actions' onlystar method that causes the segfault.
Interestingly enough, savecapture is used in a very similar manner in
Actions' autogenerate_proto method, which does *not* cause a segfault.

On 2015-09-26 08​:18​:54, rob@​hoelz.ro wrote​:

So I dug into this a bit, and I put a new struct member on
MVMCallCaptureBody objects that indicates whether they own their
effective callsite. Setting that flag in MVM_args_save_capture and
freeing based on its presence in gc_free plugs the leak; however,
building Rakudo with that change cause a segfault. Interestingly
enough, setting MVM_SPESH_DISABLE=1 allows Rakudo to build normally.

On 2015-09-25 07​:33​:24, rob@​hoelz.ro wrote​:

See subject and the attached script.

To run the attached test script, you'll need to build a patched
MoarVM
(apply meminfo-moar.patch and run tools/update_ops.p6), a patched
NQP
(apply meminfo-nqp.patch), and Rakudo using those two patched
programs. The patch adds support for a meminfo() op that reports
memory using mallinfo(), which I believe is glibc-specific.

The test script prints out memory usage each second; you only
really
need to care about the first column, which is the amount of memory
allocated not using mmap. I have also attached an Octave program I
used to plot the data from the script; I have attached one of the
plots as well.

I believe the problem is here​:

https://github.com/MoarVM/MoarVM/blob/51dcbe4a41d5e2853ff32c9d9632c53a5ae44340/src/6model/reprs/MVMCallCapture.c#L61

and here​:

https://github.com/MoarVM/MoarVM/blob/51dcbe4a41d5e2853ff32c9d9632c53a5ae44340/src/core/args.c#L62

effective_callsite points to a new callsite object, which is only
free'd if it's not equal to the original callsite object. However,
further down in args.c, callsite is set to the location of
effective_callsite.

[p6rt]

From @hoelzro

It seems the spesh/onlystar stuff was a red herring; I just made a beginner C mistake. =/ Fix for the leak and the segfault is here​: MoarVM/MoarVM#274

On 2015-09-28 06​:36​:39, rob@​hoelz.ro wrote​:

It seems to be the invocation of onlystar that happens as a result of
routine_def; I'm not sure if this is because of a problem with
routine_def itself, or just the fact that routine_def is called much
much more than other users of onlystar and this is GC oddness.

On 2015-09-27 21​:26​:16, rob@​hoelz.ro wrote​:

Dug into this further; it's the invocation of savecapture in
Perl6​::Actions' onlystar method that causes the segfault.
Interestingly enough, savecapture is used in a very similar manner in
Actions' autogenerate_proto method, which does *not* cause a
segfault.

On 2015-09-26 08​:18​:54, rob@​hoelz.ro wrote​:

So I dug into this a bit, and I put a new struct member on
MVMCallCaptureBody objects that indicates whether they own their
effective callsite. Setting that flag in MVM_args_save_capture and
freeing based on its presence in gc_free plugs the leak; however,
building Rakudo with that change cause a segfault. Interestingly
enough, setting MVM_SPESH_DISABLE=1 allows Rakudo to build
normally.

On 2015-09-25 07​:33​:24, rob@​hoelz.ro wrote​:

See subject and the attached script.

To run the attached test script, you'll need to build a patched
MoarVM
(apply meminfo-moar.patch and run tools/update_ops.p6), a patched
NQP
(apply meminfo-nqp.patch), and Rakudo using those two patched
programs. The patch adds support for a meminfo() op that reports
memory using mallinfo(), which I believe is glibc-specific.

The test script prints out memory usage each second; you only
really
need to care about the first column, which is the amount of
memory
allocated not using mmap. I have also attached an Octave program
I
used to plot the data from the script; I have attached one of the
plots as well.

I believe the problem is here​:

https://github.com/MoarVM/MoarVM/blob/51dcbe4a41d5e2853ff32c9d9632c53a5ae44340/src/6model/reprs/MVMCallCapture.c#L61

and here​:

https://github.com/MoarVM/MoarVM/blob/51dcbe4a41d5e2853ff32c9d9632c53a5ae44340/src/core/args.c#L62

effective_callsite points to a new callsite object, which is only
free'd if it's not equal to the original callsite object.
However,
further down in args.c, callsite is set to the location of
effective_callsite.

[p6rt]

@hoelzro - Status changed from 'new' to 'resolved'

[p6rt]

From @hoelzro

Apparently my patch causing a use-after-free bug​:

http://paste.scsys.co.uk/499584

I'm reverting the commit that I made to fix this, and sticking it in a branch. I'll merge the branch once it's fixed.

[p6rt]

@hoelzro - Status changed from 'resolved' to 'open'

[p6rt]

From @hoelzro

On 2015-09-30 06​:55​:11, rob@​hoelz.ro wrote​:

Apparently my patch causing a use-after-free bug​:

http://paste.scsys.co.uk/499584

I'm reverting the commit that I made to fix this, and sticking it in a
branch. I'll merge the branch once it's fixed.

This has been fixed in MoarVM 80be0cf.

[p6rt]

@hoelzro - Status changed from 'open' to 'resolved'