fix TStreamerLoop counter offset in member-wise streaming

io/io/src/TStreamerInfoActions.cxx

@@ -238,12 +238,29 @@ namespace TStreamerInfoActions
 
    struct TConfStreamerLoop : public TConfiguration {
       bool fIsPtrPtr = false; // Which are we, an array of objects or an array of pointers to objects?
+      Longptr_t fCounterOffset = 0; // Offset of the '//[n]' counter, relative to the streamed object.
 
       TConfStreamerLoop(TVirtualStreamerInfo *info, UInt_t id, TCompInfo_t *compinfo, Int_t offset, bool isPtrPtr)
-         : TConfiguration(info, id, compinfo, offset), fIsPtrPtr(isPtrPtr)
+         : TConfiguration(info, id, compinfo, offset), fIsPtrPtr(isPtrPtr),
+           fCounterOffset((Longptr_t)compinfo->fMethod)
       {
       }
 
+      void AddToOffset(Int_t delta) override
+      {
+         // Add the (potentially negative) delta to the configuration's offsets.  This is used by
+         // TBranchElement in the case of a split sub-object and by the member-wise streaming of a
+         // base class, where the whole action sequence of the base is shifted by the base-class
+         // offset.  The counter of the variable-size array lives in the same class as the array,
+         // so its offset must be shifted by the same delta as fOffset; it cannot be taken from
+         // the shared fCompInfo->fMethod, which stays relative to the declaring (base) class.
+
+         if (fOffset != TVirtualStreamerInfo::kMissing) {
+            fOffset += delta;
+            fCounterOffset += delta;
+         }
+      }
+
       TConfiguration *Copy() override { return new TConfStreamerLoop(*this); };
    };
 
@@ -1572,7 +1589,7 @@ namespace TStreamerInfoActions
          UInt_t ioffset = actionConfig->fOffset;
          // Get any private streamer which was set for the data member.
          TMemberStreamer* pstreamer = actionConfig->fCompInfo->fStreamer;
-         Int_t* counter = (Int_t*) ((char *) addr /*entry pointer*/ + actionConfig->fCompInfo->fMethod /*counter offset*/);
+         Int_t* counter = (Int_t*) ((char *) addr /*entry pointer*/ + ((const TConfStreamerLoop*)actionConfig)->fCounterOffset /*counter offset*/);
          // And call the private streamer, passing it the buffer, the object, and the counter.
          (*pstreamer)(buf, (char *) addr /*entry pointer*/ + ioffset /*object offset*/, *counter);
          return 0;
@@ -1586,7 +1603,7 @@ namespace TStreamerInfoActions
          bool isPtrPtr = ((TConfStreamerLoop*)config)->fIsPtrPtr;
 
          // Get the counter for the varying length array.
-         Int_t vlen = *((Int_t*) ((char *) addr /*entry pointer*/ + config->fCompInfo->fMethod /*counter offset*/));
+         Int_t vlen = *((Int_t*) ((char *) addr /*entry pointer*/ + ((const TConfStreamerLoop*)config)->fCounterOffset /*counter offset*/));
 
          //b << vlen;
          if (vlen) {
@@ -1629,7 +1646,7 @@ namespace TStreamerInfoActions
          bool isPtrPtr = ((TConfStreamerLoop*)config)->fIsPtrPtr;
 
          // Get the counter for the varying length array.
-         Int_t vlen = *((Int_t*) ((char *) addr /*entry pointer*/ + config->fCompInfo->fMethod /*counter offset*/));
+         Int_t vlen = *((Int_t*) ((char *) addr /*entry pointer*/ + ((const TConfStreamerLoop*)config)->fCounterOffset /*counter offset*/));
          //b << vlen;
          if (vlen) {
             // Get a pointer to the array of pointers.
@@ -1723,7 +1740,7 @@ namespace TStreamerInfoActions
 
          // Get the counter for the varying length array.
          Int_t vlen = *((Int_t *)((char *)addr /*entry pointer*/ +
-                                 config->fCompInfo->fMethod /*counter offset*/));
+                                 ((const TConfStreamerLoop*)config)->fCounterOffset /*counter offset*/));
          // Int_t realLen;
          // b >> realLen;
          // if (realLen != vlen) {
