Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling a sub that uses a subsignature leaks memory #4590

Closed
p6rt opened this issue Sep 25, 2015 · 15 comments
Closed

Calling a sub that uses a subsignature leaks memory #4590

p6rt opened this issue Sep 25, 2015 · 15 comments

Comments

@p6rt
Copy link

p6rt commented Sep 25, 2015

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

Searchable as RT126183$

@p6rt
Copy link
Author

p6rt commented Sep 25, 2015

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
Copy link
Author

p6rt commented Sep 25, 2015

From @hoelzro

memcheck-test.pl

@p6rt
Copy link
Author

p6rt commented Sep 25, 2015

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
Copy link
Author

p6rt commented Sep 25, 2015

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
Copy link
Author

p6rt commented Sep 25, 2015

From @hoelzro

plot-arena-size.m

@p6rt
Copy link
Author

p6rt commented Sep 25, 2015

From @hoelzro

plot.png

@p6rt
Copy link
Author

p6rt commented Sep 26, 2015

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
Copy link
Author

p6rt commented Sep 28, 2015

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
Copy link
Author

p6rt commented Sep 28, 2015

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
Copy link
Author

p6rt commented Sep 29, 2015

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
Copy link
Author

p6rt commented Sep 30, 2015

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

@p6rt
Copy link
Author

p6rt commented Sep 30, 2015

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
Copy link
Author

p6rt commented Sep 30, 2015

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

@p6rt
Copy link
Author

p6rt commented Nov 18, 2015

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
Copy link
Author

p6rt commented Nov 18, 2015

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

@p6rt p6rt closed this as completed Nov 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant