Ticket #185 (closed bug: fixed)

Opened 5 years ago

Last modified 5 years ago

thawed subs with :vtable don't register properly for existing classes

Reported by: jhorwitz Owned by: jhorwitz
Priority: normal Milestone:
Component: core Version:
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description (last modified by jhorwitz) (diff)

If a class is created before load_bytecode thaws a :vtable sub for that class, parrot tries to override a vtable method with the same name as the thawed sub. This blows up when the sub name is not a valid vtable function, which is common in Rakudo. Everything works in fine in pure PIR.

At first glance, the culprit appears to be add_vtable_override() in src/pmc/class.pmc, which only uses the sub's name to check if it's a valid override. Shouldn't it also check the thawed sub's vtable_index?

This breaks mod_perl6, as mod_parrot needs to load P6object before loading Rakudo's perl6.pbc.

Test case follows.

bar.pir:

.sub '__onload' :load
    $P0 = newclass ['Bar']
.end

foo.pir:

.sub '__onload' :load
    load_bytecode 'bar.pbc'
.end

.namespace ['Bar']

.sub 'invalidname' :vtable('get_string') :method
    $S0 = 'hello world'
    .return($S0)
.end

baz.pir:

.sub main :main
    load_bytecode 'bar.pbc'
    load_bytecode 'foo.pbc'
.end

Output:

[jeff@groovy parrot]$ ./parrot -o foo.pbc foo.pir
[jeff@groovy parrot]$ ./parrot -o bar.pbc bar.pir
[jeff@groovy parrot]$ ./parrot baz1.pir
'invalidname' is not a valid vtable function name.
current instr.: 'main' pc 2 (baz1.pir:3)

Attachments

parrot-vtable-method.patch Download (3.8 KB) - added by jhorwitz 5 years ago.

Change History

Changed 5 years ago by jhorwitz

  • description modified (diff)

Changed 5 years ago by jhorwitz

Changed 5 years ago by jhorwitz

  • owner set to jhorwitz
  • status changed from new to assigned

The attached patch fixes the problem and all tests pass, but I'd like a review just in case there are unintended consequences.

The patch checks for a sub's vtable_index and, where appropriate, uses the vtable method's name in lieu of the sub's name. I had to add one additional function, Parrot_get_vtable_name, so I could reliably map the vtable index to the method name (various preprocessor directives prevented me from accessing it from PMC code, and this seemed cleaner anyway).

Changed 5 years ago by allison

Looks like you've got your nesting wrong on line 100 of src/pmc/namespace.pmc. Should be:

if {

if { } else { }

}

Otherwise, thumbs up.

Changed 5 years ago by jhorwitz

  • status changed from assigned to closed
  • resolution set to fixed

The nesting is actually correct, though it's not immediately obvious. We set method_name to the supplied key and leave it alone if everything else is kosher. Perhaps there's a better way to express this, but it works as is.

Patch tweaked for new string API and applied in r37803.

Note: See TracTickets for help on using tickets.