@@ -1821,7 +1838,7 @@ namespace TStreamerInfoActions
 
          // Get the counter for the varying length array.
          Int_t vlen = *((Int_t *)((char *)addr /*entry pointer*/ +
-                                 config->fCompInfo->fMethod /*counter offset*/));
+                                 ((const TConfStreamerLoop*)config)->fCounterOffset /*counter offset*/));
          // Int_t realLen;
          // b >> realLen;
          // if (realLen != vlen) {

io/io/test/CMakeLists.txt

@@ -37,6 +37,14 @@ endif()
 
 ROOT_ADD_GTEST(ZipHeader ZipHeader.cxx LIBRARIES RIO Tree ROOTNTuple)
 
+ROOT_STANDARD_LIBRARY_PACKAGE(StreamerLoopMemberwise
+                              NO_INSTALL_HEADERS
+                              HEADERS ${CMAKE_CURRENT_SOURCE_DIR}/StreamerLoopMemberwise.hxx
+                              SOURCES StreamerLoopMemberwise.cxx
+                              LINKDEF StreamerLoopMemberwiseLinkDef.h
+                              DEPENDENCIES RIO Tree)
+ROOT_ADD_GTEST(TStreamerLoopMemberwise TStreamerLoopMemberwise.cxx LIBRARIES RIO Tree StreamerLoopMemberwise)
+
 # Temporarily disabled.  Test routinely fails on MacOS and some Linuxes.
 #if(NOT WIN32 AND (NOT MACOS_VERSION OR NOT MACOSX_VERSION VERSION_LESS 13.00))
 #  ROOT_EXECUTABLE(TMapFileTest TMapFileTest.cxx LIBRARIES RIO Hist New)

io/io/test/StreamerLoopMemberwise.cxx

@@ -0,0 +1 @@
+#include "StreamerLoopMemberwise.hxx"

io/io/test/StreamerLoopMemberwise.hxx

@@ -0,0 +1,72 @@
+#ifndef ROOT_TEST_STREAMERLOOP_MEMBERWISE
+#define ROOT_TEST_STREAMERLOOP_MEMBERWISE
+
+#include <Rtypes.h>
+
+// Classes for the regression test of member-wise streaming of a variable-size
+// array (`//[n]`, a TStreamerLoop element) that lives in a base class at a
+// non-zero offset. See io/io/src/TStreamerInfoActions.cxx
+// (TConfStreamerLoop::fCounterOffset) and TStreamerLoopMemberwise.cxx.
+
+namespace ROOTTest {
+namespace StreamerLoopMemberwise {
+
+// Element stored in the variable-size array (a class, so the array is a
+// TStreamerLoop and not a TStreamerBasicPointer).
+class Hit {
+public:
+   Hit() = default;
+   Hit(int a, int b) : fA(a), fB(b) {}
+   bool operator==(const Hit &o) const { return fA == o.fA && fB == o.fB; }
+   int fA = 0;
+   int fB = 0;
+   ClassDefNV(Hit, 1)
+};
+
+class Frame {
+public:
+   Frame() = default;
+   ~Frame() { delete[] fHits; }
+   Frame(const Frame &o) { Set(o.fN, o.fHits); }
+   Frame &operator=(const Frame &o)
+   {
+      if (this != &o)
+         Set(o.fN, o.fHits);
+      return *this;
+   }
+   void Set(int n, const Hit *hits)
+   {
+      delete[] fHits;
+      fHits = nullptr;
+      fN = 0;
+      if (n > 0) {
+         fHits = new Hit[n];
+         for (int i = 0; i < n; ++i)
+            fHits[i] = hits[i];
+         fN = n;
+      }
+   }
+   int fN = 0;
+   Hit *fHits = nullptr; //[fN]
+   ClassDef(Frame, 1)
+};
+
+// A base preceding Frame, so that the Frame base does not sit at offset 0.
+class Pad {
+public:
+   Pad() = default;
+   int fX = 0;
+   ClassDef(Pad, 1)
+};
+
+// The collection element: Frame is a base at a non-zero offset.
+class Super : public Pad, public Frame {
+public:
+   Super() = default;
+   ClassDef(Super, 1)
+};
+
+} // namespace StreamerLoopMemberwise
+} // namespace ROOTTest
+
+#endif

