Incorrect values & MemoryError with protected variable and cross-inheritance

[Vipul-Cariappa]

Python Code to reproduce the bug

import cppyy
from cppyy import gbl


cppyy.cppdef("""
class MyBaseClass {
protected:
    int x = 0;
    std::string v = "vipul";
    int y = 0;
    std::string c = "cariappa";
    int z = 0;
public:
    MyBaseClass(int x, int y, int z) : x(x), y(y), z(z) {}
    int get_x() { return x; }
    int get_y() { return y; }
    int get_z() { return z; }
};
""")

class MyDerivedClass(gbl.MyBaseClass):
    def __init__(self, x, y, z):
        super(MyDerivedClass, self).__init__(x, y, z)

derived = MyDerivedClass(5, 7, 9)
print(f"{derived.get_x() = }")
print(f"{derived.x = }")
print(f"{derived.get_y() = }")
print(f"{derived.y = }")
print(f"{derived.get_z() = }")
print(f"{derived.z = }")
# print(f"{derived.v = }") # BUG: Memory Error
# print(f"{derived.c = }")

Output:

/home/vipul-cariappa/Workspace/cpp-py/compiler-research/./tmp/bug8.py:24: RuntimeWarning: class "MyBaseClass" has no virtual destructor
  class MyDerivedClass(gbl.MyBaseClass):
derived.get_x() = 5
derived.x = 1276809352
derived.get_y() = 7
derived.y = 0
derived.get_z() = 9
derived.z = 0
Traceback (most recent call last):
  File "/home/vipul-cariappa/Workspace/cpp-py/compiler-research/./tmp/bug8.py", line 35, in <module>
    print(f"{derived.v = }") # BUG: Memory Error
            ^^^^^^^^^^^^^^
MemoryError

You can see that derived.get_x() and derived.x returns different values.

[wlav]

As the warning says:

/home/vipul-cariappa/Workspace/cpp-py/compiler-research/./tmp/bug8.py:24: RuntimeWarning: class "MyBaseClass" has no virtual destructor
  class MyDerivedClass(gbl.MyBaseClass):

Add one, i.e. virtual ~MyBaseClass() {}, and you should be good.

I forget why it's just a warning. The git log shows that it used to be an error, so I guess someone asked for it to just be a warning. Maybe the warning should be refined to an error if the base class has data, but even in C++ the above code would be problematic, so I don't expect it to be common.

[Vipul-Cariappa]

Hi @wlav, please look at this example:

cppyy.cppdef("""
class BaseA {
protected:
    int Ai;
    std::string As;
public:
    BaseA(int i, std::string s) : Ai(i), As(s) {}
    virtual ~BaseA() {}
    int get_Ai() { return Ai; }
    std::string get_As() { return As; }
};

class BaseB {
protected:
    std::string Bs;
    int Bi;
public:
    BaseB(int i, std::string s) : Bi(i), Bs(s) {}
    virtual ~BaseB() {}
    int get_Bi() { return Bi; }
    std::string get_Bs() { return Bs; }
};
""")

class Klass(cppyy.multi(gbl.BaseA, gbl.BaseB)):
    def __init__(self, int1, int2, str1, str2):
        super().__init__((int1, str1), (int2, str2))

k = Klass(5, 10, "Cpp", "Python")
print(f"{k.get_Ai() = }\t{k.Ai = }")
print(f"{k.get_Bi() = }\t{k.Bi = }")
print(f"{k.get_As() = }\t{k.As = }")
print(f"{k.get_Bs() = }\t{k.Bs = }")

Output:

k.get_Ai() = 5  k.Ai = 5
k.get_Bi() = 10 k.Bi = 1766142531
k.get_As() = b'Cpp'     k.As = b'Cpp'
Traceback (most recent call last):
  File "/home/vipul-cariappa/Workspace/cpp-py/compiler-research/./tmp/bug9.py", line 40, in <module>
    print(f"{k.get_Bs() = }\t{k.Bs = }")
MemoryError

I believe this is a genuine bug?

[wlav]

Yes, in as much. Still, you should shoot anyone who writes code like that. ;) See e.g.: https://isocpp.org/wiki/faq/multiple-inheritance#mi-disciplines

IIRC, the base address used is the one of the stub, so most likely the problem is with the relative offset from there (and hence the problem with the 2nd class, with the first being fine as the in-class offset is 0 for it). Will have a look later; having a break for a bit now that the new release is just out.

[wlav]

The offsets as I get them from ROOT/meta are off (to be sure, upstream ROOT/meta doesn't support using of data members, so it's a local change), but basically the variables are all only available on the stub class (since that's where they're public through the using), while the offsets are calculated from the base class. This is b/c the decl stored with the Cling DataMemberInfo is the target decl, not the using decl.

[Vipul-Cariappa]

I faced a similar problem at the compiler-research's fork. If it is to do with offsets; I fixed it with compiler-research/CppInterOp#396. May help.

We are storing the using decl itself.

[wlav]

Thanks, but it's a wee bit simpler in my case: the GetDecl() method would take the target instead of the using decl, as that fit better with the pre-existing code before using support was added:

const clang::Decl* TClingDataMemberInfo::GetDecl() const {
   if (const clang::Decl* SingleDecl = TClingDeclInfo::GetDecl())
      return SingleDecl;
   if (clang::UsingShadowDecl* shadow_decl = llvm::dyn_cast<clang::UsingShadowDecl>(*fIter))
      return shadow_decl->getTargetDecl();
   return *fIter;
}

The problem here being that the target decl gives the incorrect offset.

There was already a special case using the UsingShadowDecl directly for determining access (see TClingDataMemberInfo::Property()). For exampl, if the using is public, the property would still be flagged kIsPublic, regardless the access of the target.

I just did the same for the offset calculation: add a special case to use the UsingShadowDecl rather than the target. This gives me the correct offsets.