[cppyy] New crash when iterating over polymorphic std::vector

[Nowakus]

Check duplicate issues.

• Checked for duplicates

Description

Starting around 24/06/2025 a new crash appeared in 10 years old code where we iterate in python over an std::vector of polymorphic objects created on the C++ side. The inheritance structure is 4 levels deep and contains virtual inheritance that is apparently the reason cppyy trips. The iteration is the faulty part - single object uses work fine.

Reproducer

For now just quick copy/paste to give the overall feel of the situation:
Here's a brief summary of the inheritance hierarchy:

class Analysis::JetFitterTagInfo : public virtual Analysis::IJetFitterTagInfo
class Analysis::IJetFitterTagInfo : public virtual Analysis::BaseTagInfo
class Analysis::BaseTagInfo : public JetTagInfoBase
class JetTagInfoBase : public Analysis::ITagInfo

 class TagInfoTest {   public:
      typedef std::vector<const JetTagInfoBase*>       tagstore_t;
      const tagstore_t makeInfoVector(const char* tag) const {
         tagstore_t store;
         store.push_back( new Analysis::BaseTagInfo(tag) );
         store.push_back( new Analysis::JetFitterTagInfo(tag) );
         return store;
      }
   }; 

>>> import ROOT
>>> for i in ROOT.TagInfoTest().makeInfoVector('bla'):
...   print( i.infoType() )
... 
bla
free(): invalid pointer
 *** Break *** abort

ROOT version

master

Installation method

ATLAS

Operating system

Linux

Additional context

No response

[hahnjo]

Hi @guitargeek @Vipul-Cariappa, I was just chatting with Marcin. Based on the date (June 24, +- 2 days), the most likely candidate is e93464a. For the moment, the problem is worked around on the Athena side, but would be good to identify this regression.

[Vipul-Cariappa]

Hi @Nowakus, could you provide a complete reproducer. Or point me to the implementation of those classes.
I suspect that it is not to do with std::vector iteration. It might have to do with some member field being misinterpreted in cppyy.
But I see free(): invalid pointer in your report. Not sure how cppyy would cause that.

[wlav]

In the code of the commit referenced, depending on ISCONST, the valid pointer lives either in address or in *(void**)address. The added line of the commit, however, always assumes the latter.

Note that GetActualClass is the purview of BindObject. It shouldn't be called here in the converter, but the need for its call (if at all) communicated to the latter (it's an expensive call; having it in 2 places risks it being called twice, once needlessly).

The semantics of this change also look wrong.

[Vipul-Cariappa]

Hi @wlav, apparently this works.
It is an InstancePtrConverter, the object is actually located at *(void**)address.

 template <bool ISCONST>
 PyObject* CPyCppyy::InstancePtrConverter<ISCONST>::FromMemory(void* address)
 {
 // construct python object from C++ instance read at <address>
-    if (ISCONST)
-         return BindCppObject(*(void**)address, fClass);                   // by pointer value
+    return BindCppObject(*(void**)address, fClass);
-    return BindCppObject(address, fClass, CPPInstance::kIsReference);     // modifiable
 }

Another question:

class A {
   public:
    virtual ~A() {}
};

class B : public A {};

A *x = new A;

void change() { x = new B; }

What is the expected type of x before and after change()?

assert type(gbl.x) == gbl.A
gbl.change()
assert type(gbl.x) == gbl.B # else should this be gbl.A?

[Nowakus]

Just to emphasize - the original crash appeared in an iteration and the only way to reproduce the crash by hand was to iterate over a vector like in my code snippet. Also, the workaround on our side was to eliminate the virtual inheritance we had in the model. Apparently these two things together were the trigger.
I should be able to produce a standalone reproducer in a few days.

[guitargeek]

Thank you very much! Having these reproducers is worth a lot to add them as unit tests, to avoid future regressions.

[wlav]

The above mod "works" b/c dropping the kIsReference flag above changes the behavior to what you wanted it to be with the original change, but is still changes the intended semantics.

The example given isn't worrisome. This is the problem:

A *x = new B;

void change() { x = new A; }

and is why the semantics as written are what they are.