Seeking suggestions on Inline::CPP multiple inheritance bug.

[David Oswald]

Multiple inheritance is broken in Inline::CPP. The bug wasn't
detected by the module's original "11minhrt.t" test code, but as I was
rewriting the code to shift from Test.pm to Test::More (taking
advantage of some of Test::More's additional test features) I
discovered it.

Attached is a copy of a development version of grammar/t/11minhrt.t
which illustrates the bug.

I've been walking through the code with the Perl debugger (not fun,
and so far not productive). I also ran a version of grammar.pm that
dumped a bunch of diagnostic info to the screen (that was quite
informative, found another harmless bug, but didn't help me figure out
this one). I've also looked over the output from BUILD_NOISY as well
as the .xs file that a build creates.

Unfortunately I'm not seeing the needle in the haystack... or the
forest through the trees, whichever the case may be. I even tried the
"go to bed and look at it again tomorrow" trick. :)

Here is the symptom illustrated in semi-code.

my $obj = Child->new(); # Inherits from Parent1 and Parent2, in that order.
say $obj->parent1_method() # Ok.
say $obj->parent2_method() # Bug: same return value as $obj->parent1_method().

So the method that we're trying to inherit from Parent2 is being
resolved or bound as Parent1's method. These methods do not share the
same name, so that's not an issue.

What's interesting (and probably not surprising) is that if the C++
class "Child" is created with Parent2 as the first super, and Parent1
as the second super, then the bug reverses itself:
say $obj->parent1_method() # Bug: same return value as $obj->parent2_method().
say $obj->parent2_method() # Ok.

Two files attached:
11minhrt.t (the code I'm using to tickle this bug)
another.pl (a non-Test::More version that tries to keep it as
simple as possible)

If anyone has the energy and willingness, please run 11minhrt.t and
see what happens. Some theories I might chase down would be even
better! :)

Dave

[Sisyphus]

----- Original Message -----
From: "David Oswald"

Post by David Oswald
I've also looked over the output from BUILD_NOISY as well
as the .xs file that a build creates.

Also take a look at the .c file that gets generated from the .xs file. After
all, it's that .c file that gets compiled, not the .xs file.

For example, in XS(XS_main__Parent2_do_another) I see:

/////////////////////////////////////////////
if (sv_isobject(ST(0)) && (SvTYPE(SvRV(ST(0))) == SVt_PVMG)) {
THIS = (Parent2 *)SvIV((SV*)SvRV( ST(0) ));
}
/////////////////////////////////////////////

and I wonder what would happen if it should instead be looking at ST(1).

For debugging purposes Id' like to see that code become something like:

/////////////////////////////////////////////
if (sv_isobject(ST(0)) && (SvTYPE(SvRV(ST(0))) == SVt_PVMG)) {
printf("OBJECT: %s\n", HvNAME(SvSTASH(SvRV(ST(0)))));
THIS = (Parent2 *)SvIV((SV*)SvRV( ST(0) ));
}
/////////////////////////////////////////////

but I don't know how to implement that change - possibly requires a hack to
ExtUtils::ParseXS.

Anyway, even if that code change is a stupid suggestion ... don't forget to
check the generated .c file !

Cheers,
Rob

[Patrick LeBoutillier]

David,

I'm really no C++ expert, but I think the implementation is not viable
the way it is. If you create a
small C++ program that simulates Inline::CPP like this:

#include <stdio.h>

/* Your Child/Parent classes here "as is" */

int main(int argv, char **argc){
Child *c = new Child() ;
printf("%d\n", c->do_something()) ;
printf("%d\n", c->do_another()) ;

void *x = c ;
printf("%d\n", ((Parent1 *)x)->do_something()) ;
printf("%d\n", ((Parent2 *)x)->do_another()) ;
}

I gives the same error:
51
17
51
51

I think the problem is that you can't cast your pointer to either
Parent1 or Parent2 depending on
your needs because the bytes are not overlapped in memory. In the
definition of the Child object,
the Parent1 stuff is before the Parent2 stuff. To access the Parent2
stuff you probably need to
use an offset on the Child Pointer.