io/io/test/StreamerLoopMemberwiseLinkDef.h

@@ -0,0 +1,13 @@
+#ifdef __CLING__
+
+#pragma link off all globals;
+#pragma link off all classes;
+#pragma link off all functions;
+
+#pragma link C++ class ROOTTest::StreamerLoopMemberwise::Hit + ;
+#pragma link C++ class ROOTTest::StreamerLoopMemberwise::Frame + ;
+#pragma link C++ class ROOTTest::StreamerLoopMemberwise::Pad + ;
+#pragma link C++ class ROOTTest::StreamerLoopMemberwise::Super + ;
+#pragma link C++ class std::vector < ROOTTest::StreamerLoopMemberwise::Super> + ;
+
+#endif

io/io/test/TStreamerLoopMemberwise.cxx

@@ -0,0 +1,72 @@
+#include "StreamerLoopMemberwise.hxx"
+
+#include "gtest/gtest.h"
+
+#include <TFile.h>
+#include <TTree.h>
+#include <TSystem.h>
+
+#include <vector>
+
+using namespace ROOTTest::StreamerLoopMemberwise;
+
+// Regression test for the member-wise streaming of a variable-size array
+// (`Hit* fHits; //[fN]`, i.e. a TStreamerLoop element) that lives in a base
+// class (Frame) at a non-zero offset inside the collection element (Super).
+//
+// See https://github.com/root-project/root/issues/22895 for more information.
+TEST(TStreamerLoopMemberwise, VariableArrayInBaseClass)
+{
+   const char *fname = "streamerloop_memberwise.root";
+   const int kFrames = 4;
+
+   int expectedHits = 0;
+   for (int f = 0; f < kFrames; ++f)
+      expectedHits += f + 1; // every frame non-empty; n > 0 is what triggered the original bug
+
+   {
+      std::vector<Super> slice(kFrames);
+      for (int f = 0; f < kFrames; ++f) {
+         std::vector<Hit> hits;
+         for (int i = 0; i <= f; ++i)
+            hits.emplace_back(100 * f + i, 7);
+         slice[f].Set(static_cast<int>(hits.size()), hits.data());
+      }
+
+      TFile file(fname, "RECREATE");
+      TTree tree("T", "T");
+      std::vector<Super> *ptr = &slice;
+      tree.Branch("slice", &ptr); // default split level -> member-wise collection
+      tree.Fill();
+      tree.Write();
+   }
+
+   int readHits = 0;
+   int nullBuffers = 0;
+   {
+      TFile file(fname);
+      auto *tree = file.Get<TTree>("T");
+      ASSERT_NE(tree, nullptr);
+      std::vector<Super> *slice = nullptr;
+      tree->SetBranchAddress("slice", &slice);
+      ASSERT_GT(tree->GetEntry(0), 0);
+      ASSERT_NE(slice, nullptr);
+      ASSERT_EQ(static_cast<int>(slice->size()), kFrames);
+
+      for (int f = 0; f < kFrames; ++f) {
+         const Frame &frame = (*slice)[f];
+         EXPECT_EQ(frame.fN, f + 1);
+         if (frame.fN > 0 && frame.fHits == nullptr)
+            ++nullBuffers;
+         for (int i = 0; frame.fHits && i < frame.fN; ++i) {
+            EXPECT_EQ(frame.fHits[i], Hit(100 * f + i, 7));
+            ++readHits;
+         }
+      }
+   }
+
+   EXPECT_EQ(nullBuffers, 0);
+   EXPECT_EQ(readHits, expectedHits);
+
+   gSystem->Unlink(fname);
+}