[perl #22767] IMCC/Parrot leak and eventual segfault (partially solved)
Clinton A. Pierce
># New Ticket Created by "Clinton A. Pierce"
># Please include the string: [perl #22767]
># in the subject line of all future correspondence about this issue.
># <URL: http://rt.perl.org/rt2/Ticket/Display.html?id=22767 >
>
>
>I apologize for the length of this example. I've spent a goodly part of
>this afternoon trying to track this one down and can't get a handle on a
>smaller example.
Found the bug. Mostly MEA CULPA. A thousand pardons to the good Parrot folk.
When calling a sub like this:
.arg 0
call _foo
It's probably a good thing to take the 0 off the stack at some
point. Tends to help with the crashes. That was my bug. Interestingly
enough though there still lurks a bug in Parrot somewhere. Here's a much
smaller test case:
.sub _main
$P0=new PerlHash
$P0["value"]=0
store_global "PRINTCOL", $P0
USERLABEL_0
.arg 0
call _POSIX_INKEY
.arg 1.0
.arg 1
call _BUILTIN_DISPLAY
branch USERLABEL_0
end # (unreached)
.end
.sub _BUILTIN_DISPLAY
saveall
.param int argc
.local int PRINTCOL
.param float number
find_global $P0, "PRINTCOL"
set PRINTCOL, $P0["value"]
set $P0["value"], PRINTCOL
store_global "PRINTCOL", $P0
restoreall
ret
.end
.sub _POSIX_INKEY
saveall
$P0=new PerlHash
global "fcntl_mode" = $P0
restoreall
ret
.end
This too will segfault after a while. What's going on is the mail loop
(USERLABEL_0) calls POSIX_INKEY and an extra INT gets left on the
stack. After a while, these build up and something goes haywire and *crunch*.
But watch this:
.sub _main
LOOP:
.arg 0
call _foo
branch LOOP
.end
.sub _foo
ret
.end
This suffers from the same logic bug (leaving an extra INT on the stack for
each sub call) but *PARROT* *DOESN'T* *CRASH*. Leaks like hell, sure, but
no crashes. The something about the find/store global or the new PerlHash
in the loop causes the actual crash to happen.
[Although a suggestion. After the crash bug gets fixed, a "stack overflow"
error or some such nonsense being emitted from the VM might be a good
idea. Knowing that:
LOOP: push 0
branch LOOP
will eventually cause a segfault makes me pine for my 6502 where:
LOOP PHA
JMP LOOP
Would cause the SP to spin like a top all over Page 1. Over and over and
over and over... No crash, just infinite fun. When the stack can't grow a
nice message would help.]
Leopold Toetsch
> Found the bug. Mostly MEA CULPA. A thousand pardons to the good Parrot folk.
> When calling a sub like this:
> .arg 0
> call _foo
> It's probably a good thing to take the 0 off the stack at some
> point.
Thanks again for your bug report and thorough checking all kind of
parrot limits.
I've added a check for too deeply nested stacks now. Your first test
program now bails out at:
25342
Stack 'User' too deep
The limit is currently fixed at 100 chunks, but could easily be changed
with a new opcode, e.g. stack_limit <.Stack>, limit.
The stack_height() call could also be optimized then as well as other
code pieces which are walking the stack from the base.
leo
Dan Sugalski
>Clinton A. Pierce <cli...@geeksalad.org> wrote:
>
>> Found the bug. Mostly MEA CULPA. A thousand pardons to the good
>>Parrot folk.
>
>> When calling a sub like this:
>
>> .arg 0
>> call _foo
>
>> It's probably a good thing to take the 0 off the stack at some
>> point.
>
>Thanks again for your bug report and thorough checking all kind of
>parrot limits.
>
>I've added a check for too deeply nested stacks now. Your first test
>program now bails out at:
>
> 25342
>Stack 'User' too deep
>
>The limit is currently fixed at 100 chunks, but could easily be changed
>with a new opcode, e.g. stack_limit <.Stack>, limit.
This sounds like a good option, though 100 might be a bit small,
since I plan on giving new frames on pushes to COW'd stack chunks.
(No reason not to (well, besides the performance hit) and it makes
some coroutine stuff easier)
I probably ought to get started on the stack-chunk-as-PMC patch for
garbage collection of stack frames. :)
--
Dan
--------------------------------------"it's like this"-------------------
Dan Sugalski even samurai
d...@sidhe.org have teddy bears and even
teddy bears get drunk
Leopold Toetsch
> At 10:33 AM +0200 6/24/03, Leopold Toetsch wrote:
>
>> I've added a check for too deeply nested stacks now.
> I probably ought to get started on the stack-chunk-as-PMC patch for
> garbage collection of stack frames. :)
The first question is:
Do we need such a limit check on the register backing stacks too?
Second is efficiency on copying COWed stacks. Currently only Control,
Pad, and User stacks are lazy copied, whole stack at once and not
chunkwise. So it would make sense to have stack chunks derived from
buffer-like objects, which can be unCOWed per chunk.
But this would need some rework in the COW GC code, which currently
assumes that only strings might be COWed.
WRT coroutines: They have new_stack()s for Pad, User, and Control. The
latter is probably wrong due to the planned exception mechanism. I
presume that a coroutine works like a subroutine WRT exceptions, so that
some parent can catch an exception that has originated in a coroutine.
leo
Dan Sugalski
><x-flowed>Dan Sugalski wrote:
>
>> At 10:33 AM +0200 6/24/03, Leopold Toetsch wrote:
>>
>>> I've added a check for too deeply nested stacks now.
>
>> I probably ought to get started on the stack-chunk-as-PMC patch for
>> garbage collection of stack frames. :)
>
>The first question is:
>Do we need such a limit check on the register backing stacks too?
If we're going to put limits in, yes.
>Second is efficiency on copying COWed stacks. Currently only Control,
>Pad, and User stacks are lazy copied, whole stack at once and not
>chunkwise. So it would make sense to have stack chunks derived from
>buffer-like objects, which can be unCOWed per chunk.
>But this would need some rework in the COW GC code, which currently
>assumes that only strings might be COWed.
I could've sworn that the stacks were taken piece by piece when
COWed, but apparently not. They should be, though, as should the
register stacks. No point in copying more data than we need to when
we touch the things.
>WRT coroutines: They have new_stack()s for Pad, User, and Control. The
>latter is probably wrong due to the planned exception mechanism. I
>presume that a coroutine works like a subroutine WRT exceptions, so that
>some parent can catch an exception that has originated in a coroutine.
Co-routines are going to have to be dealt with oddly, since their
stacks need to reattach themselves to the caller's stack (at least
the control stack) on every invocation. That is, the bottom of the
coroutine's stack needs to attach to the top of the current stack
every time the coroutine is invoked, which can be... fun. Especially
if we need to deal with the possibility of multiple threads invoking
the same coroutine, something that just occurred to me that I don't
muck like the implications of.
Leopold Toetsch
> At 8:26 AM +0200 6/25/03, Leopold Toetsch wrote:
>>The first question is:
>>Do we need such a limit check on the register backing stacks too?
> If we're going to put limits in, yes.
If we want some secure executions modes too, we will need this anyway.
The limit check in the 3 stacks I had put in is in the slow paths only.
The same will work for the register stacks.
( /me again mumbling why we have 6 different stack implementations with
a lot of duplicate code. One general stack with different sized data
items would do it too - with some overhead ;-)
> I could've sworn that the stacks were taken piece by piece when
> COWed, but apparently not. They should be, though, as should the
> register stacks. No point in copying more data than we need to when
> we touch the things.
Yes, comments do state this. So we need general COWable buffers plus
some rework in stack code.
>>WRT coroutines: They have new_stack()s for Pad, User, and Control. The
>>latter is probably wrong due to the planned exception mechanism. I
>>presume that a coroutine works like a subroutine WRT exceptions, so that
>>some parent can catch an exception that has originated in a coroutine.
> Co-routines are going to have to be dealt with oddly, since their
> stacks need to reattach themselves to the caller's stack (at least
> the control stack) on every invocation. That is, the bottom of the
> coroutine's stack needs to attach to the top of the current stack
> every time the coroutine is invoked, which can be... fun.
This ought to be the same as not to have a separate control stack (as I
mentioned above), but put a COWed copy of the callers control stack in
place - at call time probably.
> ... Especially
> if we need to deal with the possibility of multiple threads invoking
> the same coroutine, something that just occurred to me that I don't
> muck like the implications of.
I'm thinking of multithreading Parrot currently a lot and implications
on some data structures. If you have any thouhgts floating around or
even some finished bits please spit those out.
leo
Matt Fowles
> ( /me again mumbling why we have 6 different stack implementations with
> a lot of duplicate code. One general stack with different sized data
> items would do it too - with some overhead ;-)
Were this C++ I would say that we could write a single general purpose
stack and use template meta-programming to avoid the overhead. Is there
a similar solution available in C?
My instincts tell me that this solution will be dirty to the tune of
massive macroing, but perhaps someone better with pure C than I am could
provide a better answer.
Matt
Dave Whipp
> Were this C++ I would say that we could write a single general purpose
> stack and use template meta-programming to avoid the overhead. Is there
> a similar solution available in C?
>
> My instincts tell me that this solution will be dirty to the tune of
> massive macroing, but perhaps someone better with pure C than I am could
> provide a better answer.
C does do templates, sort-of:
#define STACK_TYPE int
#define STACK_MAX_SIZE 1024
#include stack_template_decl.h
#include stack_template_impl.h
#undef STACK_TYPE
#undef STACK_MAX_SIZE
(often one instances the stack_decl in a header file, and the _impl in a .c
file.) I've also seen the calling convention where the template files do the
#undefs.
There can be some issues debugging this stuff though: gcc will give currect
line numbers, but will not tell you which instance of the stack template
you're in. So if a bug is only in, say, stacks of double: then this won't be
immediately apparent. On the plus side, the lack of type information means
that you don't get c++'s 8000-character error messages.
Dave.
Leopold Toetsch
> C does do templates, sort-of:
> #define STACK_TYPE int
> #define STACK_MAX_SIZE 1024
> #include stack_template_decl.h
> #include stack_template_impl.h
> #undef STACK_TYPE
> #undef STACK_MAX_SIZE
> There can be some issues debugging this stuff though:
And this is the big problem with it.
I think, we should have a general stack engine which just knows about
item size and chunk size. The base functions return a (void *) to an
entry.
The fast paths (now chunk add/del needed) could even be inlined. A
separate handling of the entries like in stacks.c could be on top of
this.
> Dave.
leo
Dan Sugalski
>Dan Sugalski <d...@sidhe.org> wrote:
>> At 8:26 AM +0200 6/25/03, Leopold Toetsch wrote:
>
>>>The first question is:
>>>Do we need such a limit check on the register backing stacks too?
>
>> If we're going to put limits in, yes.
>
>If we want some secure executions modes too, we will need this anyway.
>The limit check in the 3 stacks I had put in is in the slow paths only.
>The same will work for the register stacks.
Works.
>( /me again mumbling why we have 6 different stack implementations with
>a lot of duplicate code. One general stack with different sized data
>items would do it too - with some overhead ;-)
Well... we only really have three. The user and control stack are (or
should be) the same, which is fine as they're
single-element-with-pop-action stacks. The I/P/N/S stacks are all the
same, the only reason they've got different code is because the
elements are of different types and sizes. Macro-izing them would
probably be in order, and the only real reason I didn't was that I
was in too much of a hurry to do so. The integer stack's
special-purpose, so that makes sense as well.
> > I could've sworn that the stacks were taken piece by piece when
>> COWed, but apparently not. They should be, though, as should the
>> register stacks. No point in copying more data than we need to when
>> we touch the things.
>
>Yes, comments do state this. So we need general COWable buffers plus
>some rework in stack code.
Yep.
> >>WRT coroutines: They have new_stack()s for Pad, User, and Control. The
>>>latter is probably wrong due to the planned exception mechanism. I
>>>presume that a coroutine works like a subroutine WRT exceptions, so that
>>>some parent can catch an exception that has originated in a coroutine.
>
>> Co-routines are going to have to be dealt with oddly, since their
>> stacks need to reattach themselves to the caller's stack (at least
>> the control stack) on every invocation. That is, the bottom of the
>> coroutine's stack needs to attach to the top of the current stack
>> every time the coroutine is invoked, which can be... fun.
>
>This ought to be the same as not to have a separate control stack (as I
>mentioned above), but put a COWed copy of the callers control stack in
>place - at call time probably.
But calling a coroutine may involve putting the coroutine's stack in
place as well, which is where the interesting bits come in.
> > ... Especially
>> if we need to deal with the possibility of multiple threads invoking
>> the same coroutine, something that just occurred to me that I don't
>> muck like the implications of.
>
>I'm thinking of multithreading Parrot currently a lot and implications
>on some data structures. If you have any thouhgts floating around or
>even some finished bits please spit those out.
Some thoughts, but I keep redoing them. We can chat at YAPC::EU about
it, I think.
Leopold Toetsch
[ stack implementations ]
> Well... we only really have three.
Control, User & Pad have the same stack engine. The register backing
stacks and rxstack sum up to 5 more.
> ... The I/P/N/S stacks are all the
> same, the only reason they've got different code is because the elements
> are of different types and sizes. Macro-izing them would probably be in
> order, and the only real reason I didn't was that I was in too much of a
> hurry to do so. The integer stack's special-purpose, so that makes sense
> as well.
I'm, thinking of push/pop subs (or macros) for all stacks that (in case
of subs) are inlined in the normal case, when no additional stack frame
action is necessary. The slower and bigger frame handling part would be
an external sub.
>> Yes, comments do state this. So we need general COWable buffers plus
>> some rework in stack code.
>
> Yep.
Done.
[ Coroutine stack & multithreading ]
> But calling a coroutine may involve putting the coroutine's stack in
> place as well, which is where the interesting bits come in.
Coroutines as other subs might get called from different threads. So the
Sub objects for a sub called from different threads are different
(implying with separate stack items). The return continuation (presuming
a common caller thread) would be the same.
leo
Benjamin Goldberg
> Matt Fowles
>> Were this C++ I would say that we could write a single general
>> purpose stack and use template meta-programming to avoid the
>> overhead. Is there a similar solution available in C?
>>
>> My instincts tell me that this solution will be dirty to the tune of
>> massive macroing, but perhaps someone better with pure C than I am
>> could provide a better answer.
>
> C does do templates, sort-of:
>
> #define STACK_TYPE int
> #define STACK_MAX_SIZE 1024
> #include stack_template_decl.h
> #include stack_template_impl.h
> #undef STACK_TYPE
> #undef STACK_MAX_SIZE
>
> (often one instances the stack_decl in a header file, and the _impl in
> a .c file.) I've also seen the calling convention where the template
> files do the #undefs.
>
> There can be some issues debugging this stuff though: gcc will give
> currect line numbers, but will not tell you which instance of the
> stack template you're in. So if a bug is only in, say, stacks of
> double: then this won't be immediately apparent.
Any chance that this could be fixed with a #line pragma, with a filename
which is generated from the parameters? E.g., something like:
#define STRINGIFY(X) ??? /* make a string literal from a token */
#define STACK_TYPE_STR STRINGIFY(STACK_TYPE)
#define STACK_SIZE_STR STRINGIFY(STACK_MAX_SIZE)
#define GENFILENAME \
__LINE__ "[" STACK_TYPE_STR "x" STACK_SIZE_STR "]"
#line 0 GENFILENAME
> On the plus side, the lack of type information means that you don't
> get c++'s 8000-character error messages.
--
$a=24;split//,240513;s/\B/ => /for@@=qw(ac ab bc ba cb ca
);{push(@b,$a),($a-=6)^=1 for 2..$a/6x--$|;print "$@[$a%6
]\n";((6<=($a-=6))?$a+=$_[$a%6]-$a%6:($a=pop @b))&&redo;}
Dan Sugalski
>Dan Sugalski wrote:
>[ stack implementations ]
>
>Well... we only really have three.
>
>Control, User & Pad have the same stack engine. The register backing
>stacks and rxstack sum up to 5 more.
Pads shouldn't really be stacks, they should be plain linked lists.
The register backing stacks, while implemented separately (which is
just wrong) are really identical--the only difference is the size and
type of the frames being stored. I'd have macro'd them up, but as I
said I didn't quite have time, and it took less brain power to
cut-n-paste the code. Sad, but true. The integer stack's separate on
purpose, as it's supposed to be a high-speed thing for the regex
engine to use, at least for the most part.
That leaves us three stack types: control/user, register backing, and
integer stacks. The implementation of the backing stacks should be
combined up into one chunk of macro's code so there won't be problems
with cut and paste errors.
Pads and namespaces are singly-linked lists, which we need to deal
with separately.
>>... The I/P/N/S stacks are all the same, the only reason they've
>>got different code is because the elements are of different types
>>and sizes. Macro-izing them would probably be in order, and the
>>only real reason I didn't was that I was in too much of a hurry to
>>do so. The integer stack's special-purpose, so that makes sense as
>>well.
>
>I'm, thinking of push/pop subs (or macros) for all stacks that (in
>case of subs) are inlined in the normal case, when no additional
>stack frame action is necessary. The slower and bigger frame
>handling part would be an external sub.
Inlining doesn't always buy you anything and sometimes makes things
worse. Counter-intuitive, but nonetheless true. We'd be better off
with a set of no-check push/pop ops that don't check on a whole lot
of the stuff the ops do now, since pushing a register frame or
control stack thing would be much faster if we didn't have to check
for over/underflow or COW state.
>>>Yes, comments do state this. So we need general COWable buffers plus
>>>some rework in stack code.
>>
>>Yep.
>
>Done.
Keen, thanks.
>[ Coroutine stack & multithreading ]
>
>>But calling a coroutine may involve putting the coroutine's stack
>>in place as well, which is where the interesting bits come in.
>
>Coroutines as other subs might get called from different threads. So
>the Sub objects for a sub called from different threads are
>different (implying with separate stack items). The return
>continuation (presuming a common caller thread) would be the same.
I'm not sure yet how we're going to handle that, whether there'll be
a lock taken on the coroutine PMC, or whether invoking it should COW
the continuation in the coroutine. Arguably multiple threads
shouldn't be able to be inside the same version of a coroutine at
once.
Leopold Toetsch
> Pads shouldn't really be stacks, they should be plain linked lists.
A plain linked list is lacking the chunk allocation scheme. I'm not sure
if allocating a buffer_like and Parrot_allocate the memory for the pads
is better, then the current stack based implementation.
But I still don't know, how control stack, exception objects located
there, and scopes are playing together.
> Pads and namespaces are singly-linked lists, which we need to deal
> with separately.
WRT namespaces: pdd06 has named global tables and indexed access for
globals in such a table. This is similar to the indexed access in
lexical pads. The implementation of both is totally different though.
First we should know, if $HL is likely to access lexicals/globals by
name or by index or mixed ...
Then such a pad or namespace table probably should be a separate PMC
class with a hash and an array inside. So manipulation of these could be
done with normal opcodes + some special shortcut ops for common
operations.
leo
Steve Fink
> At 11:43 PM +0200 6/29/03, Leopold Toetsch wrote:
> >Dan Sugalski wrote:
> >[ stack implementations ]
> >
> >Well... we only really have three.
> >
> >Control, User & Pad have the same stack engine. The register backing
> >stacks and rxstack sum up to 5 more.
>
> high-speed thing for the regex engine to use, at least for the most
> part.
The integer stack is fairly useless for a complete regex engine. It
prevents reentrancy and complicates some of the trickier things that
need to be done in regexes. We can drop it anytime, and instead use an
integer-only array PMC instead. (Such a PMC exists and is called
"intlist.pmc", and uses the same stack engine as the control, user,
and pad stacks.)
I would remove the rxstack, but I didn't want to break the closest
thing we have to a regex engine until I or someone else managed to
release a different working implementation. The particular technique
of using a single stack tied directly the interpreter is an
evolutionary dead end, however. Good for a proof of concept, but
that's it.
Leopold Toetsch
> I would remove the rxstack, but I didn't want to break the closest
> thing we have to a regex engine until I or someone else managed to
> release a different working implementation. The particular technique
> of using a single stack tied directly the interpreter is an
> evolutionary dead end, however. Good for a proof of concept, but
> that's it.
Yep. Lets keep it, until the real rx engine is in.
leo