What's happening (I think) is that your are calling do_something()
both times because both methods
are at the same byte offset in there respective parent object. If you
add some member variables in
one of the Parent object before the method definitions you will
probably get crashes because the method
offset will no longer line up. I'm not sure if any of this is clear....

I think the correct way to do this is something like:

printf("%d\n", ((Child *)x)->Parent1::do_something()) ;
printf("%d\n", ((Child *)x)->Parent2::do_another()) ;

but that implies that your Parent functions know about Child, which is
not good...

Anyways, just some stuff to think about, unfortunately I don't have
any suggestions for you...


Patrick

Multiple inheritance is broken in Inline::CPP.  The bug wasn't
detected by the module's original "11minhrt.t" test code, but as I was
rewriting the code to shift from Test.pm to Test::More (taking
advantage of some of Test::More's additional test features) I
discovered it.
Attached is a copy of a development version of grammar/t/11minhrt.t
which illustrates the bug.
I've been walking through the code with the Perl debugger (not fun,
and so far not productive).  I also ran a version of grammar.pm that
dumped a bunch of diagnostic info to the screen (that was quite
informative, found another harmless bug, but didn't help me figure out
this one).  I've also looked over the output from BUILD_NOISY as well
as the .xs file that a build creates.
Unfortunately I'm not seeing the needle in the haystack... or the
forest through the trees, whichever the case may be.  I even tried the
"go to bed and look at it again tomorrow" trick. :)
Here is the symptom illustrated in semi-code.
my $obj = Child->new(); # Inherits from Parent1 and Parent2, in that order.
say $obj->parent1_method() # Ok.
say $obj->parent2_method() # Bug: same return value as $obj->parent1_method().
So the method that we're trying to inherit from Parent2 is being
resolved or bound as Parent1's method.  These methods do not share the
same name, so that's not an issue.
What's interesting (and probably not surprising) is that if the C++
class "Child" is created with Parent2 as the first super, and Parent1
say $obj->parent1_method() # Bug: same return value as $obj->parent2_method().
say $obj->parent2_method() # Ok.
   11minhrt.t (the code I'm using to tickle this bug)
   another.pl (a non-Test::More version that tries to keep it as
simple as possible)
If anyone has the energy and willingness, please run 11minhrt.t and
see what happens.  Some theories I might chase down would be even
better! :)
Dave
--
David Oswald

[David Oswald]

On Sun, Feb 12, 2012 at 9:25 AM, Patrick LeBoutillier

Post by Patrick LeBoutillier
I'm really no C++ expert, but I think the implementation is not viable
the way it is. If you create a
#include <stdio.h>
/* Your Child/Parent classes here "as is" */
int main(int argv, char **argc){
       Child *c = new Child() ;
       printf("%d\n", c->do_something()) ;
       printf("%d\n", c->do_another()) ;
       void *x = c ;
       printf("%d\n", ((Parent1 *)x)->do_something()) ;
       printf("%d\n", ((Parent2 *)x)->do_another()) ;
}
51
17
51
51

Patrick,

Your explanation was excellent.

What needs to be happening is that, as Child inherits from Parent1 and
Parent2, the pointer should be cast as a Child type, like in this
modification of your test:

int main(int argv, char **argc){
Child *c = new Child() ;
printf("%d\n", c->do_something()) ;
printf("%d\n", c->do_another()) ;

void *x = c ;
printf("%d\n", ((Child *)x)->do_something()) ;
printf("%d\n", ((Child *)x)->do_another()) ;
}

...or in more idiomatic C++:

int main() {
using std::cout;
using std::endl;

Child *c = new Child() ;
cout << c->do_something() << endl;
cout << c->do_another() << endl;

void *x = c ;
cout << static_cast< Child* >(x)->do_something() << endl;
cout << static_cast< Child* >(x)->do_another() << endl;

return 0;
}

Though I'm not much closer to a solution, at least your message
reminded me that we're dealing with void pointers to objects. Maybe I
need to start looking at how and where the casting is being
accomplished.

Dave