Const types can return invalid iterator ranges

[tmadlener]

When converting a type to its const version, e.g. ExampleMC to ConstExampleMC, the iterator ranges describing OneToManyRelations break if there is no related object. This seems to be true for other types as well. At least it also occurs for the edm4hep::ReconstructedParticle from EDM4hep

With the following code snippet it is possible to produce a segmentation violation in the second loop over the collection, where each element is converted to a ConstExampleMC before printing the daughters.

podio::EventStore store;
auto& collection = store.create<ExampleMCCollection>("collection");

// Create a few elements
for (int i = 0; i < 4; ++i) {
  auto elem = collection.create();
  elem.PDG(i + 10000);
}

// Add an alement adding some of the previously created elements as daughters
auto motherElem = collection.create();
motherElem.PDG(100);
motherElem.adddaughters(collection[1]);
motherElem.adddaughters(collection[2]);
motherElem.adddaughters(collection[3]);

// Add a daughter to the first element in the collection to show that the
// problem is not related to the position in the collection
collection[0].adddaughters(collection[3]);

// This works without problems and everything is as expected
for (const auto& elem : collection) {
  std::cout << "element: " << elem.id() << " -> PDG: " << elem.PDG()
            << " (" << elem.daughters_size() << " daughters)" << std::endl;
  for (auto it = elem.daughters_begin(); it != elem.daughters_end(); ++it) {
    std::cout << "  daughter: " << it->id() << " -> PDG: " << it->PDG() << std::endl;
  }
}

std::cout << std::endl << std::endl;


for (const auto& elem : collection) {
  // Convert to a ConstExampleMC and see things break
  ConstExampleMC constElem{elem};
  std::cout << "const element: " << elem.id() << " -> PDG: " << constElem.PDG()
            << " (" << constElem.daughters_size() << " daughters)" << std::endl;
  // As soon as there are no daughters, this iterator range is invalid and we
  // get a segmentation violation
  for (auto it = constElem.daughters_begin(); it != constElem.daughters_end(); ++it) {
    std::cout << "  daughter: " << it->id() << " -> PDG: " << it->PDG() << std::endl;
  }
}

The first loop works and gives the desired output:

element: 10000000 -> PDG: 10000 (1 daughters)
  daughter: 10000003 -> PDG: 10003
element: 10000001 -> PDG: 10001 (0 daughters)
element: 10000002 -> PDG: 10002 (0 daughters)
element: 10000003 -> PDG: 10003 (0 daughters)
element: 10000004 -> PDG: 100 (3 daughters)
  daughter: 10000001 -> PDG: 10001
  daughter: 10000002 -> PDG: 10002
  daughter: 10000003 -> PDG: 10003

During the second loop, the first element gets printed as expected, but trying to loop over the second element (without any daughters) a segmentation violation occurs:

const element: 10000000 -> PDG: 10000 (1 daughters)
  daughter: 10000003 -> PDG: 10003
const element: 10000001 -> PDG: 10001 (0 daughters)

 *** Break *** segmentation violation
  daughter: 

The stacktrace is the following:

#0  0x00007f98b00a1c2a in __GI___wait4 (pid=941256, stat_loc=stat_loc
entry=0x7ffcbe3d29f8, options=options
entry=0, usage=usage
entry=0x0) at ../sysdeps/unix/sysv/linux/wait4.c:27
#1  0x00007f98b00a1beb in __GI___waitpid (pid=<optimized out>, stat_loc=stat_loc
entry=0x7ffcbe3d29f8, options=options
entry=0) at waitpid.c:38
#2  0x00007f98b00110e7 in do_system (line=<optimized out>) at ../sysdeps/posix/system.c:172
#3  0x00007f98afe2a069 in TUnixSystem::Exec (shellcmd=<optimized out>, this=0x5596e81bf800) at /tmp/tmadlener/spack-stage/spack-stage-root-6.20.04-3mcg47i4cbc57dngsf3ab3lpsp63xlny/spack-src/core/unix/src/TUnixSystem.cxx:2107
#4  TUnixSystem::StackTrace (this=0x5596e81bf800) at /tmp/tmadlener/spack-stage/spack-stage-root-6.20.04-3mcg47i4cbc57dngsf3ab3lpsp63xlny/spack-src/core/unix/src/TUnixSystem.cxx:2397
#5  0x00007f98afe271fc in TUnixSystem::DispatchSignals (this=0x5596e81bf800, sig=kSigSegmentationViolation) at /tmp/tmadlener/spack-stage/spack-stage-root-6.20.04-3mcg47i4cbc57dngsf3ab3lpsp63xlny/spack-src/core/unix/src/TUnixSystem.cxx:3628
#6  <signal handler called>
#7  0x00007f98b046aece in ConstExampleMC::getObjectID (this=0x0) at /home/tmadlener/work/podio/tests/src/ExampleMCConst.cc:104
#8  0x00005596e75cebcb in ConstExampleMC::id (this=0x0) at /home/tmadlener/work/podio/tests/datamodel/ExampleMCConst.h:83
#9  main () at /home/tmadlener/work/podio/tests/invalid_iterator/invalid_iterator_repro.cpp:46

Apparently ConstExampleMC::id gets called on an invalid object. However, this should not even happen, since daughters_begin and daughters_end should describe an empty range, and it should not be possible to enter the loop.

The major problem is that T can be implicitly converted to ConstT (e.g. at function boundaries), so that the explicit conversion I did here is not too far fetched to actually happen.

[tmadlener]

Looking through earlier pull requests, this has already been found and fixed for the non const version in #16