Re: [perl #41132] [BUG] Segfault in Parrot_call_sub_ret_int
Nikolay Ananiev
# Please include the string: [perl #41132]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=41132 >
Check the attached examples.
Jonathan Worthington
> The attached patch fixes the problem. The code is taken from Parrot_call_sub (which works). But I don't know why it was not applied to the other two function. Maybe it's just a hack?
>
Perhaps, but I have applied it since it's consistent with
Parrot_call_sub. I'd like to poke into this at bit deeper at the moment,
but would rather have working code now. :-)
If you have time, please can you take a look at Parrot_call_method stuff
beneath the call sub functions and see if they need a similar fix.
I am leaving this ticket open until I or @other digs deeper into whether
there's a better answer to this, and we look into Parrot_call_method.
Thank you,
Jonathan
Jonathan Worthington
> I'd like to poke into this at bit deeper at the moment,
in as.
Don't drink and patch!
Jonathan
Bob Rogers
Date: Tue, 26 Dec 2006 19:36:58 +0000
Nikolay Ananiev wrote:
> The attached patch fixes the problem. The code is taken from
> Parrot_call_sub (which works). But I don't know why it was not
> applied to the other two function. Maybe it's just a hack?
Perhaps, but I have applied it since it's consistent with
Parrot_call_sub. I'd like to poke into this a bit deeper . . .
I am leaving this ticket open until I or @other digs deeper into whether
there's a better answer to this, and we look into Parrot_call_method.
Thank you,
Jonathan
AFAICS, the old code failed because Parrot_switch_to_cs (which is what
normally sets ctx->contants in a sub call) is not given a chance to run
in this situation. Parrot_loadbc sets interp->code without doing any of
the other Parrot_switch_to_cs bookeeping. Sub:invoke subsequently finds
that sub->seg is already installed properly in interp->code, so it skips
calling Parrot_switch_to_cs, leaving ctx->contants uninitialized.
Dropping the interp->code assignment in Parrot_loadbc sorta works
(after telling RetContinuation:invoke not to panic if no returning code
segment), but causes other failures. Replacing the assignment with a
Parrot_switch_to_cs call fails immediately; this might be made to work,
but it would be setting the constants in the old context, just as
Nikolay's patch (and Parrot_call_sub) do; this seems wrong.
The attached patch changes Sub:invoke to call Parrot_switch_to_cs
unconditionally. This seems to work without breaking anything else, and
also allows all these other extracurricular assignments to
ctx->constants to go away. But there may be something cleaner still
(and the code works now), so I too will wait to hear from @other (I
suspect they are mostly on vacation -- ;-).
-- Bob Rogers
http://rgrjr.dyndns.org/
chromatic
> AFAICS, the old code failed because Parrot_switch_to_cs (which is what
> normally sets ctx->contants in a sub call) is not given a chance to run
> in this situation. Parrot_loadbc sets interp->code without doing any of
> the other Parrot_switch_to_cs bookeeping. Sub:invoke subsequently finds
> that sub->seg is already installed properly in interp->code, so it skips
> calling Parrot_switch_to_cs, leaving ctx->contants uninitialized.
That makes sense. However, I'm curious whether Parrot_loadbc() does the right
thing.
> Dropping the interp->code assignment in Parrot_loadbc sorta works
> (after telling RetContinuation:invoke not to panic if no returning code
> segment), but causes other failures. Replacing the assignment with a
> Parrot_switch_to_cs call fails immediately; this might be made to work,
> but it would be setting the constants in the old context, just as
> Nikolay's patch (and Parrot_call_sub) do; this seems wrong.
I don't understand why; would you consider writing up some rough documentation
on how contexts and interpreters and code segments currently work? Having to
dig in the implementation and guess by reading the code isn't getting me very
far.
> The attached patch changes Sub:invoke to call Parrot_switch_to_cs
> unconditionally. This seems to work without breaking anything else, and
> also allows all these other extracurricular assignments to
> ctx->constants to go away. But there may be something cleaner still
> (and the code works now), so I too will wait to hear from @other (I
> suspect they are mostly on vacation -- ;-).
It's shorter than my attempts (which broke some threading tests, which seems
to indicate what you suggested about old contexts).
-- c
Bob Rogers
Date: Tue, 26 Dec 2006 16:26:03 -0800
On Tuesday 26 December 2006 14:35, Bob Rogers wrote:
> AFAICS, the old code failed because Parrot_switch_to_cs (which is what
> normally sets ctx->contants in a sub call) is not given a chance to run
> in this situation. Parrot_loadbc sets interp->code without doing any of
> the other Parrot_switch_to_cs bookeeping. Sub:invoke subsequently finds
> that sub->seg is already installed properly in interp->code, so it skips
> calling Parrot_switch_to_cs, leaving ctx->contants uninitialized.
That makes sense. However, I'm curious whether Parrot_loadbc() does
the right thing.
It certainly doesn't do much. I'm not sure I understand the point of
the interp->initial_pf slot . . .
> Dropping the interp->code assignment in Parrot_loadbc sorta works
> (after telling RetContinuation:invoke not to panic if no returning code
> segment), but causes other failures. Replacing the assignment with a
> Parrot_switch_to_cs call fails immediately; this might be made to work,
> but it would be setting the constants in the old context, just as
> Nikolay's patch (and Parrot_call_sub) do; this seems wrong.
I don't understand why; would you consider writing up some rough
documentation on how contexts and interpreters and code segments
currently work? Having to dig in the implementation and guess by
reading the code isn't getting me very far.
I can try, but I don't know much about code segments; I'm just digging
in the code myself. But I'll certainly post anything I learn.
-- Bob
Bob Rogers
Date: Tue, 26 Dec 2006 23:14:06 -0500
From: chromatic <chro...@wgz.org>
Date: Tue, 26 Dec 2006 16:26:03 -0800
. . .
I don't understand why; would you consider writing up some rough
documentation on how contexts and interpreters and code segments
currently work? Having to dig in the implementation and guess by
reading the code isn't getting me very far.
I can try, but I don't know much about code segments; I'm just digging
in the code myself. But I'll certainly post anything I learn.
This is not quite to the point, but I did find that Parrot_pop_cs is
unused, which makes the "prev" member of PackFile_ByteCode effectively
unused. Parrot_pop_cs looks like a pre-Continuation legacy, but was
actually added (in r2969) five months after continuation.pmc. Does
anybody object to deleting this code?
-- Bob