perlscalar morph code

1 view
Skip to first unread message

Nicholas Clark

unread,
Apr 12, 2005, 12:07:50 PM4/12/05
to perl6-i...@perl.org
I'm trying to understand how morph works. perlscalar's morph looks like this:

void morph (INTVAL type) {
if (SELF->vtable->base_type == type)
return;
if (SELF->vtable->base_type == enum_class_PerlString) {
PObj_custom_mark_CLEAR(SELF);
SELF->vtable = Parrot_base_vtables[type];
return;
}
if (type == enum_class_PerlString) {
/*
* if we morph to a string, first clear str_val
* so that after changing the vtable a parallel
* reader doesn't get a gargabe pointer
*/
PMC_str_val(SELF) = NULL;
PObj_custom_mark_SET(SELF);
SELF->vtable = Parrot_base_vtables[type];
return;
}
if (type == enum_class_BigInt || type == enum_class_Complex) {
PMC_str_val(SELF) = NULL;
SELF->vtable = Parrot_base_vtables[type];
DYNSELF.init();
return;
}
SELF->vtable = Parrot_base_vtables[type];
}


I think that there are 2 bugs here

1: Morphing from enum_class_PerlString to enum_class_BigInt or
enum_class_Complex looks broken. The return in the second if clause will
quit the function and the DYNSELF.init() will never get called.

Can anyone easily write a regression test that demonstrates this

I think that the cure is to remove

SELF->vtable = Parrot_base_vtables[type];
return;

An optimisation on top would be to change the following

if (type == enum_class_PerlString) {

to

else if (type == enum_class_PerlString) {

2: The code isn't thread safe, even though it thinks that it is:

if (type == enum_class_PerlString) {
/*
* if we morph to a string, first clear str_val
* so that after changing the vtable a parallel
* reader doesn't get a gargabe pointer
*/
PMC_str_val(SELF) = NULL;
PObj_custom_mark_SET(SELF);
SELF->vtable = Parrot_base_vtables[type];
return;
}


I see no write barrier. There's no reason why on a dual CPU machine the
parallel reader doesn't see the writes to memory in the order:


SELF->vtable = Parrot_base_vtables[type];
PMC_str_val(SELF) = NULL;

and manage go SEGV between the two.

Nicholas Clark

Simon Glover

unread,
Apr 12, 2005, 4:01:28 PM4/12/05
to Nicholas Clark, perl6-i...@perl.org

On Tue, 12 Apr 2005, Nicholas Clark wrote:

> I think that there are 2 bugs here
>
> 1: Morphing from enum_class_PerlString to enum_class_BigInt or
> enum_class_Complex looks broken. The return in the second if clause will
> quit the function and the DYNSELF.init() will never get called.
>
> Can anyone easily write a regression test that demonstrates this

This code:

new P0, .PerlString
new P1, .BigInt
typeof I1, P1
morph P0, I1
set P0, 123
print P0
end

segfaults for me with the latest snapshot of parrot, but prints '123' as
expected if you remove the two lines mentioned below. The analogous test
with Complex instead of BigInt appears to work regardless of whether the
lines are present or not.

> I think that the cure is to remove
>
> SELF->vtable = Parrot_base_vtables[type];
> return;
>

I've also re-run the test suite with these lines removed, and all
existing tests still pass.

Hope this is useful,
Simon

Leopold Toetsch

unread,
Apr 13, 2005, 2:59:37 AM4/13/05
to Nicholas Clark, perl6-i...@perl.org
Nicholas Clark <ni...@ccl4.org> wrote:
> I'm trying to understand how morph works. perlscalar's morph looks like this:

[ broken code ]

> I think that there are 2 bugs here

At least, yes. The better and general morph code is pmc.c:pmc_reuse().

> 2: The code isn't thread safe, even though it thinks that it is:

> I see no write barrier. There's no reason why on a dual CPU machine the


> parallel reader doesn't see the writes to memory in the order:

Yep.

> Nicholas Clark

leo

Nick Glencross

unread,
Apr 13, 2005, 5:02:16 AM4/13/05
to perl6-i...@perl.org
Leopold Toetsch wrote:

>Nicholas Clark <ni...@ccl4.org> wrote:
>
>
>>I'm trying to understand how morph works. perlscalar's morph looks like this:
>>
>>
>
>[ broken code ]
>

On a semi-related note, there's some broken morph code in scalar. It has
code like:

void increment () {
PMC_int_val(SELF) = DYNSELF.get_integer() + 1;
}

which is then used by subclasses String, Integer and Float.

In the case of String, the get_integer succeeds, increments it and
correctly puts it back, hanging off as an integer. However, the PMC's
not been morphed to an Integer, so the print in the next snippet core
dumps (as the integer is a union with the string address):

.sub test
new $P0, .String
$P0 = "12"
inc $P0
print $P0
print_newline
.end

Does this overlap what's being looked at, or shall I provide a patch?

Nick

Nick Glencross

unread,
Apr 13, 2005, 6:27:53 AM4/13/05
to l...@toetsch.at, Perl 6 Internals
Leopold Toetsch wrote:

> I'd say that plain String PMCs don't have increment and decrement.
>
>It seems best to just remove scalar.increment and .decrement, which then
>would automatically create the default_increment for Strings.
>
Sounds best, doesn't it? It's a bit 'perlish' to inc/dec a string
directly. If you know that it's a number, then you'd transfer to a int
register/pmc before the inc.

I bet that you're already onto it... (if not, let me know, and I'll come
up with a patch)

Nick

Leopold Toetsch

unread,
Apr 13, 2005, 5:30:40 AM4/13/05
to Nick Glencross, perl6-i...@perl.org
Nick Glencross <ni...@glencros.demon.co.uk> wrote:

> On a semi-related note, there's some broken morph code in scalar. It has
> code like:

> void increment () {
> PMC_int_val(SELF) = DYNSELF.get_integer() + 1;
> }

> which is then used by subclasses String, Integer and Float.

Integer and Float have their own implementation. But String inherits
this function, which is wrong, yep.

> In the case of String, the get_integer succeeds,

I'd say that plain String PMCs don't have increment and decrement.

It seems best to just remove scalar.increment and .decrement, which then
would automatically create the default_increment for Strings.

> Nick

leo

Leopold Toetsch

unread,
Apr 13, 2005, 8:33:50 AM4/13/05
to Nick Glencross, perl6-i...@perl.org

No, patches welcome - thanks.

> Nick

leo

Reply all
Reply to author
Forward
0 new messages